Skip to content

Refactor getFirst to not depend on our PgAirtableDb abstraction#2620

Merged
Will-Howard merged 3 commits into
masterfrom
wh-getfirstpg-2026-06
Jun 9, 2026
Merged

Refactor getFirst to not depend on our PgAirtableDb abstraction#2620
Will-Howard merged 3 commits into
masterfrom
wh-getfirstpg-2026-06

Conversation

@Will-Howard

Copy link
Copy Markdown
Collaborator

Description

We use db.getFirst as a guard against issues due to duplicates. Previously, this was tightly coupled to PgAirtableDb. We want to keep using this when we move exercise responses and resource completions to postgres-only, so I'm splitting out a getFirstFromPg in this PR.

Issue

#2584

Developer checklist

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@Will-Howard, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 21 minutes and 13 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ab3d1857-9438-4740-a61f-1cf7377dc547

📥 Commits

Reviewing files that changed from the base of the PR and between fa38cc0 and 83d7657.

📒 Files selected for processing (7)
  • apps/website/src/server/routers/exercises.ts
  • apps/website/src/server/routers/resources.ts
  • libraries/db/.env.test
  • libraries/db/src/index.ts
  • libraries/db/src/lib/client.test.ts
  • libraries/db/src/lib/client.ts
  • libraries/db/src/lib/pg-query.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch wh-getfirstpg-2026-06

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Will-Howard

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review @greptileai review

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get PR details and files changed
gh pr view 2620 --repo bluedotimpact/bluedot --json files,title,body
echo "---FILES---"
gh pr diff 2620 --repo bluedotimpact/bluedot

Length of output: 27368



@Will-Howard: Clean refactor — the extraction is well-structured and the test suite is thorough. A few points worth considering:


1. resolveSortConfig is exported unnecessarily

In libraries/db/src/lib/pg-query.ts, resolveSortConfig is exported but it's only called within getFirstFromPg in the same file, and it isn't re-exported from index.ts or imported by client.ts. Unless it's expected to be used by future callers directly, it could be un-exported to keep the internal API surface smaller.

