Added default time period to Runs, Waitpoint and Batch list pages. Improved dev presence #1832
Conversation
|
WalkthroughThis pull request introduces several cross-cutting changes. A new “checking connection” icon is added, and DevPresence-related logic is refactored across client and server components. Date-based filtering across batches, runs, and waitpoints is replaced with time-based filtering using new helper functions. Updates include improved error handling (e.g., for run environment mismatches), revised UI components with conditional styling and updated import declarations, and changes to scheduled task management including schema updates. The modifications span front-end icons/components, filtering logic, server-side presence management, and database/service adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Loader
participant DevPresence
Client->>Loader: Establish SSE connection
Loader->>DevPresence: setConnected(environmentId, ttl)
DevPresence-->>Loader: Return connection status
Loader->>Client: Stream initial presence data
loop Periodically
Loader->>DevPresence: Check connection status
DevPresence-->>Loader: Provide updated status
Loader->>Client: Stream updated presence data
end
Loader->>Client: Stream cleanup on disconnect
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 (
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a default 7-day time period filter to listing pages for Runs, Waitpoint tokens, and Batches while also improving tag filtering performance and dev presence reliability. Key changes include:
- Introducing a default time filter and refactoring time range logic across several presenters and UI components.
- Replacing “contains” with “startsWith” for run tag filtering, and adding an environment mismatch error for run pages.
- Overhauling the dev presence functionality by removing SSE stream code and implementing a Redis-backed approach.
Reviewed Changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts | Uses newly exposed lastRunTriggeredAt instead of a raw query for last run time. |
| apps/webapp/app/presenters/v3/RunTagListPresenter.server.ts | Updates tag filtering from an array-based "contains" to a single "startsWith" filter. |
| apps/webapp/app/presenters/v3/RunPresenter.server.ts | Adds a RunEnvironmentMismatchError to enforce proper environment routing. |
| apps/webapp/app/presenters/v3/RunListPresenter.server.ts | Refactors query parameters using a timeFilters helper and integrates a fallback check for empty runs. |
| apps/webapp/app/presenters/v3/DevPresence.server.ts | Implements a new Redis-backed dev presence monitor. |
| apps/webapp/app/env.server.ts | Adjusts dev presence TTL and poll intervals. |
| apps/webapp/app/components/runs/v3/* & BatchFilters.tsx | Refactors filtering components to use the new TimeFilter and removes obsolete date range components. |
| apps/webapp/app/components/primitives/* & navigation components | Minor style/variant updates and improved connectivity UI with new checking state. |
Comments suppressed due to low confidence (3)
apps/webapp/app/components/runs/v3/TaskRunsTable.tsx:291
- Make sure that the 'run.environment' property is consistently defined across all run objects since it replaces the previously used environment variable. This could lead to runtime errors if undefined.
const path = v3RunSpanPath(organization, project, run.environment, run, {
apps/webapp/app/env.server.ts:578
- The TTL for dev presence has been reduced from 30 seconds to 5 seconds; verify that this new value is intentional and will not adversely affect connection stability in various environments.
DEV_PRESENCE_TTL_MS: z.coerce.number().int().default(5_000),
apps/webapp/app/components/runs/v3/SharedFilters.tsx:99
- [nitpick] Consider handling an invalid default period more gracefully rather than throwing an error immediately at startup, which might interrupt the UI unexpectedly.
if (!defaultPeriodMs) { throw new Error("Invalid default period"); }
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a default 7‑day time period filter for list views (Runs, Waitpoint tokens, Batches) to improve query performance and also refines tag filtering, run environment redirection, and dev presence reliability. Key changes include:
- Adding a new default time period filter for list pages and centralizing time filtering logic.
- Improving dev presence tracking with Redis and SSE alongside UI updates for connection status.
- Updating run tag filtering to use a case‑insensitive “startsWith” match and refining error handling for run environment mismatches.
Reviewed Changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts | Added selection of "lastRunTriggeredAt" replacing computed latest run timestamp. |
| apps/webapp/app/presenters/v3/RunTagListPresenter.server.ts | Changed tag filter parameter from "names" to "name" and switched to startsWith matching. |
| apps/webapp/app/presenters/v3/RunPresenter.server.ts | Added run environment mismatch error handling for proper redirection. |
| apps/webapp/app/presenters/v3/RunListPresenter.server.ts | Reworked time filter processing and query raw usage for runs. |
| apps/webapp/app/presenters/v3/DevPresence.server.ts | Introduced Redis‑backed dev presence tracking. |
| apps/webapp/app/presenters/v3/BatchListPresenter.server.ts | Updated time filtering and added fallback check for batches. |
| apps/webapp/app/env.server.ts | Updated dev presence TTL and polling interval environment variables. |
| Various UI components (WaitpointTokenFilters, SharedFilters, RunFilters, BatchFilters, etc.) | Integrated and streamlined the new time filter UI and removed legacy date range filters. |
| Components like Switch, DateField, AppliedFilter, and SideMenu | Adjusted styling and connection indication to align with updated design and state logic. |
| DevPresence and ConnectionIcons | Added a new "CheckingConnectionIcon" and refined connection state handling. |
Comments suppressed due to low confidence (6)
apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts:165
- Ensure that the 'lastRunTriggeredAt' field exists in the schedule records and reliably reflects the most recent run trigger timestamp. Verify that database migrations and indexes are updated accordingly.
+ lastRunTriggeredAt: true,
apps/webapp/app/presenters/v3/RunListPresenter.server.ts:195
- Confirm that the 'parse' function (likely from 'parse-duration') is imported and correctly converts the period string to milliseconds, ensuring consistency with the new time filter logic.
const periodMs = time.period ? parse(time.period) : undefined;
apps/webapp/app/components/runs/v3/SharedFilters.tsx:100
- [nitpick] Consider providing a fallback or more graceful error handling for an invalid default period rather than throwing an error abruptly.
if (!defaultPeriodMs) { throw new Error("Invalid default period"); }
apps/webapp/app/components/runs/v3/TaskRunsTable.tsx:291
- Verify that 'run.environment' is always defined and correctly populated, as this change from using a higher‑level 'environment' variable may affect path resolution.
const path = v3RunSpanPath(organization, project, run.environment, run, {
apps/webapp/app/components/navigation/SideMenu.tsx:540
- Confirm that handling an undefined 'isConnected' state by displaying a 'CheckingConnectionIcon' and appropriate tooltip text is consistent and clear across all connection-related UI components.
isConnected === undefined ? (
apps/webapp/app/components/primitives/DateField.tsx:13
- [nitpick] Ensure that reducing the font size to 'text-xs' for the small variant does not negatively impact readability or overall UI consistency in different contexts.
fieldStyles: "h-5 text-xs rounded-sm px-0.5",
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
apps/webapp/app/components/primitives/Select.tsx (3)
2-2: Good TypeScript practice improvement.Adding the
typekeyword to the import statement clarifies thatSelectPropsis being imported as a type rather than a value, which aligns with TypeScript best practices and helps with tree-shaking.
7-7: Good TypeScript practice improvement.Adding the
typekeyword to the import statement forShortcutDefinitionexplicitly indicates it's a type-only import, consistent with modern TypeScript practices.
11-11: Good TypeScript practice improvement.Adding the
typekeyword to the import statement forMatchSorterOptionsmakes it clear that this is a type-only import, following TypeScript's recommended import patterns.packages/cli-v3/src/dev/devSupervisor.ts (1)
334-337: Modified presence connection handlingThe
presenceConnectionmethod is now called withoutawait, indicating that the method has been changed to return anEventSourcedirectly rather than a Promise that resolves to an EventSource. This aligns with the PR's objective to improve dev presence reliability.Ensure that any potential errors in establishing the connection are properly handled. Consider adding error handling in the event listener for connection failures:
const eventSource = this.options.client.dev.presenceConnection(); + +eventSource.addEventListener("error", (event: any) => { + logger.debug("[DevSupervisor] Presence connection error", { event }); +});apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx (1)
104-105: Check for unused destructured fields
successis destructured but not referenced later. If not needed, consider removing it to reduce clutter.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (1)
99-99: Unused import
redirectWithErrorMessageappears imported but not utilized. Removing it would keep the file clean.- import { redirectWithErrorMessage } from "~/models/message.server";apps/webapp/app/presenters/v3/DevPresence.server.ts (1)
9-11: Consider handling potential Redis connection errors in the constructor.
Currently, if Redis initialization fails, the application might not handle it gracefully. You could add a reconnection strategy or error handling if necessary.apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts (1)
248-260: Efficiently determininghasAnyTokens.
You make an extra database call when the first page is empty, ensuring you accurately classify whether tokens exist at all. While correct, this may be a performance concern in large tables. You might unify these queries into a single conditional or leverage a lightweight metadata caching approach.Also applies to: 282-282
apps/webapp/app/components/navigation/SideMenu.tsx (1)
89-89: Added Spinner component import.The Spinner component is imported but not used in the modified code. Consider whether this is needed.
If the Spinner component isn't used in this file, consider removing this import to keep the imports clean.
apps/webapp/app/routes/engine.v1.dev.presence.ts (1)
37-37: Empty cleanup function may be unnecessaryThe cleanup function is now empty. Consider whether this function is still needed or if it can be removed entirely.
If the function is required by the SSE loader contract but doesn't need to perform any actions, adding a comment explaining why would improve clarity:
- cleanup: async () => {}, + cleanup: async () => { + // No cleanup needed with the new devPresence system + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
apps/webapp/app/assets/icons/ConnectionIcons.tsx(1 hunks)apps/webapp/app/components/DevPresence.tsx(2 hunks)apps/webapp/app/components/navigation/SideMenu.tsx(4 hunks)apps/webapp/app/components/primitives/AppliedFilter.tsx(1 hunks)apps/webapp/app/components/primitives/DateField.tsx(1 hunks)apps/webapp/app/components/primitives/Select.tsx(1 hunks)apps/webapp/app/components/primitives/Switch.tsx(2 hunks)apps/webapp/app/components/runs/v3/BatchFilters.tsx(3 hunks)apps/webapp/app/components/runs/v3/RunFilters.tsx(6 hunks)apps/webapp/app/components/runs/v3/SharedFilters.tsx(2 hunks)apps/webapp/app/components/runs/v3/TaskRunsTable.tsx(1 hunks)apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx(2 hunks)apps/webapp/app/env.server.ts(1 hunks)apps/webapp/app/presenters/v3/BatchListPresenter.server.ts(6 hunks)apps/webapp/app/presenters/v3/DevPresence.server.ts(1 hunks)apps/webapp/app/presenters/v3/DevPresenceStream.server.ts(0 hunks)apps/webapp/app/presenters/v3/RunListPresenter.server.ts(6 hunks)apps/webapp/app/presenters/v3/RunPresenter.server.ts(4 hunks)apps/webapp/app/presenters/v3/RunTagListPresenter.server.ts(2 hunks)apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts(2 hunks)apps/webapp/app/presenters/v3/WaitpointTagListPresenter.server.ts(2 hunks)apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts(5 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.batches/route.tsx(4 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx(4 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx(3 hunks)apps/webapp/app/routes/engine.v1.dev.presence.ts(2 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.dev.presence.tsx(1 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dev.presence.tsx(0 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tags.ts(1 hunks)apps/webapp/app/routes/resources.projects.$projectParam.runs.tags.tsx(1 hunks)apps/webapp/app/utils/pathBuilder.ts(1 hunks)apps/webapp/app/v3/services/triggerScheduledTask.server.ts(4 hunks)internal-packages/database/prisma/migrations/20250327181650_add_last_run_triggered_at_to_task_schedule/migration.sql(1 hunks)internal-packages/database/prisma/schema.prisma(1 hunks)packages/cli-v3/src/apiClient.ts(2 hunks)packages/cli-v3/src/dev/devSupervisor.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/webapp/app/presenters/v3/DevPresenceStream.server.ts
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dev.presence.tsx
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
🔇 Additional comments (98)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tags.ts (1)
34-34: Updated API parameter structure for WaitpointTagListPresenter.The change from passing an array of names to a single name parameter aligns with the modified presenter API and simplifies the filtering approach. This improves consistency across the codebase and potentially enhances query performance.
apps/webapp/app/routes/resources.projects.$projectParam.runs.tags.tsx (1)
29-29: Updated API parameter structure for RunTagListPresenter.The change from passing an array of names to a single name parameter aligns with the presenter API modifications and simplifies the filtering approach. This is consistent with the same pattern applied to WaitpointTagListPresenter.
apps/webapp/app/presenters/v3/WaitpointTagListPresenter.server.ts (4)
5-5: Simplified tag filtering model.Switching from an array of names to a single name parameter simplifies the API and matches the UI interaction pattern where users typically filter by one tag at a time.
19-19: Updated method parameter to reflect type change.Parameter destructuring properly updated to match the type definition change.
23-23: Improved filter validation.The filter validation now correctly checks if the name parameter exists and isn't just whitespace, which is a more robust approach.
28-34: Optimized query filtering with startsWith.The filtering now uses
startsWithwith case insensitivity instead of potential multiple exact matches. This change:
- Aligns with the PR objective to improve performance through better indexing
- Provides a more user-friendly filtering experience
- Simplifies the query logic
apps/webapp/app/presenters/v3/RunTagListPresenter.server.ts (4)
7-7: Simplified tag filtering model.Replacing multiple filter arrays (
namesandenvironments) with a singlenameparameter streamlines the API and aligns with the UI filtering pattern, where users typically search for one tag at a time.
22-22: Updated method parameter to reflect type definition changes.Parameter destructuring properly updated to match the simplified type definition.
26-26: Improved filter validation.The filter validation now correctly checks if the name parameter exists and isn't just whitespace, which is a more robust approach.
31-35: Optimized query filtering with startsWith.The filtering logic has been improved by:
- Using
startsWithinstead ofcontainsfor better index utilization- Implementing case-insensitive matching for a better user experience
- Simplifying the query structure
This directly addresses the PR objective to optimize tag filtering for runs to improve speed.
internal-packages/database/prisma/migrations/20250327181650_add_last_run_triggered_at_to_task_schedule/migration.sql (1)
1-3: Good addition for tracking scheduled task execution.Adding the
lastRunTriggeredAttimestamp column to theTaskScheduletable supports the PR objective to ensure "development schedules will now only trigger if the CLI is connected." This will track when a task was last triggered, providing valuable metadata for controlling scheduled task execution.apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (1)
291-291: Good fix for environment redirection issue.Using
run.environmentinstead of a globalenvironmentvariable ensures each run's path is constructed with its own specific environment. This directly addresses the PR objective to fix "incorrect redirection when accessing a run page with the wrong environment in the URL."apps/webapp/app/utils/pathBuilder.ts (1)
245-251: Good addition for flexible run redirects.The new
v3RunRedirectPathfunction provides a redirection path without specifying an environment, which complements the fix for environment-specific redirections. This allows the application to handle redirects to runs more flexibly, working in tandem with the change inTaskRunsTable.tsxthat now uses each run's own environment.apps/webapp/app/assets/icons/ConnectionIcons.tsx (1)
50-70: New icon component for checking connection state looks good.This new
CheckingConnectionIconcomponent follows the same structure and pattern as the existing connection icon components, providing a visual indicator for the "checking" state when connection status is being determined.internal-packages/database/prisma/schema.prisma (1)
2921-2922: Good addition of timestamp for schedule tracking.Adding the
lastRunTriggeredAtfield to theTaskSchedulemodel will help track when schedules were last triggered, supporting the requirement that "development schedules will now only trigger if the CLI is connected".apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx (1)
204-204: Improved conditional check for empty runs list.Changing the condition from checking for filters (
!list.hasFilters) to checking for actual run data (!list.hasAnyRuns) is more accurate, especially with the new default time period filter that's been introduced.apps/webapp/app/components/primitives/DateField.tsx (2)
13-13: Reduced text size for better UI density.Changing from
text-smtotext-xsfor the small variant helps create a more compact date field, which is beneficial for filtering interfaces.
18-18: Consistent text size reduction for medium variant.Similarly reducing the medium variant from
text-basetotext-smcreates a more consistent appearance across the application while maintaining readability.apps/webapp/app/components/primitives/AppliedFilter.tsx (1)
37-43: Improved styling for non-removable filtersThe component now correctly adds right padding (
pr-2) when a filter is not removable, which improves visual consistency when the remove button is absent.apps/webapp/app/components/primitives/Switch.tsx (3)
6-6: Type import clarificationUsing
typefor importingShortcutDefinitionis a good practice as it ensures the type is only used for type checking and not at runtime.
8-14: Extracted small switch styling to constantExtracting the small switch styling into a reusable constant improves code organization and maintainability.
23-32: Added new tertiary/small switch variantThe new
tertiary/smallvariant enhances the switch component with specific styling for tertiary contexts while reusing the base small switch properties. This is a good pattern for maintaining consistency while allowing customization.apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts (2)
165-165: Added lastRunTriggeredAt to schedule selectionIncluding the
lastRunTriggeredAtfield in the selection allows direct access to the last run timestamp without needing separate queries.
226-226: Simplified last run timestamp assignmentThe code now directly uses the
lastRunTriggeredAtfield from the schedule record instead of potentially requiring a separate query or calculation. This simplification aligns with the PR objective of improving performance.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.batches/route.tsx (2)
102-103: Destructuring looks good
The inclusion ofhasAnyBatchesfor clarity is beneficial and consistent with usage below. No issues identified here.
121-125: Simplified conditional check
Switching from multiple conditions to a single!hasAnyBatchescheck makes the conditional logic more readable. Ensure that the new check covers all scenarios previously handled by!hasFilters && batches.length === 0.apps/webapp/app/env.server.ts (1)
577-579: Verify performance implications
A 5s TTL with a 1s poll interval may trigger frequent redis updates or SSE traffic. Consider confirming that these defaults won't cause performance or scaling issues under heavy usage.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx (2)
10-10: Refined import statement
Changing to a single import forCopyableTextis fine. No issues identified.
126-126: Cleaner “no tokens” check
Using!hasAnyTokenssimplifies the condition over checking both filters and token length. This is a neat improvement.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (6)
21-22: IntroducingtryCatch
AdoptingtryCatchfor async calls can make error handling more explicit. Looks good.
81-81: Additional import for mismatch error
ImportingRunEnvironmentMismatchErroris appropriate for the custom handling below. No concerns.
93-93: Importingv3RunRedirectPath
v3RunRedirectPathis correctly utilized for environment mismatch redirection.
100-100: Importingredirect
redirectfromremix-typedjsonis used in environment mismatch handling and is functioning as intended.
140-150: Non-throwing async call withtryCatch
This approach neatly separates the error and result, enhancing clarity. No functional issues found.
151-163: Custom error handling
CatchingRunEnvironmentMismatchErrorand performing aredirectis a clear pattern for environment mismatch logic. Confirm whether any additional cleanup or user message is needed.apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx (2)
39-39: Import looks consistent with refactored time-based filtering.
The additional import ofTimeFilterfrom"./SharedFilters"aligns well with the new time-based filter approach.
74-74: TimeFilter usage is straightforward.
Replacing the previous date-range UI with<TimeFilter />simplifies user interactions by focusing on relative time periods.apps/webapp/app/presenters/v3/DevPresence.server.ts (4)
6-8: Class definition is structured well.
TheDevPresenceclass encapsulates Redis interactions cleanly, improving maintainability by centralizing logic.
13-17: Method isConnected() is succinct and effective.
This cleanly queries Redis for a specific presence key and returns a boolean status.
24-26: Private method getPresenceKey() is a good abstraction.
This ensures key naming consistency for presence lookups.
29-36: Redis client instantiation with environment config is handled well.
Auto-pipelining and TLS support are correctly toggled, which should be efficient and secure.apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts (1)
49-49: New propertyhasAnyTokensin the Result type.
AddinghasAnyTokens: booleanenriches the response object and clarifies whether any tokens exist, even when the returned list is empty.Also applies to: 63-63, 92-92
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.dev.presence.tsx (2)
1-8: Import structure is clear and aligns with the new Redis-based presence logic.
No issues found here.
9-75: SSE loader logic appears correct and well-structured.
ThecreateSSELoaderusage, presence checks withdevPresence, and error handling with fallback logs provide a robust approach for real-time developer presence updates.apps/webapp/app/presenters/v3/RunListPresenter.server.ts (7)
9-9: Good addition of time filtering utilities.Adding the
timeFiltersutility function import streamlines the filtering logic across different list views.
63-68: Excellent implementation of default time period filters.This implementation addresses the performance issue mentioned in the PR objectives by setting a default time period for filtering, which prevents inefficient searches through all items.
82-83: Properly updated filter condition to include time defaults.The condition now correctly accounts for whether the time filter is using default values, ensuring consistent behavior across the application.
195-195: Appropriate use of the time object for period calculation.The code now correctly uses the standardized time object for period calculation, maintaining consistency with the filtering utility.
301-307: Good SQL query condition updates for time filtering.The SQL query conditions have been properly updated to use the structured time object properties, which should improve query consistency and maintainability.
347-363: Nice addition of the hasAnyRuns check.This check efficiently determines if any runs exist for the project, which is valuable for UI rendering decisions and prevents unnecessary queries when filtering returns no results.
428-432: Return object properly updated with new properties.The return object now includes the standardized time filter values and the hasAnyRuns property, providing a complete and consistent API for consumers.
apps/webapp/app/components/navigation/SideMenu.tsx (6)
23-27: Improved connection icon imports.Good organizational change to import all connection-related icons together, including the new CheckingConnectionIcon for better UX feedback.
540-547: Enhanced connection status handling with intermediary state.The code now correctly handles the undefined connection state, displaying the appropriate CheckingConnectionIcon when the connection status is being determined.
553-557: Improved tooltip messaging for connection states.The tooltip content now provides appropriate feedback for all possible connection states, including the new "checking connection" state.
563-567: Dialog header correctly updated for all connection states.The dialog header now properly displays different messages based on the connection status, improving user feedback.
572-574: Correctly updated image selection for connection status.The image selection logic now properly handles the undefined state by only showing the connected image when isConnected is strictly true.
578-582: Enhanced paragraph content based on connection status.The paragraph now displays appropriate messaging for all connection states, maintaining consistency with other UI elements.
packages/cli-v3/src/apiClient.ts (5)
497-497: Simplified devPresenceConnection method signature.The method now returns EventSource directly instead of a Promise, which makes the API more straightforward and fits better with the event-driven nature of EventSource.
502-505: Added robust retry mechanism.Good addition of retry logic with configurable parameters for handling connection failures, which will improve the reliability of the dev presence feature.
517-520: Added connection success handler.The onopen handler now properly resets the retry count on successful connection, which is essential for the retry mechanism to work correctly over time.
522-548: Implemented comprehensive error handling with exponential backoff.The error handling logic now includes:
- Detailed logging of connection errors
- Checking if reconnection is necessary
- Implementing exponential backoff for retries
- Limiting the maximum number of retries
This significantly improves the reliability of the dev presence feature.
550-550: Simplified return statement.The method now directly returns the EventSource object, making the code more straightforward and easier to understand.
apps/webapp/app/presenters/v3/RunPresenter.server.ts (4)
5-5: Added import for redirectWithErrorMessage utility.Good addition of the import for the redirectWithErrorMessage utility, which will be used for handling environment mismatch errors.
15-20: Added custom error class for environment mismatches.The new RunEnvironmentMismatchError class properly extends Error and sets a custom name, allowing for specific error handling in routes that use this presenter.
33-33: Added environmentSlug parameter to call method.The environmentSlug parameter is correctly added to the method signature, enabling environment validation.
Also applies to: 40-41
92-96: Implemented environment validation check.This check effectively addresses the PR objective of fixing incorrect redirection when accessing a run page with the wrong environment in the URL by throwing a specific error that can be caught by the route handler.
apps/webapp/app/routes/engine.v1.dev.presence.ts (3)
3-3: Good transition to the new DevPresence systemThe import of
devPresencefrom the new module is a clean approach to centralizing presence management logic.
9-10: Improved timing configurationUsing
env.DEV_PRESENCE_SSE_TIMEOUTdirectly and calculating interval as a percentage (80%) of TTL is a good practice. This ensures the connection stays alive while preventing unnecessary refreshes.
30-35: Simplified presence managementThe code now uses the centralized
devPresence.setConnectedmethod in both initialization and iteration phases, which provides better consistency.apps/webapp/app/v3/services/triggerScheduledTask.server.ts (3)
1-13: Well-structured imports and Redis configurationThe type import for PrismaClientOrTransaction and the addition of Redis with environment-based configuration are good practices for maintainability and flexibility across different environments.
72-83: Improved handling of dev environment scheduled tasksThe code now checks both v3 (session-based) and v4 (presence-based) connection status before deciding whether to trigger a task in a development environment. This ensures backward compatibility while supporting the new system.
169-176: Useful addition of last run timestampRecording the
lastRunTriggeredAttimestamp provides valuable metadata about when tasks were last executed, which will help with debugging and auditing scheduled tasks.apps/webapp/app/presenters/v3/BatchListPresenter.server.ts (4)
46-50: Improved time filtering with standardized helperUsing the
timeFiltershelper provides a more consistent approach to time-based filtering across the application, which is a good refactoring choice.
54-54: Simplified filter conditionThe condition for
hasFiltershas been streamlined to include status filters, friendlyId checks, and time defaults, making the code more readable.
148-152: Consistent date filtering in SQL queryThe SQL query now uses the standardized
time.fromandtime.toobjects consistently, which aligns with the goal of standardizing time filtering across the application.
185-198: Added check for empty batch resultsThe new
hasAnyBatchesimplementation improves UX by checking if any batches exist even when the filtered results are empty. This allows the UI to distinguish between "no results for this filter" and "no batches at all."apps/webapp/app/components/runs/v3/RunFilters.tsx (4)
15-15: Updated imports for new filtering approachThe import changes reflect the shift from multiple date-related components to a more centralized time filtering approach, which is consistent with the changes in other files.
Also applies to: 47-47
83-83: Simplified period handling in filter schemaThe preprocessing for the period field is now cleaner, using a consistent approach to convert "all" to undefined, which works well with the new time filtering system.
121-121: Added unified TimeFilter componentThe addition of the
TimeFiltercomponent aligns with the PR objectives of adding a default time period filter, which should improve performance for listing pages.
681-685: UI consistency improvementsThe styling changes to the RootOnlyToggle component improve visual consistency with other filter elements while maintaining clear functionality.
apps/webapp/app/components/DevPresence.tsx (7)
9-9: Added a third state to connection trackingThe
isConnectedproperty now allowsundefinedas a possible value, which enables the UI to properly indicate when connection status is being determined (checking state).
14-14: Default connection state is now indeterminateSetting the default value to
undefinedaligns with the third state introduced above, showing "checking connection" in the UI until an actual connection state is established.
30-30: Simplified event source URLRemoving the environment slug from the event source URL simplifies the connection checking mechanism, which aligns with the PR objective of making the dev presence feature more reliable.
37-37: Updated state type to reflect three possible statesThe state now correctly handles the three possible connection states: connected (true), disconnected (false), or checking (undefined).
41-43: Added proper handling for the disabled stateIf the component is disabled or there are no events, the connection status is appropriately set to
undefinedrather than maintaining a potentially incorrect state.
48-57: Improved connection status determination logicThe connection status determination is now more direct and focused on the
isConnectedproperty in the parsed data. This simplifies the code and makes it more maintainable.
65-67: Simplified context valueThe context value is now focused solely on the
isConnectedstate, removing the unnecessary complexity of tracking the last seen timestamp.apps/webapp/app/components/runs/v3/BatchFilters.tsx (4)
39-39: Improved filter importsUpdated imports to use the new
TimeFiltercomponent, which provides a more consistent and streamlined approach to time-based filtering across the application.
51-54: Consistent period handlingThe schema processing for period values correctly converts "all" to undefined while preserving other period values, maintaining backward compatibility with existing URLs.
65-65: Simplified filter detectionThe
hasFilterscheck has been simplified to only look for status and ID filters, as the time filter is now always present with a default value, improving UI clarity.
70-70: Added default time filteringThe
TimeFiltercomponent is now included in the filter row, implementing the PR objective of adding a default time period filter to address performance issues with large datasets.apps/webapp/app/components/runs/v3/SharedFilters.tsx (7)
3-3: Added parse-duration dependencyAdded the
parse-durationlibrary to convert human-readable time durations (like "7d") into milliseconds for filtering, supporting the new time-based filtering approach.
46-94: Improved time period labels and valuesUpdated the time periods with more concise labels and a broader range of options, making the filtering UI more user-friendly while maintaining precise filtering capabilities.
97-101: Established default time periodSet up a default period of 7 days with proper error handling, directly implementing the PR objective to add a default time period filter for better performance.
103-146: Created unified time filtering logicThe new
timeFiltersfunction provides centralized logic for handling different time filtering scenarios (period-based or date range-based), with appropriate defaults when no filters are specified. This ensures consistency across the application.
148-202: Refactored period filter to time filterRenamed and updated the filter component to use the new centralized time filtering logic, providing a more consistent user experience while maintaining the same functionality.
204-246: Enhanced date range selectionImproved the date range selection by managing state more effectively and adding a convenient period selection interface, making the filtering experience more user-friendly.
256-331: Redesigned time filter UIThe time filter UI has been completely redesigned with a grid of quick-select buttons for common time periods and improved date fields for custom ranges, significantly enhancing usability.
...pp/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.batches/route.tsx
Show resolved
Hide resolved
Default time period filter
For projects with lots of items and in situations where there are fewer than 25 results, queries were slow on the listing pages. This is because it was effectively searching through every single item in the table, because there was no default time period.
Added a default time period filter of 7 days to all list views:
This involved a switch around of the UI to make it work nicely.
Tag filtering
Run tag filtering should now be much faster. It uses
startsWithcase insensitive, instead of contains case insensitive. This should have much better query performance with our indexes.Wrong run env redirection
If you go to a run page with the wrong env in the URL, it will redirect you to the correct one.
Dev presence reliability improvements
The dev presence (are you connected with the CLI) is now much more reliable and faster to update in the dashboard. It's also simpler and has been extracted more nicely.
v4 Schedules firing in dev
Dev schedules only trigger if the CLI is connected. We weren't checking if the v4 CLI was connected, so no scheduled dev runs were happening for v4.
Summary by CodeRabbit
New Features
UI Improvements
Refactor