Skip to content

Conversation

ericallam
Copy link
Member

No description provided.

Copy link
Contributor

coderabbitai bot commented Jul 31, 2025

Walkthrough

A new module was introduced to centralize the logic for generating Prisma query filter objects for task schedules, based on project and schedule identifiers. Two functions, scheduleUniqWhereClause and scheduleWhereClause, were added to encapsulate this logic. Several files that previously constructed their own where clauses for querying or updating task schedules now import and utilize these helper functions instead. No changes were made to error handling, response structures, or the core business logic in these files.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Complexity is low, as the changes are primarily refactoring to centralize and reuse query logic.
  • The volume of changes spans several files but remains straightforward, with no significant alterations to business logic or data flow.
  • Reviewers should verify correctness of the new helper functions and their integration points.

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/schedules-deduplication-key-api

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🔭 Outside diff range comments (1)
apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts (1)

14-17: Return a proper JSON Response for the 405 branch
Returning a plain object ({ status: 405, body: … }) differs from every other branch that uses json(), so callers receive inconsistent response shapes / headers.

-  if (request.method.toUpperCase() !== "POST") {
-    return { status: 405, body: "Method Not Allowed" };
+  if (request.method.toUpperCase() !== "POST") {
+    return json({ error: "Method Not Allowed" }, { status: 405 });
   }
🧹 Nitpick comments (2)
apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts (2)

35-41: Prefer findUnique + scheduleUniqWhereClause to avoid a full scan
findFirst performs a filter; findUnique leverages the unique index and is both faster and clearer, given the clause is already unique.

-    const existingSchedule = await prisma.taskSchedule.findFirst({
-      where: scheduleWhereClause(
-        authenticationResult.environment.projectId,
-        parsedParams.data.scheduleId
-      ),
-    });
+    const uniqWhere = scheduleUniqWhereClause(
+      authenticationResult.environment.projectId,
+      parsedParams.data.scheduleId
+    );
+
+    const existingSchedule = await prisma.taskSchedule.findUnique({
+      where: uniqWhere,
+    });

48-52: Reuse the same uniqWhere for the update to remove duplication

-    await prisma.taskSchedule.update({
-      where: scheduleUniqWhereClause(
-        authenticationResult.environment.projectId,
-        parsedParams.data.scheduleId
-      ),
+    await prisma.taskSchedule.update({
+      where: uniqWhere,
       data: {
         active: true,
       },
     });
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b153324 and a3fa1fa.

📒 Files selected for processing (6)
  • apps/webapp/app/models/schedules.server.ts (1 hunks)
  • apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts (2 hunks)
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts (2 hunks)
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts (2 hunks)
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts (2 hunks)
  • apps/webapp/app/v3/services/upsertTaskSchedule.server.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations

Files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts
  • apps/webapp/app/v3/services/upsertTaskSchedule.server.ts
  • apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts
  • apps/webapp/app/models/schedules.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

We use zod a lot in packages/core and in the webapp

Files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts
  • apps/webapp/app/v3/services/upsertTaskSchedule.server.ts
  • apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts
  • apps/webapp/app/models/schedules.server.ts
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: In the webapp, all environment variables must be accessed through the env export of env.server.ts, instead of directly accessing process.env.
When importing from @trigger.dev/core in the webapp, never import from the root @trigger.dev/core path; always use one of the subpath exports as defined in the package's package.json.

Files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts
  • apps/webapp/app/v3/services/upsertTaskSchedule.server.ts
  • apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts
  • apps/webapp/app/models/schedules.server.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using idempotency, use the `idempotencyKeys` API and `idempotencyKey`/`idempotencyKeyTTL` options as shown.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing scheduled (cron) tasks, use `schedules.task` from `@trigger.dev/sdk/v3` and follow the shown patterns.
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when implementing scheduled (cron) tasks, use `schedule...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing scheduled (cron) tasks, use `schedules.task` from `@trigger.dev/sdk/v3` and follow the shown patterns.

Applied to files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts
  • apps/webapp/app/v3/services/upsertTaskSchedule.server.ts
  • apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts
  • apps/webapp/app/models/schedules.server.ts
📚 Learning: applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : we use zod a lot in packages/core and in the ...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T17:49:24.468Z
Learning: Applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : We use zod a lot in packages/core and in the webapp

Applied to files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts
📚 Learning: applies to internal-packages/database/**/*.{ts,tsx} : we use prisma in internal-packages/database fo...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T17:49:24.468Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : We use prisma in internal-packages/database for our database interactions using PostgreSQL

Applied to files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts
  • apps/webapp/app/v3/services/upsertTaskSchedule.server.ts
  • apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts
  • apps/webapp/app/models/schedules.server.ts
📚 Learning: applies to apps/webapp/app/v3/presenters/**/*.server.ts : favor the use of 'presenters' to move comp...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-07-18T17:49:47.180Z
Learning: Applies to apps/webapp/app/v3/presenters/**/*.server.ts : Favor the use of 'presenters' to move complex loader code into a class, as seen in `app/v3/presenters/**/*.server.ts`.

Applied to files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts
  • apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts
  • apps/webapp/app/models/schedules.server.ts
📚 Learning: in apps/webapp/app/services/runsrepository.server.ts, the in-memory status filtering after fetching ...
Learnt from: matt-aitken
PR: triggerdotdev/trigger.dev#2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025-07-12T18:06:04.133Z
Learning: In apps/webapp/app/services/runsRepository.server.ts, the in-memory status filtering after fetching runs from Prisma is intentionally used as a workaround for ClickHouse data delays. This approach is acceptable because the result set is limited to a maximum of 100 runs due to pagination, making the performance impact negligible.

Applied to files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts
  • apps/webapp/app/v3/services/upsertTaskSchedule.server.ts
  • apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts
  • apps/webapp/app/models/schedules.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when implementing schema tasks, use `schematask` from `...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing schema tasks, use `schemaTask` from `@trigger.dev/sdk/v3` and validate payloads as shown.

Applied to files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
  • apps/webapp/app/v3/services/upsertTaskSchedule.server.ts
  • apps/webapp/app/models/schedules.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : you must use `@trigger.dev/sdk/v3` when writing trigger...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST use `@trigger.dev/sdk/v3` when writing Trigger.dev tasks.

Applied to files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when triggering a task from backend code, use `tasks.tr...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from backend code, use `tasks.trigger`, `tasks.batchTrigger`, or `tasks.triggerAndPoll` as shown in the examples.

Applied to files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : always generate trigger.dev tasks using the `task` func...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : ALWAYS generate Trigger.dev tasks using the `task` function from `@trigger.dev/sdk/v3` and export them as shown in the correct pattern.

Applied to files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
  • apps/webapp/app/models/schedules.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when using retry, queue, machine, or maxduration option...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using retry, queue, machine, or maxDuration options, configure them as shown in the examples for Trigger.dev tasks.

Applied to files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : you must never use `client.definejob` in trigger.dev ta...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST NEVER use `client.defineJob` in Trigger.dev task files.

Applied to files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : never generate deprecated code patterns using `client.d...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : NEVER generate deprecated code patterns using `client.defineJob` and related deprecated APIs, as shown in the prohibited code block.

Applied to files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : the `run` function contains your task logic in trigger....
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : The `run` function contains your task logic in Trigger.dev tasks.

Applied to files:

  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts
📚 Learning: the `objecttosearchparams` function in `apps/webapp/app/utils/searchparams.ts` is used to generate u...
Learnt from: matt-aitken
PR: triggerdotdev/trigger.dev#2264
File: apps/webapp/app/utils/searchParams.ts:16-18
Timestamp: 2025-07-12T18:00:06.163Z
Learning: The `objectToSearchParams` function in `apps/webapp/app/utils/searchParams.ts` is used to generate URL parameters from objects and is separate from code that parses incoming search parameters. Changes to this function only affect places where it's used to create URLs, not places that parse search parameters from external sources.

Applied to files:

  • apps/webapp/app/v3/services/upsertTaskSchedule.server.ts
  • apps/webapp/app/models/schedules.server.ts
📚 Learning: applies to apps/webapp/app/services/**/*.server.ts : for testable services, separate service logic a...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-07-18T17:49:47.180Z
Learning: Applies to apps/webapp/app/services/**/*.server.ts : For testable services, separate service logic and configuration, as exemplified by `realtimeClient.server.ts` (service) and `realtimeClientGlobal.server.ts` (configuration).

Applied to files:

  • apps/webapp/app/v3/services/upsertTaskSchedule.server.ts
  • apps/webapp/app/models/schedules.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : each task needs a unique id within your project....
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Each task needs a unique ID within your project.

Applied to files:

  • apps/webapp/app/models/schedules.server.ts
🧬 Code Graph Analysis (4)
apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts (1)
apps/webapp/app/models/schedules.server.ts (2)
  • scheduleWhereClause (22-37)
  • scheduleUniqWhereClause (3-20)
apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts (1)
apps/webapp/app/models/schedules.server.ts (1)
  • scheduleUniqWhereClause (3-20)
apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts (1)
apps/webapp/app/models/schedules.server.ts (2)
  • scheduleWhereClause (22-37)
  • scheduleUniqWhereClause (3-20)
apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts (1)
apps/webapp/app/models/schedules.server.ts (1)
  • scheduleWhereClause (22-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (13)
apps/webapp/app/models/schedules.server.ts (2)

3-20: LGTM! Well-designed helper function for unique queries.

The scheduleUniqWhereClause function correctly handles both friendly IDs (starting with "sched_") and deduplication keys using the appropriate Prisma unique constraint patterns. The composite unique key projectId_deduplicationKey is properly structured for the deduplication key case.


22-37: LGTM! Consistent design with the unique variant.

The scheduleWhereClause function mirrors the logic of scheduleUniqWhereClause but returns a standard WhereInput instead of WhereUniqueInput, making it suitable for findFirst and similar operations. The consistent branching logic ensures both functions handle the same ID formats identically.

apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts (2)

6-6: LGTM! Proper import of the helper function.

The import correctly brings in the scheduleUniqWhereClause function needed for the delete operation.


40-43: LGTM! Correct usage of the helper function for delete operation.

The scheduleUniqWhereClause helper is appropriately used here since Prisma's delete operation requires a unique where clause. The parameters are correctly passed: projectId from the authenticated environment and scheduleId from the parsed params.

apps/webapp/app/v3/services/upsertTaskSchedule.server.ts (2)

10-10: LGTM! Proper import of the helper function.

The import correctly brings in the scheduleWhereClause function for use in the service.


41-41: LGTM! Centralized query logic improves consistency.

The service now uses the centralized scheduleWhereClause helper function, which will correctly handle both friendly IDs and deduplication keys based on the ID format. This ensures consistent behavior across the application when querying schedules.

apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts (2)

7-7: LGTM! Proper import of the helper function.

The import correctly brings in the scheduleWhereClause function for use in the presenter.


67-67: LGTM! Centralized query logic supports dual ID format.

The presenter now uses the scheduleWhereClause helper function, which enables it to handle both friendly IDs (starting with "sched_") and deduplication keys. This expands the presenter's capability beyond just friendly IDs, aligning with the PR objective to accept both deduplicationKeys and schedule IDs.

apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts (3)

5-5: LGTM! Proper imports of both helper functions.

The imports correctly bring in both scheduleUniqWhereClause and scheduleWhereClause functions needed for the different Prisma operations in this route.


37-40: LGTM! Correct usage of standard where clause for findFirst.

The scheduleWhereClause helper is appropriately used for the findFirst operation, which accepts a standard where input rather than requiring a unique constraint.


48-51: LGTM! Correct usage of unique where clause for update.

The scheduleUniqWhereClause helper is appropriately used for the update operation, which requires a unique where clause. This ensures the update operation can correctly identify the specific schedule to modify.

apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts (2)

5-5: Importing the shared helpers is spot-on
Re-using scheduleUniqWhereClause / scheduleWhereClause keeps the route logic DRY and aligned with the rest of the codebase.


57-63: Verify presenter lookup works for deduplicationKey routes
If the caller supplied a plain deduplicationKey (doesn’t start with sched_), we pass it as friendlyId, which may cause ViewSchedulePresenter to miss the record and return 404 even after a successful update.

Please confirm ViewSchedulePresenter.call() accepts both identifier shapes; otherwise, consider forwarding both friendlyId and deduplicationKey, or add a helper mirroring scheduleWhereClause.

@ericallam ericallam merged commit 134942b into main Jul 31, 2025
33 checks passed
@ericallam ericallam deleted the feat/schedules-deduplication-key-api branch July 31, 2025 13:12
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