Skip to content

feat: Add Go Gin API (016) + Angular UI (017)#172

Merged
WhatIfWeDigDeeper merged 30 commits into
mainfrom
feat/016-017-go-gin-angular
Feb 28, 2026
Merged

feat: Add Go Gin API (016) + Angular UI (017)#172
WhatIfWeDigDeeper merged 30 commits into
mainfrom
feat/016-017-go-gin-angular

Conversation

@WhatIfWeDigDeeper

@WhatIfWeDigDeeper WhatIfWeDigDeeper commented Feb 26, 2026

Copy link
Copy Markdown
Owner

Summary

Go Gin API (spec 016)

  • Full 15-endpoint REST API matching OpenAPI contract
  • Type-safe SQL via manually-written sqlc-style db package (pgx v5)
  • Snapshot-based history with field diffs and restore-to-version
  • CSV import (multipart), export, and template download
  • testcontainers-go integration tests (no mocks — real PostgreSQL)
  • golangci-lint config (errcheck, govet, staticcheck, revive)
  • Migration: go-api/migrations/001_initial.sqlgo_gin schema

Angular UI (spec 017)

  • Angular 21 standalone components (no NgModule)
  • Angular Signals for all component state (signal(), computed())
  • Reactive Forms with typed FormGroup for application CRUD
  • CanDeactivate guard for unsaved-changes prompt
  • History panel: slide-in timeline with field diffs and restore
  • CSV import (file picker + result summary) and export
  • @testing-library/angular + Jest — 15 unit tests passing
  • Proxy config routes /api → Go API on port 5070

Cross-stack fixes

  • Action menu clipping fix: Angular's 3-dot menu was hidden by the table's overflow-x-auto container. Fixed by rendering the dropdown portal outside that container. Added data-menu-dropdown attribute to all 6 other UI implementations for consistency.
  • E2E test: Extended action-menu.spec.ts with a bounding-box test that verifies the dropdown is fully within the viewport (catches overflow clipping in any stack).

Enum alignment (NestJS compatibility)

Aligned Go Gin's DB schema with NestJS so CSVs can be imported across implementations without errors:

  • company_category: replaced 7 size-based values with 18 NestJS sector-based values (ai, education, health, cybersecurity, etc.)
  • skills_match: changed from TEXT enum (strong_match etc.) to INTEGER 1–5
  • job_source: renamed company_websitecompany-website (hyphen); added friend/colleague; removed referral
  • Added TestImportCSV_CrossImplementationValues test

Code review fixes (round 1)

  • Spec status: Marked specs 016 and 017 as Complete
  • govulncheck: Fixed audit script to pass ./... argument
  • Dockerfile: Updated to golang:1.24-alpine (matches go.mod)
  • nginx proxy: Fixed double-prefix bug — proxy_pass http://go-api:5070/ (trailing / strips the /api/ location prefix correctly)
  • Migration 005: Aligns application_status CHECK constraint with NestJS/Angular values ('given offer', 'accepted offer', 'declined offer', 'no offer'); migrates any legacy rows
  • RemoveStage phantom snapshots: DeleteStage now returns pgconn.CommandTag; RemoveStage checks RowsAffected() == 0 and returns a 404 instead of creating a snapshot for a non-existent stage
  • RestoreToVersion transaction: Added querier interface (satisfied by both *pgxpool.Pool and pgx.Tx); all six DB writes in RestoreToVersion now execute inside a transaction, with CreateSnapshot called after commit

