fix: ensure task list and test action use correct dev env #1845
Conversation
|
WalkthroughThis pull request updates several components in the web application to streamline task and test handling. Method signatures are simplified to require only essential parameters (e.g., environmentId and projectId) while removing extraneous ones. New helper functions are introduced to retrieve projects and environments by slug, with added error handling for missing resources. Additionally, the SQL queries and return values in the task presenter have been modified, and outdated navigation-related logic has been removed. Updates in the test task service now directly utilize the authenticated environment for triggering tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Loader
participant ProjectService as findProjectBySlug
participant EnvService as findEnvironmentBySlug
participant Presenter as TaskListPresenter
Client->>Loader: Request task list
Loader->>ProjectService: findProjectBySlug(organizationSlug, projectParam, userId)
ProjectService-->>Loader: Return project / error 404
Loader->>EnvService: findEnvironmentBySlug(projectId, envParam, userId)
EnvService-->>Loader: Return environment / error 404
Loader->>Presenter: call({ environmentId, projectId })
Presenter-->>Loader: Return tasks, activity, runningStats, durations
Loader-->>Client: Response with task data
sequenceDiagram
participant Caller
participant TestTaskService
participant TriggerTaskService
Caller->>TestTaskService: call(environment, testTaskData)
TestTaskService->>TriggerTaskService: call(environment, testTaskData)
TriggerTaskService-->>TestTaskService: Test run result
TestTaskService-->>Caller: Return result
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/webapp/app/presenters/v3/TaskListPresenter.server.ts(4 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam._index/route.tsx(2 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test/route.tsx(0 hunks)apps/webapp/app/v3/services/testTask.server.ts(2 hunks)
💤 Files with no reviewable changes (1)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test/route.tsx
🧰 Additional context used
🧬 Code Definitions (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx (1)
apps/webapp/app/v3/services/testTask.server.ts (1)
TestTaskService(7-50)
apps/webapp/app/presenters/v3/TaskListPresenter.server.ts (1)
internal-packages/run-engine/src/engine/index.ts (1)
environmentId(1324-1326)
apps/webapp/app/v3/services/testTask.server.ts (2)
apps/webapp/app/v3/services/batchTriggerV3.server.ts (1)
environment(318-413)apps/webapp/app/v3/services/triggerTask.server.ts (1)
TriggerTaskService(37-92)
🪛 Biome (1.9.4)
apps/webapp/app/v3/services/testTask.server.ts
[error] 13-19: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (13)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx (3)
107-116: Improved error handling for project validation.The added code properly validates the project's existence by using
findProjectBySlugand returns a clear error message if the project is not found. This prevents attempts to run tests against non-existent projects.
112-116: Added environment validation for test execution.Great addition of environment validation before proceeding with test execution. This ensures tests only run in valid environments and provides clear error messages to users.
120-120: Updated testService.call parameter to use environment directly.This change properly aligns with the updated
TestTaskService.callmethod signature, which now accepts anAuthenticatedEnvironmentobject directly instead of auserId. This eliminates the need to look up the environment twice and is more efficient.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam._index/route.tsx (3)
73-74: Imported helper functions for project and environment validation.The addition of these imports provides access to the helper functions needed for proper validation of projects and environments by slug.
109-123: Added robust validation for project and environment.This is a critical improvement that ensures proper validation of both project and environment before retrieving tasks. The code now throws appropriate 404 responses with helpful error messages when resources don't exist, enhancing both security and user experience.
127-130: Updated TaskListPresenter call parameters.The parameter structure has been simplified to only pass the essential
environmentIdandprojectIdvalues rather than retrieving them again inside the presenter. This matches the updated method signature inTaskListPresenterand reduces redundant lookups.apps/webapp/app/v3/services/testTask.server.ts (3)
2-2: Added import for AuthenticatedEnvironment type.This import supports the updated method signature for the
callmethod.
8-8: Updated method signature to accept environment directly.The
callmethod now accepts anAuthenticatedEnvironmentobject directly instead of auserId. This simplifies the code by eliminating the need to look up the environment separately and aligns with how the environment is handled across the application.
36-36: Updated environment parameter in SCHEDULED case.The environment parameter is now passed directly to the triggerTaskService in the SCHEDULED case as well, matching the pattern used in the STANDARD case.
apps/webapp/app/presenters/v3/TaskListPresenter.server.ts (4)
26-26: Simplified method signature for TaskListPresenter.call.The method now only requires the essential
environmentIdandprojectIdparameters, removing the need foruserId,organizationSlug,projectSlug, andenvironmentSlug. This streamlines the code and removes redundant parameter passing.
41-42: Updated SQL queries to use environmentId directly.The SQL queries now reference the
environmentIdparameter directly instead of accessing it through an environment object. This simplifies the code and aligns with the updated method signature.Also applies to: 47-48
57-61: Updated helper method calls with direct parameter passing.The calls to internal helper methods now pass
projectIdandenvironmentIddirectly, aligning with the simplified parameter structure throughout the codebase.Also applies to: 63-67, 69-73
75-75: Simplified return statement.The return statement has been simplified to exclude the
environmentobject, focusing only on the task-related data:tasks,activity,runningStats, anddurations. This aligns with the overall approach of passing only necessary data.
Summary by CodeRabbit
Bug Fixes
Refactor