Skip to content

refactor: migrate pool.query() calls in push_subscriptions to table methods#275

Merged
hoiekim merged 1 commit intohoiekim:mainfrom
moltboie:refactor-pool-query-push-subscriptions
Mar 25, 2026
Merged

refactor: migrate pool.query() calls in push_subscriptions to table methods#275
hoiekim merged 1 commit intohoiekim:mainfrom
moltboie:refactor-pool-query-push-subscriptions

Conversation

@moltboie
Copy link
Copy Markdown
Contributor

Summary

Completes the pool.query() migration started in PR #248 (sessions + spam_allowlists). This PR covers the remaining repository file with direct pool.query() calls.

push_subscriptions.ts (2 pool.query calls removed):

  • Convert pushSubscriptionsTable from createTable() to a named PushSubscriptionsTable class
  • Add getByUserIds(userIds: string[]) — encapsulates SELECT * WHERE user_id IN (...)
  • Add deleteOlderThan(cutoff: string) — encapsulates DELETE WHERE updated < $1
  • cleanSubscriptions() now delegates to pushSubscriptionsTable.deleteOlderThan()
  • getSubscriptions() now delegates to pushSubscriptionsTable.getByUserIds()
  • Direct pool import removed from the repository file

Motivation

Per DEVELOPMENT.md Table Class Methods convention: all DB operations in repository files should go through table class methods. Direct pool.query() calls in repositories are migration debt.

No Behavior Changes

Same SQL queries, same logic. TypeScript compiles cleanly.

Closes #244 (together with PR #248)

@moltboie
Copy link
Copy Markdown
Contributor Author

Self-review: pool.query() migration for push_subscriptions.

Pattern consistency:

getByUserIds review:

  • Uses positional params ($1, $2, ...) built from array length — safe, no SQL injection
  • Returns PushSubscriptionModel[] (typed, validated by Model constructor)
  • Empty array guard returns early, consistent with base class queryByIds pattern

deleteOlderThan review:

  • Parameterized query with single $1 cutoff — safe
  • Returns count (rowCount ?? 0) consistent with base class deleteWhere
  • Repository caller builds cutoff via new Date(...).toISOString() — unchanged

getSubscriptions cleanup:

  • Removed manual row casting (row.user_id as string, etc.) — now uses typed model fields
  • row.last_notified is typed as string | null; new Date(null) would be Invalid Date but existing logic was the same (passing through)

TypeScript clean. No behavior changes.

@moltboie
Copy link
Copy Markdown
Contributor Author

Self-Review

Discussion thread status:

Checked:

  • Migration scope: push_subscriptions.ts — 2 direct pool.query() calls converted to Table class methods. Consistent with DEVELOPMENT.md pattern.
  • PushSubscriptionsTable extends Table: Table class handles pool internally — correct abstraction. Repository file now delegates to table instance.
  • Logic equivalence: SQL queries unchanged in semantics; just relocated to the correct abstraction layer.
  • Export chain: Table instance exported correctly through barrels.
  • CI: build + test both passing ✅

E2E Testing:

  • Push subscription functionality (subscribe/unsubscribe) should work identically. Refactor-only.

Issues found:

  • None

Confidence: High

…ethods

Per DEVELOPMENT.md convention, DB operations in repository files should go
through Table class methods rather than calling pool.query() directly.

push_subscriptions.ts (2 pool.query calls removed):
- Convert pushSubscriptionsTable from createTable() to a PushSubscriptionsTable class
- Add getByUserIds() method for SELECT WHERE user_id IN (...) queries
- Add deleteOlderThan() method for DELETE WHERE updated < cutoff
- cleanSubscriptions() now delegates to pushSubscriptionsTable.deleteOlderThan()
- getSubscriptions() now delegates to pushSubscriptionsTable.getByUserIds()
- Removed direct pool import from repository file

No behavior changes; TypeScript compiles cleanly.

Part of hoiekim#244
@moltboie moltboie force-pushed the refactor-pool-query-push-subscriptions branch from fc257c0 to 390cec8 Compare March 21, 2026 16:40
@hoiekim hoiekim merged commit 4667a0e into hoiekim:main Mar 25, 2026
2 checks passed
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.

refactor: migrate legacy pool.query() calls in repositories to table methods

2 participants