Copilot review fixes (round 2)

  • Migration 005 NULL violation: WHEN 'withdrawn' THEN NULLWHEN 'withdrawn' THEN 'rejected' — prevents NOT NULL constraint violation at runtime
  • ErrSnapshotNotFound sentinel: Added typed sentinel error; restoreHistory handler now returns 404 (not 500) when snapshot not found
  • CreateSnapshot post-commit: Log failure with log.Printf instead of returning error to client — restore was already committed; client should not receive 500
  • ErrStageNotFound sentinel: Replaced fragile err.Error() == "stage not found" string comparison with errors.Is(err, service.ErrStageNotFound)
  • Angular subscription leaks: Injected DestroyRef + takeUntilDestroyed in history-panel (2 subscriptions) and application-list (4 subscriptions)
  • config.go: Log warning when DATABASE_URL fails to parse instead of silently using wrong schema
  • go-gin schema docs: Regenerated; removed stale cross-schema enum list (go_gin uses TEXT + CHECK, no PG enum types)
  • DATABASE_ARCHITECTURE.md: Fixed broken filename 001_initial.sql001_initial.up.sql
  • stop-all.sh: Restored ascending port order (5070 before 5160)
  • api/package.json: Removed ^ from eslint version (exact version per project convention)
  • Spec refs: Go 1.23+ → 1.24+, Angular 19 → 21

Test Plan

  • Start Go API: npm run dev:go-api (requires PostgreSQL + npm run migrate:go)
  • Start Angular UI: npm run dev:angular-ui
  • Manual smoke test: create/edit/delete application, add stages, view history, CSV export
  • Run Go unit tests: npm run test:go-api (requires Docker for testcontainers-go)
  • Run Angular unit tests: npm run test:angular-ui
  • Run E2E tests: npm run test:e2e:angular (requires both servers running)
  • Verify npm run build:go-api and npm run build:angular-ui both pass
  • Import NestJS-exported CSV via Angular UI — expect 30 imported, 0 errors

New npm Scripts

Script Description
dev:go-api Start Go Gin API dev server
dev:angular-ui Start Angular dev server (port 3060)
build:go-api / build:go Go build
build:angular-ui / build:angular Angular build
lint:go-api golangci-lint
lint:angular-ui ng lint
test:go-api testcontainers-go integration tests
test:angular-ui Jest + @testing-library/angular
test:e2e:angular Playwright E2E (port 3060)
migrate:go Run go_gin schema migrations

🤖 Generated with Claude Code

WhatIfWeDigDeeper and others added 25 commits February 26, 2026 15:18
- Go 1.23 + Gin + pgx v5 + sqlc (manual db package)
- Port 5070, schema go_gin
- Full CRUD, interview stages, snapshot history, CSV import/export
- testcontainers-go integration tests
- npm scripts, stop-all.sh, docker-compose, README, DATABASE_ARCHITECTURE docs

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Angular 19 standalone components with Signals
- Reactive Forms, Angular Router with CanDeactivate guard
- Application list, detail/edit, history panel, CSV import/export
- @testing-library/angular unit tests with Jest (15 tests passing)
- Tailwind CSS 4.x styling
- Proxy to Go Gin API (port 5070), served on port 3060
- npm scripts (build:angular, lint:angular-ui, test:angular-ui, test:e2e:angular)
- stop-all.sh port 3060, playwright.config.ts port 3060
- docker-compose-all.yml angular-ui service with nginx
- README.md implementation docs

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Resolved conflicts in package.json, docker-compose-all.yml, and README.md,
combining scripts and docs for both implementations:
- go-api: port 5070, go_gin schema
- angular-ui: port 3060, proxies to go-api
…bsent

When running via 'cd go-api && go run ./cmd/server' from the monorepo,
the CWD is go-api/ but DATABASE_URL lives in the root .env file.
Try local .env first, then ../. env as fallback.
…ui script

ng is installed locally in angular-ui/node_modules/.bin — calling it directly
from the monorepo root fails. npm start adds node_modules/.bin to PATH automatically.
Tailwind CSS 4.x requires @tailwindcss/postcss to work with Angular's
PostCSS pipeline. Without it the @import "tailwindcss" in styles.css
is not processed and no styles are applied.
…ly reads .json not .mjs

Angular's Application builder (esbuild) ignores postcss.config.mjs/.js;
it only reads postcss.config.json or .postcssrc.json. Also add @source
directives so the utility scanner finds component HTML and TS files.
…er to prevent clipping

Move the 3-dot menu portal outside the overflow-x-auto table div so it is
not clipped in Safari. Add data-menu-dropdown attribute to all UI
implementations and extend action-menu.spec.ts with a bounding-box test
that verifies the dropdown is fully within the viewport.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…SV import

