From 6a5213588143b6b1fac925ee7eab1955f1878c03 Mon Sep 17 00:00:00 2001 From: riddim-developer-bot Date: Sun, 14 Jun 2026 13:18:19 -0400 Subject: [PATCH] [EPAC-2287] Serve bill version diffs --- .../internal/adapter/sqlite/repository.go | 135 ++++++++++++- .../adapter/sqlite/repository_test.go | 161 +++++++++++++++ backend/bills/internal/domain/domain.go | 15 ++ backend/bills/internal/usecase/bills.go | 39 ++++ backend/bills/internal/usecase/bills_test.go | 130 ++++++++++++ .../internal/usecase/open_bills_index.go | 2 + backend/bills/main.go | 87 +++++++- backend/bills/main_test.go | 186 ++++++++++++++++++ backend/openapi/openapi.json | 3 +- docs/architecture/use-case-catalog.md | 14 +- 10 files changed, 761 insertions(+), 11 deletions(-) create mode 100644 backend/bills/internal/adapter/sqlite/repository_test.go create mode 100644 backend/bills/internal/usecase/bills_test.go diff --git a/backend/bills/internal/adapter/sqlite/repository.go b/backend/bills/internal/adapter/sqlite/repository.go index bfe9d1e5..3fc593d6 100644 --- a/backend/bills/internal/adapter/sqlite/repository.go +++ b/backend/bills/internal/adapter/sqlite/repository.go @@ -169,6 +169,50 @@ func (r *Repository) GetBillCommitteeStage(ctx context.Context, id string) (*dom }, nil } +func (r *Repository) GetBillVersionDiff(ctx context.Context, id, fromVersionID, toVersionID string) (*domain.BillVersionDiff, error) { + billID, err := r.lookupBillID(ctx, id) + if err != nil { + return nil, err + } + if ok, err := r.tableExists(ctx, "bill_diffs"); err != nil || !ok { + return nil, err + } + if ok, err := r.tableExists(ctx, "bill_clause_diffs"); err != nil || !ok { + return nil, err + } + + var diffID string + err = r.db.QueryRowContext(ctx, ` + SELECT id + FROM bill_diffs + WHERE bill_id = ? AND from_version_id = ? AND to_version_id = ? + LIMIT 1`, billID, fromVersionID, toVersionID).Scan(&diffID) + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("query bill diff sqlite artifact: %w", err) + } + + fromVersion, ok, err := r.billVersionByID(ctx, billID, fromVersionID) + if err != nil || !ok { + return nil, err + } + toVersion, ok, err := r.billVersionByID(ctx, billID, toVersionID) + if err != nil || !ok { + return nil, err + } + clauses, err := r.billClauseDiffs(ctx, billID, diffID) + if err != nil { + return nil, err + } + return &domain.BillVersionDiff{ + From: fromVersion, + To: toVersion, + Clauses: clauses, + }, nil +} + func (r *Repository) queryBills(ctx context.Context, query string, args ...any) ([]domain.Bill, error) { rows, err := r.db.QueryContext(ctx, query, args...) if err != nil { @@ -396,12 +440,12 @@ func (r *Repository) billVersions(ctx context.Context, billID string) ([]domain. WHERE %s = ? ORDER BY %s`, columnExpr(columns, "id", "version_id"), - columnExpr(columns, "label", "version_label", "name"), + columnExpr(columns, "label", "version_label", "name", "stage"), columnExpr(columns, "title"), columnExpr(columns, "stage"), columnExpr(columns, "chamber"), - columnExpr(columns, "published_on", "version_date", "date"), - columnExpr(columns, "source_url", "url"), + columnExpr(columns, "published_on", "published_date", "version_date", "date"), + columnExpr(columns, "source_url", "html_url", "url", "text_source_url", "xml_url", "pdf_url"), billIDColumn, orderExpr(columns), ) @@ -433,6 +477,91 @@ func (r *Repository) billVersions(ctx context.Context, billID string) ([]domain. return versions, nil } +func (r *Repository) billVersionByID(ctx context.Context, billID, versionID string) (domain.BillVersion, bool, error) { + columns, ok, err := r.tableColumns(ctx, "bill_versions") + if err != nil || !ok { + return domain.BillVersion{}, false, err + } + billIDColumn := firstColumn(columns, "bill_id", "legisinfo_id") + versionIDColumn := firstColumn(columns, "id", "version_id") + if billIDColumn == "" || versionIDColumn == "" { + return domain.BillVersion{}, false, nil + } + query := fmt.Sprintf(` + SELECT %s, %s, %s, %s, %s, %s, %s + FROM bill_versions + WHERE %s = ? AND %s = ? + LIMIT 1`, + columnExpr(columns, "id", "version_id"), + columnExpr(columns, "label", "version_label", "name", "stage"), + columnExpr(columns, "title"), + columnExpr(columns, "stage"), + columnExpr(columns, "chamber"), + columnExpr(columns, "published_on", "published_date", "version_date", "date"), + columnExpr(columns, "source_url", "html_url", "url", "text_source_url", "xml_url", "pdf_url"), + billIDColumn, + versionIDColumn, + ) + + var version domain.BillVersion + var id, label, title, stage, chamber, publishedOn, sourceURL sql.NullString + err = r.db.QueryRowContext(ctx, query, billID, versionID).Scan( + &id, + &label, + &title, + &stage, + &chamber, + &publishedOn, + &sourceURL, + ) + if errors.Is(err, sql.ErrNoRows) { + return domain.BillVersion{}, false, nil + } + if err != nil { + return domain.BillVersion{}, false, fmt.Errorf("query bill version sqlite artifact: %w", err) + } + version.ID = stringValue(id) + version.Label = stringValue(label) + version.Title = stringValue(title) + version.Stage = stringValue(stage) + version.Chamber = stringValue(chamber) + version.PublishedOn = stringPtr(publishedOn) + version.SourceURL = stringValue(sourceURL) + return version, true, nil +} + +func (r *Repository) billClauseDiffs(ctx context.Context, billID, diffID string) ([]domain.BillClauseDiff, error) { + rows, err := r.db.QueryContext(ctx, ` + SELECT id, label, change_type, from_text, to_text, hansard_anchor_url + FROM bill_clause_diffs + WHERE bill_id = ? AND diff_id = ? + ORDER BY sort_order, rowid`, billID, diffID) + if err != nil { + return nil, fmt.Errorf("query bill clause diffs sqlite artifact: %w", err) + } + defer rows.Close() + + clauses := make([]domain.BillClauseDiff, 0) + for rows.Next() { + var clause domain.BillClauseDiff + var id, label, changeType, fromText, toText, hansardAnchorURL sql.NullString + if err := rows.Scan(&id, &label, &changeType, &fromText, &toText, &hansardAnchorURL); err != nil { + return nil, fmt.Errorf("scan bill clause diffs sqlite artifact: %w", err) + } + clause.ID = stringValue(id) + clause.Label = stringValue(label) + clause.ChangeType = stringValue(changeType) + clause.FromText = stringValue(fromText) + clause.ToText = stringValue(toText) + clause.HansardAnchorURL = stringPtr(hansardAnchorURL) + clauses = append(clauses, clause) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterate bill clause diffs sqlite artifact: %w", err) + } + return clauses, nil +} + func (r *Repository) billAmendments(ctx context.Context, billID string) ([]domain.BillAmendment, error) { columns, ok, err := r.tableColumns(ctx, "bill_amendments") if err != nil || !ok { diff --git a/backend/bills/internal/adapter/sqlite/repository_test.go b/backend/bills/internal/adapter/sqlite/repository_test.go new file mode 100644 index 00000000..24875eb6 --- /dev/null +++ b/backend/bills/internal/adapter/sqlite/repository_test.go @@ -0,0 +1,161 @@ +package sqlite + +import ( + "context" + "database/sql" + "errors" + "testing" + + "epac/bills/internal/usecase" + + _ "modernc.org/sqlite" +) + +func TestRepositoryGetBillVersionDiffMapsCurrentArtifactSchema(t *testing.T) { + db := openDiffFixtureDB(t) + defer db.Close() + repo := New(db) + + diff, err := repo.GetBillVersionDiff(context.Background(), "C-2", "v1", "v2") + if err != nil { + t.Fatalf("GetBillVersionDiff error: %v", err) + } + if diff == nil { + t.Fatal("diff = nil") + } + if diff.From.ID != "v1" || diff.From.Label != "First Reading" || diff.From.Stage != "First Reading" { + t.Fatalf("from version = %+v", diff.From) + } + if diff.From.PublishedOn == nil || *diff.From.PublishedOn != "2026-06-01" { + t.Fatalf("from published_on = %+v", diff.From.PublishedOn) + } + if diff.From.SourceURL != "https://www.parl.ca/v1" { + t.Fatalf("from source_url = %q", diff.From.SourceURL) + } + if diff.To.ID != "v2" || diff.To.Label != "Third Reading" { + t.Fatalf("to version = %+v", diff.To) + } + if len(diff.Clauses) != 2 { + t.Fatalf("clauses = %+v", diff.Clauses) + } + if diff.Clauses[0].ID != "clause-1" || diff.Clauses[0].ChangeType != "added" || diff.Clauses[0].FromText != "" { + t.Fatalf("first clause = %+v", diff.Clauses[0]) + } + if diff.Clauses[1].ID != "clause-2" || diff.Clauses[1].HansardAnchorURL == nil || *diff.Clauses[1].HansardAnchorURL != "https://hansard.test/clause-2" { + t.Fatalf("second clause = %+v", diff.Clauses[1]) + } +} + +func TestRepositoryGetBillVersionDiffReturnsNilForUnknownVersionPair(t *testing.T) { + db := openDiffFixtureDB(t) + defer db.Close() + repo := New(db) + + diff, err := repo.GetBillVersionDiff(context.Background(), "C-2", "v1", "missing") + if err != nil { + t.Fatalf("GetBillVersionDiff error: %v", err) + } + if diff != nil { + t.Fatalf("diff = %+v, want nil", diff) + } +} + +func TestRepositoryGetBillVersionDiffReturnsNilWhenDiffTablesAreAbsent(t *testing.T) { + db, err := sql.Open("sqlite", ":memory:") + if err != nil { + t.Fatalf("open db: %v", err) + } + db.SetMaxOpenConns(1) + defer db.Close() + if _, err := db.Exec(` + CREATE TABLE bills (id TEXT PRIMARY KEY, number TEXT NOT NULL); + INSERT INTO bills (id, number) VALUES ('13543613', 'C-2'); + `); err != nil { + t.Fatalf("seed db: %v", err) + } + repo := New(db) + + diff, err := repo.GetBillVersionDiff(context.Background(), "C-2", "v1", "v2") + if err != nil { + t.Fatalf("GetBillVersionDiff error: %v", err) + } + if diff != nil { + t.Fatalf("diff = %+v, want nil", diff) + } +} + +func TestRepositoryGetBillVersionDiffReturnsBillNotFound(t *testing.T) { + db := openDiffFixtureDB(t) + defer db.Close() + repo := New(db) + + diff, err := repo.GetBillVersionDiff(context.Background(), "C-404", "v1", "v2") + if !errors.Is(err, usecase.ErrBillNotFound) { + t.Fatalf("error = %v, want ErrBillNotFound", err) + } + if diff != nil { + t.Fatalf("diff = %+v, want nil", diff) + } +} + +func openDiffFixtureDB(t *testing.T) *sql.DB { + t.Helper() + + db, err := sql.Open("sqlite", ":memory:") + if err != nil { + t.Fatalf("open db: %v", err) + } + db.SetMaxOpenConns(1) + statements := []string{ + `CREATE TABLE bills ( + id TEXT PRIMARY KEY, + number TEXT NOT NULL + )`, + `CREATE TABLE bill_versions ( + bill_id TEXT NOT NULL, + id TEXT NOT NULL, + stage TEXT NOT NULL DEFAULT '', + html_url TEXT NOT NULL DEFAULT '', + published_date TEXT, + sort_order INTEGER NOT NULL DEFAULT 0 + )`, + `CREATE TABLE bill_diffs ( + bill_id TEXT NOT NULL, + id TEXT NOT NULL, + from_version_id TEXT NOT NULL, + to_version_id TEXT NOT NULL, + source_url TEXT NOT NULL DEFAULT '', + PRIMARY KEY (bill_id, id) + )`, + `CREATE TABLE bill_clause_diffs ( + bill_id TEXT NOT NULL, + diff_id TEXT NOT NULL, + id TEXT NOT NULL, + label TEXT, + change_type TEXT NOT NULL, + from_text TEXT, + to_text TEXT, + hansard_anchor_url TEXT, + sort_order INTEGER NOT NULL, + PRIMARY KEY (bill_id, diff_id, id) + )`, + `INSERT INTO bills (id, number) VALUES ('13543613', 'C-2')`, + `INSERT INTO bill_versions (bill_id, id, stage, html_url, published_date, sort_order) VALUES + ('13543613', 'v1', 'First Reading', 'https://www.parl.ca/v1', '2026-06-01', 1), + ('13543613', 'v2', 'Third Reading', 'https://www.parl.ca/v2', '2026-06-10', 2)`, + `INSERT INTO bill_diffs (bill_id, id, from_version_id, to_version_id, source_url) + VALUES ('13543613', 'diff-1', 'v1', 'v2', 'https://www.parl.ca/diff')`, + `INSERT INTO bill_clause_diffs ( + bill_id, diff_id, id, label, change_type, from_text, to_text, hansard_anchor_url, sort_order + ) VALUES + ('13543613', 'diff-1', 'clause-2', 'Clause 2', 'modified', 'Old text', 'New text', 'https://hansard.test/clause-2', 2), + ('13543613', 'diff-1', 'clause-1', 'Clause 1', 'added', NULL, 'Inserted text', NULL, 1)`, + } + for _, statement := range statements { + if _, err := db.Exec(statement); err != nil { + _ = db.Close() + t.Fatalf("exec fixture statement: %v", err) + } + } + return db +} diff --git a/backend/bills/internal/domain/domain.go b/backend/bills/internal/domain/domain.go index 439a8c1a..88eb8c07 100644 --- a/backend/bills/internal/domain/domain.go +++ b/backend/bills/internal/domain/domain.go @@ -40,6 +40,21 @@ type BillAmendment struct { SourceURL string `json:"source_url,omitempty"` } +type BillClauseDiff struct { + ID string `json:"id,omitempty"` + Label string `json:"label,omitempty"` + ChangeType string `json:"change_type"` + FromText string `json:"from_text"` + ToText string `json:"to_text"` + HansardAnchorURL *string `json:"hansard_anchor_url,omitempty"` +} + +type BillVersionDiff struct { + From BillVersion `json:"from"` + To BillVersion `json:"to"` + Clauses []BillClauseDiff `json:"clauses"` +} + type ParliamentaryCommittee struct { Code string `json:"code"` Name string `json:"name"` diff --git a/backend/bills/internal/usecase/bills.go b/backend/bills/internal/usecase/bills.go index 97f721f3..19fb7d16 100644 --- a/backend/bills/internal/usecase/bills.go +++ b/backend/bills/internal/usecase/bills.go @@ -13,6 +13,7 @@ type BillRepository interface { ListBills(ctx context.Context) ([]domain.Bill, error) GetBillDepth(ctx context.Context, id string) (domain.Bill, error) GetBillCommitteeStage(ctx context.Context, id string) (*domain.BillCommitteeStage, error) + GetBillVersionDiff(ctx context.Context, id, fromVersionID, toVersionID string) (*domain.BillVersionDiff, error) } type ListBillsInput struct { @@ -68,6 +69,44 @@ func (u *GetBillCommitteeStage) Execute(ctx context.Context, id string) (*domain return u.repo.GetBillCommitteeStage(ctx, id) } +type LoadBillVersionDiffInput struct { + BillID string + FromVersionID string + ToVersionID string +} + +type LoadBillVersionDiff struct { + repo BillRepository +} + +func NewLoadBillVersionDiff(repo BillRepository) *LoadBillVersionDiff { + return &LoadBillVersionDiff{repo: repo} +} + +func (u *LoadBillVersionDiff) Execute(ctx context.Context, input LoadBillVersionDiffInput) (*domain.BillVersionDiff, error) { + billID := strings.TrimSpace(input.BillID) + if billID == "" { + return nil, ErrBillNotFound + } + fromVersionID := strings.TrimSpace(input.FromVersionID) + if fromVersionID == "" { + return nil, ErrDiffMissingFrom + } + toVersionID := strings.TrimSpace(input.ToVersionID) + if toVersionID == "" { + return nil, ErrDiffMissingTo + } + + diff, err := u.repo.GetBillVersionDiff(ctx, billID, fromVersionID, toVersionID) + if err != nil || diff == nil { + return diff, err + } + if len(diff.Clauses) == 0 { + return nil, nil + } + return diff, nil +} + var normalizeRe = regexp.MustCompile(`[^a-z0-9]+`) func filterBills(bills []domain.Bill, statusFilter, parliamentFilter string) []domain.Bill { diff --git a/backend/bills/internal/usecase/bills_test.go b/backend/bills/internal/usecase/bills_test.go new file mode 100644 index 00000000..216624ff --- /dev/null +++ b/backend/bills/internal/usecase/bills_test.go @@ -0,0 +1,130 @@ +package usecase + +import ( + "context" + "errors" + "testing" + + "epac/bills/internal/domain" +) + +func TestLoadBillVersionDiffReturnsRepositoryDiff(t *testing.T) { + repo := &fakeBillRepository{ + diff: &domain.BillVersionDiff{ + From: domain.BillVersion{ID: "v1"}, + To: domain.BillVersion{ID: "v2"}, + Clauses: []domain.BillClauseDiff{{ + ID: "clause-1", + ChangeType: "modified", + FromText: "Old", + ToText: "New", + }}, + }, + } + + diff, err := NewLoadBillVersionDiff(repo).Execute(context.Background(), LoadBillVersionDiffInput{ + BillID: " C-2 ", + FromVersionID: " v1 ", + ToVersionID: " v2 ", + }) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + if diff == nil || diff.From.ID != "v1" || diff.To.ID != "v2" || len(diff.Clauses) != 1 { + t.Fatalf("diff = %+v", diff) + } + if repo.billID != "C-2" || repo.fromVersionID != "v1" || repo.toVersionID != "v2" { + t.Fatalf("repo input = %q/%q/%q", repo.billID, repo.fromVersionID, repo.toVersionID) + } +} + +func TestLoadBillVersionDiffValidatesRequiredInput(t *testing.T) { + tests := []struct { + name string + input LoadBillVersionDiffInput + want error + }{ + { + name: "missing bill", + input: LoadBillVersionDiffInput{FromVersionID: "v1", ToVersionID: "v2"}, + want: ErrBillNotFound, + }, + { + name: "missing from", + input: LoadBillVersionDiffInput{BillID: "C-2", ToVersionID: "v2"}, + want: ErrDiffMissingFrom, + }, + { + name: "missing to", + input: LoadBillVersionDiffInput{BillID: "C-2", FromVersionID: "v1"}, + want: ErrDiffMissingTo, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repo := &fakeBillRepository{} + diff, err := NewLoadBillVersionDiff(repo).Execute(context.Background(), tt.input) + if !errors.Is(err, tt.want) { + t.Fatalf("error = %v, want %v", err, tt.want) + } + if diff != nil { + t.Fatalf("diff = %+v, want nil", diff) + } + if repo.called { + t.Fatal("repository should not be called for invalid input") + } + }) + } +} + +func TestLoadBillVersionDiffMapsEmptyClauseDiffToUnavailable(t *testing.T) { + repo := &fakeBillRepository{ + diff: &domain.BillVersionDiff{ + From: domain.BillVersion{ID: "v1"}, + To: domain.BillVersion{ID: "v2"}, + Clauses: []domain.BillClauseDiff{}, + }, + } + + diff, err := NewLoadBillVersionDiff(repo).Execute(context.Background(), LoadBillVersionDiffInput{ + BillID: "C-2", + FromVersionID: "v1", + ToVersionID: "v2", + }) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + if diff != nil { + t.Fatalf("diff = %+v, want nil", diff) + } +} + +type fakeBillRepository struct { + diff *domain.BillVersionDiff + err error + called bool + billID string + fromVersionID string + toVersionID string +} + +func (f *fakeBillRepository) ListBills(context.Context) ([]domain.Bill, error) { + return nil, nil +} + +func (f *fakeBillRepository) GetBillDepth(context.Context, string) (domain.Bill, error) { + return domain.Bill{}, nil +} + +func (f *fakeBillRepository) GetBillCommitteeStage(context.Context, string) (*domain.BillCommitteeStage, error) { + return nil, nil +} + +func (f *fakeBillRepository) GetBillVersionDiff(_ context.Context, billID, fromVersionID, toVersionID string) (*domain.BillVersionDiff, error) { + f.called = true + f.billID = billID + f.fromVersionID = fromVersionID + f.toVersionID = toVersionID + return f.diff, f.err +} diff --git a/backend/bills/internal/usecase/open_bills_index.go b/backend/bills/internal/usecase/open_bills_index.go index 1885a1e9..a605a808 100644 --- a/backend/bills/internal/usecase/open_bills_index.go +++ b/backend/bills/internal/usecase/open_bills_index.go @@ -12,6 +12,8 @@ var ( ErrChecksumMismatch = errors.New("bills index checksum mismatch") ErrSchemaMismatch = errors.New("bills index schema version mismatch") ErrBillNotFound = errors.New("bill not found") + ErrDiffMissingFrom = errors.New("missing required from version id") + ErrDiffMissingTo = errors.New("missing required to version id") ) type ManifestLoader interface { diff --git a/backend/bills/main.go b/backend/bills/main.go index 8280fbb3..03170bd6 100644 --- a/backend/bills/main.go +++ b/backend/bills/main.go @@ -1,5 +1,5 @@ -// bills Lambda - GET /api/v1/bills, GET /api/v1/bills/{id}, and -// GET /api/v1/bills/{id}/committee-stage +// bills Lambda - GET /api/v1/bills, GET /api/v1/bills/{id}, +// GET /api/v1/bills/{id}/diff, and GET /api/v1/bills/{id}/committee-stage package main import ( @@ -40,6 +40,8 @@ type BillDepthResponse struct { type Bill = domain.Bill type BillStage = domain.BillStage type BillVersion = domain.BillVersion +type BillVersionDiff = domain.BillVersionDiff +type BillClauseDiff = domain.BillClauseDiff type BillAmendment = domain.BillAmendment type BillCommitteeStage = domain.BillCommitteeStage @@ -87,6 +89,29 @@ func (r *billsRuntime) repository(ctx context.Context) (usecase.BillRepository, } func HandleRequest(ctx context.Context, req events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error) { + if id := billVersionDiffIDFromRequest(req); id != "" { + fromVersionID, toVersionID, message := billVersionDiffQueryFromRequest(req) + if message != "" { + return jsonError(http.StatusBadRequest, message), nil + } + repo, err := billData.repository(ctx) + if err != nil { + return mapInitializationError(err), nil + } + diff, err := usecase.NewLoadBillVersionDiff(repo).Execute(ctx, usecase.LoadBillVersionDiffInput{ + BillID: id, + FromVersionID: fromVersionID, + ToVersionID: toVersionID, + }) + if err != nil { + return mapBillVersionDiffError(err), nil + } + if diff == nil { + return noContentResponse(), nil + } + return jsonResponse(http.StatusOK, diff), nil + } + repo, err := billData.repository(ctx) if err != nil { return mapInitializationError(err), nil @@ -141,6 +166,50 @@ func billIDFromRequest(req events.APIGatewayProxyRequest) string { return "" } +func billVersionDiffIDFromRequest(req events.APIGatewayProxyRequest) string { + path := strings.Trim(req.Path, "/") + for _, prefix := range []string{"api/v1/bills/", "bills/"} { + if strings.HasPrefix(path, prefix) { + rest := strings.TrimPrefix(path, prefix) + if !strings.HasSuffix(rest, "/diff") { + return "" + } + id := strings.TrimSuffix(rest, "/diff") + if id == "" || strings.Contains(id, "/") { + return "" + } + return strings.TrimSpace(id) + } + } + if strings.Contains(req.Resource, "/diff") { + for _, key := range []string{"id", "bill_id", "legisinfo_id"} { + if id := strings.TrimSpace(req.PathParameters[key]); id != "" { + return id + } + } + } + return "" +} + +func billVersionDiffQueryFromRequest(req events.APIGatewayProxyRequest) (string, string, string) { + fromVersionID := strings.TrimSpace(req.QueryStringParameters["from"]) + toVersionID := strings.TrimSpace(req.QueryStringParameters["to"]) + missing := make([]string, 0, 2) + if fromVersionID == "" { + missing = append(missing, "from") + } + if toVersionID == "" { + missing = append(missing, "to") + } + if len(missing) == 1 { + return "", "", "missing required query parameter: " + missing[0] + } + if len(missing) > 1 { + return "", "", "missing required query parameters: " + strings.Join(missing, ", ") + } + return fromVersionID, toVersionID, "" +} + func billCommitteeStageIDFromRequest(req events.APIGatewayProxyRequest) string { path := strings.Trim(req.Path, "/") for _, prefix := range []string{"api/v1/bills/", "bills/"} { @@ -211,6 +280,20 @@ func mapBillError(err error) events.APIGatewayProxyResponse { return jsonError(http.StatusInternalServerError, "internal error") } +func mapBillVersionDiffError(err error) events.APIGatewayProxyResponse { + switch { + case errors.Is(err, usecase.ErrDiffMissingFrom): + return jsonError(http.StatusBadRequest, "missing required query parameter: from") + case errors.Is(err, usecase.ErrDiffMissingTo): + return jsonError(http.StatusBadRequest, "missing required query parameter: to") + case errors.Is(err, usecase.ErrBillNotFound): + return jsonError(http.StatusNotFound, "bill not found") + default: + slog.Error("load bill version diff request failed", "error", err) + return jsonError(http.StatusInternalServerError, "internal error") + } +} + func jsonResponse(status int, payload any, extraHeaders ...map[string]string) events.APIGatewayProxyResponse { body, err := json.Marshal(payload) if err != nil { diff --git a/backend/bills/main_test.go b/backend/bills/main_test.go index 9fc29ba0..4efa0c65 100644 --- a/backend/bills/main_test.go +++ b/backend/bills/main_test.go @@ -118,6 +118,143 @@ func TestHandleRequestGetsBillCommitteeStage(t *testing.T) { } } +func TestHandleRequestGetsBillVersionDiff(t *testing.T) { + dir := t.TempDir() + p45 := 45 + writeBillSQLiteUnitFixture(t, dir, []Bill{ + {ID: "C-2287", Number: "C-2287", Title: "Diff Act", Status: "InProgress", CurrentStage: "Third Reading", Parliament: &p45}, + }) + withLocalIndex(t, dir) + + resp, err := HandleRequest(context.Background(), events.APIGatewayProxyRequest{ + Path: "/api/v1/bills/C-2287/diff", + QueryStringParameters: map[string]string{"from": "C-2287-v1", "to": "C-2287-v2"}, + }) + if err != nil { + t.Fatalf("HandleRequest error: %v", err) + } + if resp.StatusCode != 200 { + t.Fatalf("status = %d, body = %s", resp.StatusCode, resp.Body) + } + var body BillVersionDiff + if err := json.Unmarshal([]byte(resp.Body), &body); err != nil { + t.Fatalf("decode body: %v", err) + } + if body.From.ID != "C-2287-v1" || body.To.ID != "C-2287-v2" { + t.Fatalf("versions = %+v -> %+v", body.From, body.To) + } + if len(body.Clauses) != 2 { + t.Fatalf("clauses = %+v", body.Clauses) + } + if body.Clauses[0].ID != "C-2287-clause-1" || body.Clauses[0].ChangeType != "added" || body.Clauses[0].FromText != "" { + t.Fatalf("first clause = %+v", body.Clauses[0]) + } + if body.Clauses[1].ID != "C-2287-clause-2" || body.Clauses[1].HansardAnchorURL == nil || *body.Clauses[1].HansardAnchorURL != "https://www.ourcommons.ca/hansard#clause-2" { + t.Fatalf("second clause = %+v", body.Clauses[1]) + } +} + +func TestHandleRequestBillVersionDiffMissingQueryReturns400(t *testing.T) { + tests := []struct { + name string + query map[string]string + body string + }{ + { + name: "both missing", + body: `{"error":"missing required query parameters: from, to"}`, + }, + { + name: "from missing", + query: map[string]string{"to": "C-2287-v2"}, + body: `{"error":"missing required query parameter: from"}`, + }, + { + name: "to missing", + query: map[string]string{"from": "C-2287-v1"}, + body: `{"error":"missing required query parameter: to"}`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resp, err := HandleRequest(context.Background(), events.APIGatewayProxyRequest{ + Path: "/api/v1/bills/C-2287/diff", + QueryStringParameters: tt.query, + }) + if err != nil { + t.Fatalf("HandleRequest error: %v", err) + } + if resp.StatusCode != 400 { + t.Fatalf("status = %d, body = %s", resp.StatusCode, resp.Body) + } + if resp.Body != tt.body { + t.Fatalf("body = %s", resp.Body) + } + }) + } +} + +func TestHandleRequestBillVersionDiffUnknownBillReturns404(t *testing.T) { + dir := t.TempDir() + p45 := 45 + writeBillSQLiteUnitFixture(t, dir, []Bill{ + {ID: "C-2287", Number: "C-2287", Title: "Diff Act", Status: "InProgress", Parliament: &p45}, + }) + withLocalIndex(t, dir) + + resp, err := HandleRequest(context.Background(), events.APIGatewayProxyRequest{ + Path: "/api/v1/bills/C-404/diff", + QueryStringParameters: map[string]string{"from": "v1", "to": "v2"}, + }) + if err != nil { + t.Fatalf("HandleRequest error: %v", err) + } + if resp.StatusCode != 404 { + t.Fatalf("status = %d, body = %s", resp.StatusCode, resp.Body) + } +} + +func TestHandleRequestBillVersionDiffUnknownVersionsReturnsNoContent(t *testing.T) { + dir := t.TempDir() + p45 := 45 + writeBillSQLiteUnitFixture(t, dir, []Bill{ + {ID: "C-2287", Number: "C-2287", Title: "Diff Act", Status: "InProgress", Parliament: &p45}, + }) + withLocalIndex(t, dir) + + resp, err := HandleRequest(context.Background(), events.APIGatewayProxyRequest{ + Path: "/api/v1/bills/C-2287/diff", + QueryStringParameters: map[string]string{"from": "missing", "to": "C-2287-v2"}, + }) + if err != nil { + t.Fatalf("HandleRequest error: %v", err) + } + if resp.StatusCode != 204 { + t.Fatalf("status = %d, body = %s", resp.StatusCode, resp.Body) + } +} + +func TestHandleRequestBillVersionDiffOneVersionBillReturnsNoContent(t *testing.T) { + dir := t.TempDir() + p45 := 45 + writeBillSQLiteUnitFixture(t, dir, []Bill{ + {ID: "C-1", Number: "C-1", Title: "One Version Act", Status: "InProgress", Parliament: &p45}, + }) + withLocalIndex(t, dir) + + resp, err := HandleRequest(context.Background(), events.APIGatewayProxyRequest{ + Path: "/api/v1/bills/C-1/diff", + QueryStringParameters: map[string]string{"from": "C-1-v1", "to": "C-1-v2"}, + }) + if err != nil { + t.Fatalf("HandleRequest error: %v", err) + } + if resp.StatusCode != 204 { + t.Fatalf("status = %d, body = %s", resp.StatusCode, resp.Body) + } +} + func TestHandleRequestReturnsNoContentWhenBillHasNoCommitteeStage(t *testing.T) { dir := t.TempDir() p45 := 45 @@ -238,6 +375,30 @@ func writeBillSQLiteUnitFixture(t *testing.T, dir string, bills []Bill) { )`); err != nil { t.Fatalf("create bill versions table: %v", err) } + if _, err := db.Exec(`CREATE TABLE bill_diffs ( + bill_id TEXT NOT NULL, + id TEXT NOT NULL, + from_version_id TEXT NOT NULL, + to_version_id TEXT NOT NULL, + source_url TEXT NOT NULL DEFAULT '', + PRIMARY KEY (bill_id, id) + )`); err != nil { + t.Fatalf("create bill diffs table: %v", err) + } + if _, err := db.Exec(`CREATE TABLE bill_clause_diffs ( + bill_id TEXT NOT NULL, + diff_id TEXT NOT NULL, + id TEXT NOT NULL, + label TEXT, + change_type TEXT NOT NULL, + from_text TEXT, + to_text TEXT, + hansard_anchor_url TEXT, + sort_order INTEGER NOT NULL, + PRIMARY KEY (bill_id, diff_id, id) + )`); err != nil { + t.Fatalf("create bill clause diffs table: %v", err) + } if _, err := db.Exec(`CREATE TABLE bill_amendments ( bill_id TEXT NOT NULL, id TEXT NOT NULL, @@ -306,6 +467,31 @@ func writeBillSQLiteUnitFixture(t *testing.T, dir string, bills []Bill) { VALUES ('C-2260', 'C-2260-v1', 'First reading', 'Depth Act first reading', 'House First Reading', 'House', '2026-06-01', 'https://www.parl.ca/version', 1)`); err != nil { t.Fatalf("insert version fixture: %v", err) } + if _, err := db.Exec(` + INSERT INTO bill_versions (bill_id, id, label, title, stage, chamber, published_on, source_url, sort_order) + VALUES ('C-1', 'C-1-v1', 'First reading', 'One Version Act first reading', 'House First Reading', 'House', '2026-06-01', 'https://www.parl.ca/c1/v1', 1)`); err != nil { + t.Fatalf("insert one-version fixture: %v", err) + } + if _, err := db.Exec(` + INSERT INTO bill_versions (bill_id, id, label, title, stage, chamber, published_on, source_url, sort_order) + VALUES + ('C-2287', 'C-2287-v1', 'First reading', 'Diff Act first reading', 'House First Reading', 'House', '2026-06-01', 'https://www.parl.ca/c2287/v1', 1), + ('C-2287', 'C-2287-v2', 'Third reading', 'Diff Act third reading', 'House Third Reading', 'House', '2026-06-10', 'https://www.parl.ca/c2287/v2', 2)`); err != nil { + t.Fatalf("insert diff versions fixture: %v", err) + } + if _, err := db.Exec(` + INSERT INTO bill_diffs (bill_id, id, from_version_id, to_version_id, source_url) + VALUES ('C-2287', 'C-2287-diff-v1-v2', 'C-2287-v1', 'C-2287-v2', 'https://www.parl.ca/c2287/diff')`); err != nil { + t.Fatalf("insert diff fixture: %v", err) + } + if _, err := db.Exec(` + INSERT INTO bill_clause_diffs ( + bill_id, diff_id, id, label, change_type, from_text, to_text, hansard_anchor_url, sort_order + ) VALUES + ('C-2287', 'C-2287-diff-v1-v2', 'C-2287-clause-2', 'Clause 2', 'modified', 'Old clause 2', 'New clause 2', 'https://www.ourcommons.ca/hansard#clause-2', 2), + ('C-2287', 'C-2287-diff-v1-v2', 'C-2287-clause-1', 'Clause 1', 'added', NULL, 'New clause 1', NULL, 1)`); err != nil { + t.Fatalf("insert clause diff fixture: %v", err) + } if _, err := db.Exec(` INSERT INTO bill_amendments (bill_id, id, number, title, status, stage, sponsor_name, proposed_on, text, source_url, sort_order) VALUES ('C-2260', 'C-2260-a1', 'NDP-1', 'Add review clause', 'adopted', 'Committee', 'Jane Example', '2026-06-02', 'Clause 2 is amended...', 'https://www.parl.ca/amendment', 1)`); err != nil { diff --git a/backend/openapi/openapi.json b/backend/openapi/openapi.json index a13062d4..bfbe5b9c 100644 --- a/backend/openapi/openapi.json +++ b/backend/openapi/openapi.json @@ -970,7 +970,8 @@ } } }, - "204": { "description": "Backend has no diff available for the requested version pair (text missing or diff job not run yet)." }, + "204": { "description": "Backend has no diff available for the requested version pair (one-version bill, text missing, unknown version pair, or diff job not run yet)." }, + "400": { "$ref": "#/components/responses/Error" }, "404": { "$ref": "#/components/responses/Error" }, "429": { "$ref": "#/components/responses/RateLimit" }, "500": { "$ref": "#/components/responses/Error" }, diff --git a/docs/architecture/use-case-catalog.md b/docs/architecture/use-case-catalog.md index 7c62ef9f..4299873a 100644 --- a/docs/architecture/use-case-catalog.md +++ b/docs/architecture/use-case-catalog.md @@ -101,7 +101,7 @@ to the issue that will build the missing artifact. | `MemberRepository` | iOS Swift | outbound | Implemented: `ios/epac/Domain/Ports/MemberRepository.swift`; adapter: `ios/epac/Data/Adapters/RidingLookupMemberRepository.swift`. | Resolve member-related lookups, starting with riding lookup by postal code for FindMyMP. | | `SittingRepository` | iOS Swift | outbound | Implemented: `ios/epac/Domain/Ports/SittingRepository.swift`; adapter: `ios/epac/Data/Adapters/HansardSittingRepositoryAdapter.swift`. | List sitting dates and load transcripts for a sitting date. | | `BillRepository` | iOS Swift | outbound | Implemented: `ios/epac/Domain/Ports/BillRepository.swift`; adapter: `ios/epac/Data/Adapters/LEGISinfoBillRepository.swift`. | List current-session bills. | -| `BillRepository` | backend Go | outbound | Implemented: `backend/bills/internal/usecase/bills.go`; adapter: `backend/bills/internal/adapter/sqlite/repository.go`. | List bills and load bill-depth rows from the verified bills SQLite artifact. | +| `BillRepository` | backend Go | outbound | Implemented: `backend/bills/internal/usecase/bills.go`; adapter: `backend/bills/internal/adapter/sqlite/repository.go`. | List bills, load bill-depth rows, load committee-stage rows, and load bill version diffs from the verified bills SQLite artifact. | | `BillCommitteeStageRepository` | iOS Swift | outbound | Implemented: `ios/epac/Domain/Ports/BillCommitteeStageRepository.swift`; adapter: `ios/epac/Data/Repositories/BackendBillCommitteeStageRepository.swift`. | Load the committee currently studying a bill, including study dates and meeting rows. | | `BillAmendmentsRepository` | iOS Swift | outbound | Implemented: `ios/epac/Domain/Ports/BillAmendmentsRepository.swift`; adapter: `ios/epac/Data/Repositories/BackendBillAmendmentsRepository.swift`. | Load amendments tabled against a bill (number, mover, stage, status, verbatim text) from the backend bill-depth endpoint. | | `BillVersionsRepository` | iOS Swift | outbound | Implemented: `ios/epac/Domain/Ports/BillVersionsRepository.swift`; adapter: `ios/epac/Data/Repositories/BackendBillVersionsRepository.swift`. | Load the published versions of a bill (label, stage, chamber, publication date, source URL) from the backend bill-depth endpoint. | @@ -752,14 +752,18 @@ Current implementation: ### LoadBillVersionDiff ``` -Actor: User (iOS app, bill diff viewer) +Actor: User (iOS app, bill diff viewer) / Backend API caller Goal: See what changed at the clause level between two published versions of a bill (additions, deletions, modifications) with the verbatim before/after clause text, and follow the change back to the chamber speech that introduced it when known. Inputs: LEGISinfo bill ID, "before" version ID, "after" version ID. -Outputs: Optional BillVersionDiff; nil when the backend cannot produce a diff for the requested version pair (either version is missing text, or the diff job has not run yet) — the diff viewer renders an unavailable state. +Outputs: Optional BillVersionDiff; nil when the backend cannot produce a diff for the requested version pair (one-version bill, unknown version pair, either version is missing text, or the diff job has not run yet) — the diff viewer renders an unavailable state. Entities / values: Bill, BillVersion, BillVersionDiff, BillClauseDiff, BillClauseChangeType. -Ports: iOS Swift: `BillVersionDiffRepository`. -Primary adapters: BackendBillVersionDiffRepository (GET /api/v1/bills/{id}/diff?from=…&to=…), BillVersionsDiffView. +Ports: iOS Swift: `BillVersionDiffRepository`; backend Go: `BillRepository.GetBillVersionDiff`. +Primary adapters: BackendBillVersionDiffRepository (GET /api/v1/bills/{id}/diff?from=…&to=…), backend bills Lambda handler, backend bills SQLite repository, BillVersionsDiffView. Current implementation: + backend/bills/main.go + backend/bills/internal/usecase/bills.go + backend/bills/internal/domain/domain.go + backend/bills/internal/adapter/sqlite/repository.go ios/epac/Application/LoadBillVersionDiff.swift ios/epac/Domain/Entities/BillVersionDiff.swift ios/epac/Domain/Ports/BillVersionDiffRepository.swift