-export function resolveSortConfig<T extends PgTable>(
+function resolveSortConfig<T extends PgTable>(

2. No direct tests for getFirstFromPg with a plain PgTable

All tests in client.test.ts exercise db.getFirst, which delegates to getFirstFromPg. The primary motivation of this PR is standalone use of getFirstFromPg (e.g. getFirstFromPg(db.pg, exerciseResponseTable.pg, ...) as now done in exercises.ts/resources.ts), but there are no tests that call it directly with a bare PgTable. A couple of integration-style tests covering this path would give confidence the new public API works end-to-end.


3. filter: {} throws instead of matching all rows

In buildWhereClauseInner, an empty plain-object filter (not AND/OR) falls through to the field-iteration loop with zero entries, then throws 'No valid filter conditions found'. This is a pre-existing behaviour but now that getFirstFromPg is a wider-surface public API it may surprise callers who expect filter: {} to be equivalent to no filter. Worth either documenting or converting the empty case to a passthrough:

// at the end of buildWhereClauseInner, before the throw:
if (conditions.length === 0) {
  // empty filter object → no constraint (match all rows)
  return sql`TRUE`;
}

Minor: limit in GetFirstFromPgOptions

The limit option feels slightly mismatched with the getFirst name (implies 1 result). It was already in the original implementation so this isn't a regression, but it might be worth a JSDoc note clarifying the intent (e.g. "advanced use only; default is 1").


Overall the refactor is solid and the new tests are a good improvement over the placeholder that was there before. The points above are all non-blocking.


🧠 Learnings used
Learnt from: Will-Howard
Repo: bluedotimpact/bluedot PR: 1323
File: libraries/ui/src/Modal.tsx:46-48
Timestamp: 2025-09-11T08:09:21.405Z
Learning: Will-Howard prefers to keep PR scope focused and avoid including accessibility improvements that are outside the main diff changes, even when they would be beneficial additions.

Learnt from: alyssia-x
Repo: bluedotimpact/bluedot PR: 1477
File: apps/website/src/server/routers/exercises.ts:37-56
Timestamp: 2025-10-21T01:37:21.573Z
Learning: The database layer uses Airtable + Postgres (PgAirtableDb) which doesn't support atomic upserts due to Airtable API constraints. The codebase uses a read-then-write pattern (getFirst followed by update or insert) consistently across endpoints like exercise responses and unit feedback.

@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extracts getFirstFromPg (and its helpers buildWhereClause, resolveSortConfig, Filter, PgDatabase) from PgAirtableDb in client.ts into a standalone pg-query.ts module. PgAirtableDb.getFirst is then delegated to getFirstFromPg, and call-sites in exercises.ts and resources.ts are updated to call getFirstFromPg directly with raw Drizzle PgTable objects, decoupling the query logic from the Airtable abstraction.

  • libraries/db/src/lib/pg-query.ts — new file containing the extracted query helpers, exported as getFirstFromPg and GetFirstFromPgOptions from index.ts.
  • libraries/db/src/lib/client.ts — substantially slimmed down; getFirst now delegates entirely to getFirstFromPg, removing ~170 lines of duplicated logic.
  • client.test.ts — placeholder replaced with a comprehensive suite covering sorting, filtering, edge cases, and a direct getFirstFromPg test path.

Confidence Score: 5/5

This is a safe refactor — logic is extracted verbatim into a standalone function, the existing wrapper delegates to it unchanged, and the new test suite exercises the full call path including the direct getFirstFromPg route.

The extracted getFirstFromPg is a faithful port of the removed code in client.ts with no behavioral changes. All call-sites pass the same filter arguments as before, and the expanded test suite covers the previously untested direct-PgTable path that motivated the extraction.

No files require special attention.

Important Files Changed

Filename Overview
libraries/db/src/lib/pg-query.ts New module containing extracted getFirstFromPg, buildWhereClause, and resolveSortConfig; logic is an accurate port of the removed code in client.ts with resolveSortConfig correctly kept unexported.
libraries/db/src/lib/client.ts Substantially cleaned up; getFirst now delegates to getFirstFromPg, removing ~170 lines of logic that lived here before the extraction.
libraries/db/src/lib/client.test.ts Placeholder replaced with a thorough integration test suite covering db.getFirst and direct getFirstFromPg calls, sorting defaults, filter operators, AND/OR logic, and edge cases.
libraries/db/src/index.ts Exports getFirstFromPg and GetFirstFromPgOptions from the new pg-query module; Filter type is intentionally not re-exported.
apps/website/src/server/routers/exercises.ts Updated two db.getFirst calls to getFirstFromPg(db.pg, table.pg, opts); semantics and filter arguments are identical.
apps/website/src/server/routers/resources.ts Single db.getFirst call updated to getFirstFromPg(db.pg, table.pg, opts); semantics unchanged.

Sequence Diagram

sequenceDiagram
    participant Caller as exercises.ts / resources.ts
    participant PgAirtableDb as PgAirtableDb (client.ts)
    participant PgQuery as getFirstFromPg (pg-query.ts)
    participant Drizzle as Drizzle ORM / Postgres

    Note over Caller,PgAirtableDb: Before refactor
    Caller->>PgAirtableDb: db.getFirst(table, opts)
    PgAirtableDb->>PgAirtableDb: resolveSortConfig()
    PgAirtableDb->>PgAirtableDb: buildWhereClause()
    PgAirtableDb->>Drizzle: SELECT ... LIMIT 1
    Drizzle-->>Caller: result

    Note over Caller,PgQuery: After refactor
    Caller->>PgQuery: getFirstFromPg(db.pg, table.pg, opts)
    PgQuery->>PgQuery: resolveSortConfig()
    PgQuery->>PgQuery: buildWhereClause()
    PgQuery->>Drizzle: SELECT ... LIMIT 1
    Drizzle-->>Caller: result

    Note over PgAirtableDb,PgQuery: db.getFirst still works (delegates)
    PgAirtableDb->>PgQuery: getFirstFromPg(pgUnrestricted, table.pg, opts)
    PgQuery-->>PgAirtableDb: result
Loading

Reviews (2): Last reviewed commit: "Address AI comments" | Re-trigger Greptile

Comment thread libraries/db/src/lib/pg-query.ts Outdated
Comment thread libraries/db/src/lib/client.test.ts
@Will-Howard Will-Howard temporarily deployed to wh-getfirstpg-2026-06 - bluedot-storybook-preview PR #2620 June 8, 2026 09:01 — with Render Destroyed
@Will-Howard Will-Howard marked this pull request as ready for review June 8, 2026 09:02
@Will-Howard Will-Howard requested a review from marn-in-prod June 8, 2026 11:36
@Will-Howard Will-Howard temporarily deployed to wh-getfirstpg-2026-06 - bluedot-preview PR #2620 June 9, 2026 08:11 — with Render Destroyed
@Will-Howard

Copy link
Copy Markdown
Collaborator Author

@marn-in-prod I would appreciate a retroactive review on this when you get a chance!

@Will-Howard Will-Howard merged commit 3b4c862 into master Jun 9, 2026
7 checks passed
@Will-Howard Will-Howard deleted the wh-getfirstpg-2026-06 branch June 9, 2026 08:56
Will-Howard added a commit that referenced this pull request Jun 10, 2026
Squashed rebase of the original branch onto master: the getFirst refactor
commits merged separately as #2620, and the computed-airtable-fields
library (#2614) is adapted to read the flipped tables.
Will-Howard added a commit that referenced this pull request Jun 10, 2026
Squashed rebase of the original branch onto master: the getFirst refactor
commits merged separately as #2620, and the computed-airtable-fields
library (#2614) is adapted to read the flipped tables.
Will-Howard added a commit that referenced this pull request Jun 10, 2026
Squashed rebase of the original branch onto master: the getFirst refactor
commits merged separately as #2620, and the computed-airtable-fields
library (#2614) is adapted to read the flipped tables.
Will-Howard added a commit that referenced this pull request Jun 11, 2026
Squashed rebase of the original branch onto master: the getFirst refactor
commits merged separately as #2620, and the computed-airtable-fields
library (#2614) is adapted to read the flipped tables.
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.

1 participant