- Migrate company_category from size-based to sector-based values (18 NestJS values)
- Migrate skills_match from TEXT enum to INTEGER 1-5
- Migrate job_source to match NestJS values (company-website hyphen, add friend/colleague)
- Update Go types, query params, CSV import/export accordingly
- Add TestImportCSV_CrossImplementationValues test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Updated eslint from ^10.0.0 to 10.0.2 in package.json files for tanstack-start-ui, tanstack-ui, ui, and vue-ui.
- Updated package-lock.json files to reflect the new eslint version and its dependencies.
- Updated various dependencies such as @eslint/config-array, @eslint/object-schema, minimatch, and espree to their latest versions.
- Adjusted node engine requirements for several dependencies to support a wider range of Node.js versions.
- Added overrides for minimatch in vue-ui to ensure compatibility with editorconfig.
…tibility

- Replace removed setup-jest import with setupZoneTestEnv from setup-env/zone
- Add isolatedModules: true to tsconfig.spec.json for Angular 21 subpath resolution
- Explicit transform config passing tsconfig.spec.json to jest-preset-angular
- Remove GHSA-3ppc-4f35-3m26 from audit allowlists (no longer needed)
- Drop ./... from audit:ci:go-api govulncheck command
- Add public/favicon.svg with Angular shield (DD0031/C3002F) and white A letterform
- Update index.html link rel from favicon.ico to favicon.svg (image/svg+xml)
- Update specs/core/ui/screens.md: add App Shell section documenting
  favicon and page title conventions for all implementations
- Add docs/types/angular-ui/application.model.mermaid (ts-to-mermaid)
- Add docs/schema/go-gin/ (tbls ERD for go_gin schema)
- Add docs:types:angular-ui script to package.json
- Include angular-ui in docs:types aggregate script
- Add go_gin to generate-schema-docs.sh SCHEMAS array
- Fix README: go_gin schema table row links to docs/schema/go-gin/
- Add angular-ui type diagram link to README Type Diagrams section
- Add port 3060 to isTargetUI in csv-import-export.spec.ts
- Refactor csv-export to anchor tags matching test contract (Export CSV, Template)
- Refactor csv-import: visible file input, Import Applications heading,
  Download template link, Close button, data-testid on errors section,
  separate Imported/Skipped/Errors lines
- Wire (closeImport) event in application-list template
- Update csv-import.component.spec.ts to match new component API
- Always render Import button (disabled when no file) — removes @if(selectedFile())
  conditional that caused WebKit timing issues with setInputFiles
- Emit importSuccess output after successful import
- Reload applications list on importSuccess to show newly imported rows
Remove inner @if(errors.length > 0) around the error <ul> so the list
is always in the DOM — prevents WebKit from missing the Row N text due
to deferred @if change detection after the result signal updates.
…@for timing

@if creates/destroys DOM across two CD cycles in WebKit — the outer @if
resolving true does not guarantee inner @for items render in the same tick.
Replace @if(result()) and @if(error()) with [hidden] so all elements are
always in the DOM; Angular updates bindings including @for in one pass
when the result() signal changes.
…mbering

- Add ImportError struct {Row int, Message string} to Go API
- Change ImportResult.Errors from []string to []ImportError
- Start rowNum at 2 (header=row 1, data rows start at 2) to match test expectations
- Angular errorLines computed maps {row,message} to 'Row N: message' strings
- Use [hidden] + pre interpolation to avoid WebKit @for/@if timing issues
@WhatIfWeDigDeeper

Copy link
Copy Markdown
Owner Author

Code review

Found 7 issues:

  1. Spec status not updated to Complete — both spec files still have Status: Draft. CLAUDE.md says "When a feature has a spec file in specs/, update its Status to Complete before merging the PR."

