[feat] Add completedAt column to resource_completion#2684
Conversation
📝 WalkthroughWalkthroughA 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
9c0eb32 to
fc1694d
Compare
fc1694d to
9c0eb32
Compare
Greptile SummaryThis PR adds a
Confidence Score: 4/5Safe to merge once the server mutation is updated to write The schema column, test utility, and optimistic UI update are all correctly added, but the apps/website/src/server/routers/resources.ts — the mutation's INSERT and UPDATE paths both need Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant UI as ResourceListItem
participant Cache as React Query Cache
participant Server as resources.saveResourceCompletion
participant DB as PostgreSQL
UI->>Cache: "Optimistic update (completedAt = now)"
UI->>Server: "mutation({ isCompleted, feedback, ... })"
Server->>DB: INSERT/UPDATE (isCompleted, feedback, ...) — completedAt NOT set
DB-->>Server: "row (completedAt = null)"
Server-->>UI: "response (completedAt = null)"
UI->>Cache: Invalidate + refetch
Cache-->>UI: "completedAt = null (optimistic value lost)"
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant UI as ResourceListItem
participant Cache as React Query Cache
participant Server as resources.saveResourceCompletion
participant DB as PostgreSQL
UI->>Cache: "Optimistic update (completedAt = now)"
UI->>Server: "mutation({ isCompleted, feedback, ... })"
Server->>DB: INSERT/UPDATE (isCompleted, feedback, ...) — completedAt NOT set
DB-->>Server: "row (completedAt = null)"
Server-->>UI: "response (completedAt = null)"
UI->>Cache: Invalidate + refetch
Cache-->>UI: "completedAt = null (optimistic value lost)"
Reviews (2): Last reviewed commit: "[feat] Add completedAt column to resourc..." | Re-trigger Greptile |
| // Points at courseBuilderUserTable (the Course-builder-base sync of User) | ||
| createdByUserId: text().array(), | ||
| createdAt: text(), | ||
| completedAt: text(), |
There was a problem hiding this comment.
Type mismatch with
exerciseResponsePgTable.completedAt
The PR description cites parity with exercise responses, but exerciseResponsePgTable.completedAt is declared as text() (line 153), while this new column uses timestamp({ mode: 'string', withTimezone: true }). When Drizzle reads a TIMESTAMPTZ column with mode: 'string', PostgreSQL returns the value in its own format ("2026-01-01 10:00:00+00") rather than the ISO 8601 format that new Date().toISOString() produces ("2026-01-01T10:00:00.000Z"). Future code that inserts ISO strings or treats both completedAt fields interchangeably (e.g. PostHog event shipping) may encounter subtle format differences.
There was a problem hiding this comment.
👍 I did go back to the basic text() type for this reason
9c0eb32 to
8bef584
Compare
8bef584 to
f2d6980
Compare
f2d6980 to
c9e396f
Compare
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libraries/db/src/schema.ts`:
- Line 1520: The addition of the completedAt column to the schema in
libraries/db/src/schema.ts must be split into separate PRs to follow the
required rollout contract for schema changes. Remove all changes related to
consumer code usage (the updates in
apps/website/src/components/courses/ResourceListItem.tsx and
apps/website/src/__tests__/testUtils.ts), keeping only the schema column
addition at line 1520. After this PR is merged, create a second PR that adds the
consumer code changes to use the newly available completedAt column.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4af999d4-c840-47b3-938e-63bcc497c93a
📒 Files selected for processing (3)
apps/website/src/__tests__/testUtils.tsapps/website/src/components/courses/ResourceListItem.tsxlibraries/db/src/schema.ts
| // Points at courseBuilderUserTable (the Course-builder-base sync of User) | ||
| createdByUserId: text().array(), | ||
| createdAt: text(), | ||
| completedAt: text(), |
There was a problem hiding this comment.
Split this column addition from consumer usage into staged PRs.
Line 1520 adds completedAt, but this PR also updates consumer code in apps/website/src/components/courses/ResourceListItem.tsx and apps/website/src/__tests__/testUtils.ts. That breaks the required rollout contract for schema changes and creates deployment-order risk.
As per coding guidelines: “Adding a column requires 2 PRs: (1) add column in libraries/db/src/schema.ts and merge, (2) then PR the code that uses it.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libraries/db/src/schema.ts` at line 1520, The addition of the completedAt
column to the schema in libraries/db/src/schema.ts must be split into separate
PRs to follow the required rollout contract for schema changes. Remove all
changes related to consumer code usage (the updates in
apps/website/src/components/courses/ResourceListItem.tsx and
apps/website/src/__tests__/testUtils.ts), keeping only the schema column
addition at line 1520. After this PR is merged, create a second PR that adds the
consumer code changes to use the newly available completedAt column.
Source: Coding guidelines
Description
For parity with exercise responses, and to allow shipping resource_completed events to PostHog, I'm migrating the
isCompletedfield tocompletedAtIssue
#2679
Developer checklist