- **Created**: 2026-02-26
- **Status**: Draft

  1. audit:ci:go-api calls govulncheck without ./...govulncheck without arguments may not scan all packages. Should be govulncheck ./... (the spec's tasks.md explicitly documents this form).

"audit:ci:fastapi": "cd fastapi && uv export --frozen | uvx pip-audit --strict -r /dev/stdin --disable-pip --no-deps",
"audit:ci:go-api": "cd go-api && govulncheck",
"audit:ci:angular-ui": "cd angular-ui && npx -y audit-ci --config .auditconfig.json",

  1. Dockerfile uses Go 1.23 but go.mod and CI require Go 1.24 — building in Docker will use the wrong toolchain version.

FROM golang:1.23-alpine AS builder
WORKDIR /app

  1. Status enum mismatch between Angular UI and Go API database — the Angular model uses 'given offer', 'accepted offer', 'declined offer', 'no offer' but the Go API's PostgreSQL enum only allows 'phone_screen', 'offer', 'withdrawn', 'accepted'. Saving any of those Angular-side values will fail with a CHECK constraint violation (500 error). Migrations 003/004 aligned company_category and job_source but application_status was not updated to match.

Angular model:

export type ApplicationStatus =
| 'unsubmitted'
| 'applied'
| 'interviewing'
| 'given offer'
| 'accepted offer'
| 'declined offer'
| 'rejected'
| 'no offer';

Go API enum:

CREATE TYPE go_gin.application_status AS ENUM (
'unsubmitted', 'applied', 'phone_screen', 'interviewing',
'offer', 'rejected', 'withdrawn', 'accepted'

  1. nginx.conf proxy doubles the /api/ prefix in Docker — the Angular dev proxy (proxy.conf.json) strips the /api prefix before forwarding to the Go API (which registers routes under /applications, not /api/applications). But nginx.conf proxies location /api/ to http://go-api:5070/api/, so in Docker every API request hits go-api:5070/api/applications and gets a 404. Should be proxy_pass http://go-api:5070/;.

location /api/ {
proxy_pass http://go-api:5070/api/;
proxy_http_version 1.1;

  1. RemoveStage creates a phantom history snapshot when the stage doesn't existdb.DeleteStage uses pool.Exec(), which succeeds silently with 0 rows affected when the stage ID is not found. CreateSnapshot then fires unconditionally, writing a false "Removed interview stage" history entry even though nothing was deleted. RowsAffected() on the command tag should be checked before creating the snapshot.

// RemoveStage removes an interview stage and returns the full application.
func RemoveStage(ctx context.Context, pool *pgxpool.Pool, applicationID, stageID string) (*ApplicationResponse, error) {
uid, err := parseUUID(applicationID)
if err != nil {
return nil, fmt.Errorf("invalid application UUID: %w", err)
}
sid, err := parseUUID(stageID)
if err != nil {
return nil, fmt.Errorf("invalid stage UUID: %w", err)
}
if err := db.DeleteStage(ctx, pool, uid, sid); err != nil {
return nil, err
}
if err := CreateSnapshot(ctx, pool, uid, "Removed interview stage"); err != nil {
return nil, err
}
return getApplicationWithStages(ctx, pool, uid)

  1. RestoreToVersion is not wrapped in a transaction — it performs multiple sequential DB writes (UPDATE application, DELETE all stages in a loop, INSERT all stages in a loop, CreateSnapshot) using the pool directly with no transaction. If any step fails the application is left in a partially-restored state. The same issue was caught and fixed in the Python FastAPI implementation (PR feat: add Python FastAPI API backend with asyncpg and Pydantic v2 #132). Should use pool.BeginTx and pass the pgx.Tx through each DB call.

// RestoreToVersion restores an application to a specific snapshot version.
func RestoreToVersion(ctx context.Context, pool *pgxpool.Pool, applicationID, snapshotID string) (*ApplicationResponse, error) {
uid, err := parseUUID(applicationID)
if err != nil {
return nil, fmt.Errorf("invalid application UUID: %w", err)
}
sid, err := parseUUID(snapshotID)
if err != nil {
return nil, fmt.Errorf("invalid snapshot UUID: %w", err)
}
snap, err := db.GetSnapshot(ctx, pool, sid, uid)
if err != nil {
return nil, err
}
if snap == nil {
return nil, fmt.Errorf("snapshot not found")
}
var data SnapshotData
if err := json.Unmarshal(snap.SnapshotData, &data); err != nil {
return nil, fmt.Errorf("failed to unmarshal snapshot: %w", err)
}
app := data.Application
// Restore application fields
updateParams := db.UpdateApplicationParams{
ID: uid,
CompanyName: app.CompanyName,
PositionTitle: app.PositionTitle,
Status: app.Status,
DateApplied: toNullableDate(app.DateApplied),
CompanyURL: toNullableText(app.CompanyURL),
JobPostingURL: toNullableText(app.JobPostingURL),
CompanyCareerURL: toNullableText(app.CompanyCareerURL),
CompanyCategory: toNullableText(app.CompanyCategory),
SkillsMatch: toNullableInt4(app.SkillsMatch),
JobSource: toNullableText(app.JobSource),
SalaryMin: toNullableInt4(app.SalaryMin),
SalaryMax: toNullableInt4(app.SalaryMax),
CoverLetterRequired: app.CoverLetterRequired,
OfferDueDate: toNullableDate(app.OfferDueDate),
SpecialRequirements: toNullableText(app.SpecialRequirements),
Notes: toNullableText(app.Notes),
}
_, err = db.UpdateApplication(ctx, pool, updateParams)
if err != nil {
return nil, err
}
// Replace all interview stages
existingStages, err := db.GetStagesByApplicationID(ctx, pool, uid)
if err != nil {
return nil, err
}
for _, s := range existingStages {
if err := db.DeleteStage(ctx, pool, uid, s.ID); err != nil {
return nil, err
}
}
for _, s := range data.InterviewStages {
p := db.CreateStageParams{
ApplicationID: uid,
StageName: s.StageName,
StageOrder: s.StageOrder,
IsCompleted: s.IsCompleted,
PerformanceRating: toNullableText(s.PerformanceRating),
Notes: toNullableText(s.Notes),
}
if _, err := db.CreateStage(ctx, pool, p); err != nil {
return nil, err
}
}
desc := fmt.Sprintf("Restored to version %d", snap.SequenceNumber)
if err := CreateSnapshot(ctx, pool, uid, desc); err != nil {
return nil, err
}
return getApplicationWithStages(ctx, pool, uid)
}

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

- Mark specs 016 and 017 as Complete
- Fix govulncheck script to pass ./... argument
- Update Dockerfile to golang:1.24-alpine
- Fix nginx proxy double-prefix bug (drop /api/ suffix from proxy_pass)
- Add migration 005 to align application_status CHECK constraint with NestJS/Angular values
- Return pgconn.CommandTag from DeleteStage; RemoveStage checks RowsAffected to avoid phantom snapshots on missing stage (404)
- Add querier interface so DB functions accept both *pgxpool.Pool and pgx.Tx
- Wrap RestoreToVersion writes in a transaction; CreateSnapshot called after commit

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds two new full-stack implementations to the monorepo: a Go Gin API (spec 016, port 5070) using Go 1.24 + Gin + pgx v5 + sqlc, and an Angular UI (spec 017, port 3060) using Angular 21 standalone components with Signals and Reactive Forms. It also delivers several cross-stack fixes: a data-menu-dropdown attribute added to all 6 existing UI implementations' action menus (to prevent clipping), enum alignment between Go Gin and NestJS for cross-implementation CSV compatibility, and audit allowlist cleanup removing a now-patched CVE across all packages.

Changes:

  • New Go Gin REST API with 15 endpoints, testcontainers-go integration tests, raw SQL migrations, and Docker support
  • New Angular 21 UI with standalone components, Angular Signals state management, @testing-library/angular unit tests, and Nginx Dockerfile
  • Cross-stack fixes: action menu data-menu-dropdown attribute, enum alignment migrations, ESLint version pinning, and audit allowlist cleanup

Reviewed changes

Copilot reviewed 135 out of 151 changed files in this pull request and generated 18 comments.

Show a summary per file
File Description
go-api/cmd/server/main.go Gin server entry point with auto-migration and pool init
go-api/cmd/migrate/main.go Standalone migration CLI runner
go-api/internal/config/config.go Config loading from env with search_path injection
go-api/internal/db/models.go + pool.go Manually written sqlc-style models and pgxpool setup
go-api/internal/handler/applications.go CRUD + archive/restore HTTP handlers
go-api/internal/handler/stages.go Interview stage handlers with fragile string-based error checking
go-api/internal/handler/history.go History + restore handlers
go-api/internal/handler/csv.go CSV import/export/template handlers
go-api/internal/handler/router.go Gin route registration with CORS middleware
go-api/internal/service/stages.go Stage CRUD service
go-api/internal/service/history.go Snapshot-based history, diffs, restore-in-transaction
go-api/internal/migrations/*.up.sql Embedded migration files (001–005)
go-api/migrations/*.up.sql Standalone migration files (dual copies of same SQL)
go-api/sql/queries/*.sql sqlc query definitions
go-api/tests/*.go testcontainers-go integration tests
go-api/go.mod Go module with exact version deps
go-api/Dockerfile Multi-stage Go build
angular-ui/src/app/** Angular standalone components, service, models, guards, pipes
angular-ui/package.json Angular 21 + Jest dependencies (exact versions)
angular-ui/nginx.conf Nginx config with /api/ reverse proxy
angular-ui/Dockerfile Multi-stage Angular build with Nginx
angular-ui/proxy.conf.json Dev proxy config for /api → port 5070
vue-ui/src/components/ApplicationCard.vue + others Added data-menu-dropdown attribute to all existing UIs
tests/e2e/action-menu.spec.ts New bounding-box test for dropdown overflow clipping
tests/e2e/csv-import-export.spec.ts Extended to include Angular UI (port 3060)
*/. auditconfig.json (multiple) Removed GHSA-3ppc-4f35-3m26 from all allowlists
*/package.json (multiple) Pinned eslint to exact version 10.0.2
docker-compose-all.yml Added go-api and angular-ui services
scripts/stop-all.sh Added ports 3060 and 5070
playwright.config.ts Added Angular UI dev server entry
docs/ + specs/ Schema docs, type diagrams, spec files
README.md Updated with implementation 7
Files not reviewed (11)
  • api/package-lock.json: Language not supported
  • hono-api/package-lock.json: Language not supported
  • koa-api/package-lock.json: Language not supported
  • nest-api/package-lock.json: Language not supported
  • nuxt-api/package-lock.json: Language not supported
  • react-ui/package-lock.json: Language not supported
  • svelte-ui/package-lock.json: Language not supported
  • tanstack-start-ui/package-lock.json: Language not supported
  • tanstack-ui/package-lock.json: Language not supported
  • ui/package-lock.json: Language not supported
  • vue-ui/package-lock.json: Language not supported

Comment thread specs/017-angular-ui/spec.md Outdated
Comment on lines +8 to +13
Add an Angular frontend (`angular-ui/`) that connects to the Go Gin API (port 5070). Completes the "big 4" frontend frameworks in the portfolio (React × 3, Vue, Svelte, Angular). Uses modern Angular 19 patterns: standalone components, Angular Signals for component state, Reactive Forms with typed controls, and `@testing-library/angular` for user-centric unit tests.

## Technology Stack

### Frontend (`angular-ui/` — port 3060)
- Angular 19+ (latest stable)

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The specs/017-angular-ui/spec.md describes the implementation as using "Angular 19" (lines 8, 12, 13), but the actual implementation uses Angular 21 (as specified in both the PR description and angular-ui/package.json where all @angular/* packages are 21.2.0). The spec version references are inconsistent with the implemented version.

Copilot uses AI. Check for mistakes.
Comment on lines +128 to +137
loadHistory() {
this.loading.set(true);
this.service.getHistory(this.applicationId).subscribe({
next: (entries) => {
this.history.set(entries);
this.loading.set(false);
},
error: () => this.loading.set(false),
});
}

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The HistoryPanelComponent subscribes to service.getHistory() in loadHistory() (line 130) and to service.restoreHistory() in onRestore() (line 160) without ever unsubscribing or completing those observables. Since this component uses OnChanges (and can be reloaded multiple times), each call to loadHistory() creates a new subscription that is never cleaned up, potentially causing memory leaks and multiple active HTTP subscriptions if the component is destroyed and recreated. The subscriptions should be managed using the takeUntilDestroyed() operator or stored and unsubscribed in ngOnDestroy.

Copilot uses AI. Check for mistakes.
Comment on lines +147 to +165
onArchive(id: string) {
this.openMenuId.set(null);
this.menuPosition.set(null);
this.service.archive(id).subscribe(() => this.loadApplications());
}

onRestore(id: string) {
this.openMenuId.set(null);
this.menuPosition.set(null);
this.service.restore(id).subscribe(() => this.loadApplications());
}

onDelete(id: string) {
this.openMenuId.set(null);
this.menuPosition.set(null);
if (confirm('Delete this application?')) {
this.service.delete(id).subscribe(() => this.loadApplications());
}
}

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ApplicationListComponent methods onArchive, onRestore, and onDelete (lines 147-165) subscribe to service observables without managing those subscriptions. If the component is destroyed before the HTTP response returns (e.g., user navigates away), the callbacks still execute and call loadApplications(), potentially causing errors or unexpected state changes on an already-destroyed component. These subscriptions should use takeUntilDestroyed() or equivalent cleanup.

Copilot uses AI. Check for mistakes.
| applications_job_source_check | CHECK | CHECK ((job_source = ANY (ARRAY['recruiter'::text, 'linkedin'::text, 'indeed'::text, 'friend'::text, 'colleague'::text, 'company-website'::text, 'other'::text]))) |
| applications_position_title_not_null | n | NOT NULL position_title |
| applications_skills_match_check | CHECK | CHECK (((skills_match >= 1) AND (skills_match <= 5))) |
| applications_status_check | CHECK | CHECK ((status = ANY (ARRAY['unsubmitted'::text, 'applied'::text, 'phone_screen'::text, 'interviewing'::text, 'offer'::text, 'rejected'::text, 'withdrawn'::text, 'accepted'::text]))) |

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The applications_status_check constraint documented in go_gin.applications.md still shows the old migration 002 values: 'unsubmitted', 'applied', 'phone_screen', 'interviewing', 'offer', 'rejected', 'withdrawn', 'accepted'. After migration 005, the constraint should be 'unsubmitted', 'applied', 'interviewing', 'given offer', 'accepted offer', 'declined offer', 'rejected', 'no offer'. This schema doc is stale and inconsistent with the actual applied migrations, which could mislead developers.

Copilot uses AI. Check for mistakes.
Comment on lines +34 to +37
app, err := service.RestoreToVersion(c.Request.Context(), pool, id, historyID)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
return

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The restoreHistory handler (lines 34-43) does not differentiate between "snapshot not found" errors and other internal errors — all service errors result in a 500 response. When RestoreToVersion returns an error from fmt.Errorf("snapshot not found"), the handler should return 404, not 500. This leads to confusing API behavior for clients that send an invalid snapshot ID.

Copilot uses AI. Check for mistakes.
Comment on lines +2 to +8
UPDATE go_gin.applications SET status = CASE status
WHEN 'phone_screen' THEN 'interviewing'
WHEN 'offer' THEN 'given offer'
WHEN 'accepted' THEN 'accepted offer'
WHEN 'withdrawn' THEN NULL
ELSE status
END;

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The migration 005_align_status_with_nestjs.up.sql sets status = NULL for rows with status = 'withdrawn' (line 6). However, the status column has a NOT NULL constraint (established in migration 001 and carried forward). This UPDATE statement will fail at runtime with a constraint violation because NULL cannot be stored in a non-nullable column. The row should be mapped to an existing valid status value (e.g., 'rejected') instead of NULL.

Copilot uses AI. Check for mistakes.
Comment thread docs/DATABASE_ARCHITECTURE.md Outdated
- No ORM — uses raw SQL queries with asyncpg

**Go-Gin-pgx:**
- Schema defined in: `go-api/migrations/001_initial.sql`

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs/DATABASE_ARCHITECTURE.md entry for the Go-Gin-pgx implementation (line 48) references go-api/migrations/001_initial.sql as the schema definition, but the actual file is named 001_initial.up.sql (not 001_initial.sql). This broken reference would mislead developers trying to find the initial schema.

Copilot uses AI. Check for mistakes.
Comment thread scripts/stop-all.sh Outdated
Comment on lines +18 to +19
5160 # FastAPI
5070 # Go API

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stop-all.sh script has port 5070 (Go API) added at line 19, but it's placed after port 5160 (FastAPI). Based on the ascending numerical ordering of all other ports in the list (3030, 3040, 3050, 3060, 5001, 5010, 5030, 5040, 5050, 5160), port 5070 should appear before 5160 to maintain consistency. This is a minor ordering issue only.

Copilot uses AI. Check for mistakes.
Comment on lines +27 to +36
// Recover from testcontainers panicking when Docker socket is not reachable.
defer func() {
if r := recover(); r != nil {
msg := fmt.Sprintf("%v", r)
if strings.Contains(msg, "Docker") || strings.Contains(msg, "docker") {
t.Skip("Docker not available, skipping integration test: " + msg)
}
panic(r)
}
}()

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The helper_test.go uses defer inside setupTestDB (lines 28-36) to recover from panics and skip the test when Docker is unavailable. However, this defer is registered at the beginning of setupTestDB, and the t.Skip() call inside the deferred function will not properly skip the test because t.Skip() must be called from the goroutine running the test. Inside a defer called after a panic from a separate goroutine (which testcontainers might spin up), the t.Skip() call may not propagate correctly. The testcontainers-go library typically returns errors rather than panicking for Docker unavailability; this recover() pattern is fragile.

Copilot uses AI. Check for mistakes.
Comment on lines +89 to +92
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getApplication handler (line 90) returns http.StatusBadRequest for all errors, including cases where parsing the UUID fails or a database error occurs. A UUID parse error or a database error should typically return http.StatusBadRequest and http.StatusInternalServerError respectively (rather than both being StatusBadRequest). More critically, if service.GetApplication encounters a database error, it will return a 400 instead of 500, which is semantically incorrect and confusing to API consumers.

Copilot uses AI. Check for mistakes.
WhatIfWeDigDeeper and others added 3 commits February 28, 2026 07:32
- Migration 005: map 'withdrawn' to 'rejected' (not NULL) to avoid NOT NULL violation
- Add ErrSnapshotNotFound sentinel; return 404 from restoreHistory handler
- Log CreateSnapshot failure post-commit instead of returning error to client
- Add ErrStageNotFound sentinel; use errors.Is instead of string comparison
- Angular: pipe takeUntilDestroyed into all bare subscribe() calls
- config.go: log warning when DATABASE_URL fails to parse
- Regenerate go-gin schema docs; remove stale cross-schema enum list
- Fix broken migration filename ref in DATABASE_ARCHITECTURE.md
- Reorder stop-all.sh ports to ascending order (5070 before 5160)
- Exact eslint version in api/package.json (remove ^)
- Spec version refs: Go 1.23+ -> 1.24+, Angular 19 -> 21

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…confirm()

- Create reusable ConfirmDialogComponent in shared/components
- Replace inline modals in application-detail with app-confirm-dialog
- Replace window.confirm() in application-list with signals + app-confirm-dialog
- Add dialog/confirmation pattern guidance to specs/core/ui/components.md
- Add package change and type diagram steps to CLAUDE.md validation chain

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add dark:hover:file:bg-gray-600 to step up from gray-700 on hover,
making the state change visible against the dark background.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@WhatIfWeDigDeeper WhatIfWeDigDeeper merged commit 9c6846f into main Feb 28, 2026
1 check passed
@WhatIfWeDigDeeper WhatIfWeDigDeeper deleted the feat/016-017-go-gin-angular branch February 28, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants