Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughThe changes introduce a "Live" tailing mode for configuration changes pages. They add an optional Changes
Sequence DiagramsequenceDiagram
participant User
participant Page as ConfigChangesPage
participant Hook as useGetAllConfigsChangesQuery
participant Service as getConfigsChanges
participant API as Backend API
User->>Page: Click "Live" Toggle
activate Page
Note over Page: Initialize tailCursor from newest inserted_at
Page->>Hook: Call with from_inserted_at: tailCursor
activate Hook
Hook->>Service: Forward from_inserted_at parameter
activate Service
Service->>API: POST with from_inserted_at in payload
activate API
API-->>Service: Return changes since cursor
deactivate API
deactivate Service
Hook-->>Page: Return polled changes
deactivate Hook
Note over Page: Deduplicate changes by id<br/>Accumulate in tailedChanges<br/>Advance tailCursor to newest timestamp
Page->>Page: Render merged base + tailed changes
deactivate Page
Note over Page: Wait 5 seconds
Page->>Page: Repeat polling with updated cursor
Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/pages/config/details/ConfigDetailsChangesPage.tsx (1)
11-19:getNewestInsertedAthelper is duplicated across pages.This helper function is identical in both
ConfigChangesPage.tsxandConfigDetailsChangesPage.tsx. Consider extracting it to a shared utility module to improve maintainability.Also applies to: 21-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/config/details/ConfigDetailsChangesPage.tsx` around lines 11 - 19, The getNewestInsertedAt helper is duplicated; extract it into a shared utility (e.g., export function getNewestInsertedAt(changes: ConfigChange[]): string | undefined) and replace the copies in ConfigDetailsChangesPage and ConfigChangesPage with an import from that new module; ensure the new util exports the same signature and any required types (ConfigChange) or import/export the type from its original location so both components compile without changing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/config/ConfigChangesPage.tsx`:
- Around line 96-118: The mapping uses non-null assertions on optional fields
(config_id, type, name) in baseChanges and tailedWithConfig; replace those
assertions with defensive handling by either filtering out entries missing
required fields before mapping or constructing the config object only when
change.config_id, change.type, and change.name are present (e.g., return
undefined or skip the item if any are missing), and ensure baseIds/uniqueTailed
logic works with the possibly filtered arrays; update the transformation that
builds config (in baseChanges and tailedWithConfig) to handle undefined values
safely rather than using the ! operator.
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx`:
- Around line 80-96: The mapping for baseChanges and tailedWithConfig is using
non-null assertions on change.config_id, change.type, and change.name which can
throw if the API returns incomplete records; update the mapping logic in
ConfigDetailsChangesPage (functions/vars: baseChanges and tailedWithConfig) to
either filter out changes missing required fields (e.g., drop entries where any
of config_id/type/name is null/undefined) or provide safe fallbacks (e.g., id:
change.config_id ?? 'unknown', type: change.type ?? 'unknown', name: change.name
?? 'Unnamed') before constructing the config object so runtime errors are
avoided.
---
Nitpick comments:
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx`:
- Around line 11-19: The getNewestInsertedAt helper is duplicated; extract it
into a shared utility (e.g., export function getNewestInsertedAt(changes:
ConfigChange[]): string | undefined) and replace the copies in
ConfigDetailsChangesPage and ConfigChangesPage with an import from that new
module; ensure the new util exports the same signature and any required types
(ConfigChange) or import/export the type from its original location so both
components compile without changing behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d18ca48-c849-4e94-b69c-3b4868d0fe21
📒 Files selected for processing (7)
src/api/query-hooks/useConfigChangesHooks.tssrc/api/services/configs.tssrc/api/types/configs.tssrc/components/Configs/Changes/ConfigChangesFilters/ConfigChangesFilters.tsxsrc/components/Configs/Changes/ConfigsRelatedChanges/FilterBar/ConfigRelatedChangesFilters.tsxsrc/pages/config/ConfigChangesPage.tsxsrc/pages/config/details/ConfigDetailsChangesPage.tsx
Comment on lines
+96
to
+118
| const baseChanges = (data?.changes ?? []).map((change) => ({ | ||
| ...change, | ||
| config: { | ||
| id: change.config_id!, | ||
| type: change.type!, | ||
| name: change.name!, | ||
| deleted_at: change.deleted_at | ||
| } | ||
| })); | ||
|
|
||
| const tailedWithConfig = tailedChanges.map((change) => ({ | ||
| ...change, | ||
| config: { | ||
| id: changes.config_id!, | ||
| type: changes.type!, | ||
| name: changes.name!, | ||
| deleted_at: changes.deleted_at | ||
| id: change.config_id!, | ||
| type: change.type!, | ||
| name: change.name!, | ||
| deleted_at: change.deleted_at | ||
| } | ||
| })); | ||
|
|
||
| const totalChanges = data?.total ?? 0; | ||
| const baseIds = new Set(baseChanges.map((c) => c.id)); | ||
| const uniqueTailed = tailedWithConfig.filter((c) => !baseIds.has(c.id)); | ||
| const changes = [...uniqueTailed, ...baseChanges]; |
Contributor
There was a problem hiding this comment.
Same non-null assertion concern as ConfigDetailsChangesPage.tsx.
The mapping logic uses non-null assertions on optional fields (config_id, type, name). Apply the same defensive handling as suggested for the other page.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/config/ConfigChangesPage.tsx` around lines 96 - 118, The mapping
uses non-null assertions on optional fields (config_id, type, name) in
baseChanges and tailedWithConfig; replace those assertions with defensive
handling by either filtering out entries missing required fields before mapping
or constructing the config object only when change.config_id, change.type, and
change.name are present (e.g., return undefined or skip the item if any are
missing), and ensure baseIds/uniqueTailed logic works with the possibly filtered
arrays; update the transformation that builds config (in baseChanges and
tailedWithConfig) to handle undefined values safely rather than using the !
operator.
Comment on lines
+80
to
+96
| const baseChanges = (data?.changes ?? []).map((change) => ({ | ||
| ...change, | ||
| config: { | ||
| id: changes.config_id!, | ||
| type: changes.type!, | ||
| name: changes.name! | ||
| id: change.config_id!, | ||
| type: change.type!, | ||
| name: change.name! | ||
| } | ||
| })); | ||
|
|
||
| const totalChanges = data?.total ?? 0; | ||
| const tailedWithConfig = tailedChanges.map((change) => ({ | ||
| ...change, | ||
| config: { | ||
| id: change.config_id!, | ||
| type: change.type!, | ||
| name: change.name! | ||
| } | ||
| })); |
Contributor
There was a problem hiding this comment.
Non-null assertions may cause runtime errors if API returns incomplete data.
The code uses non-null assertions (!) on config_id, type, and name properties which are typed as optional in ConfigChange. If the API returns changes without these fields, this will result in undefined values being assigned.
Consider adding fallback values or filtering out incomplete records:
Proposed defensive handling
const baseChanges = (data?.changes ?? []).map((change) => ({
...change,
config: {
- id: change.config_id!,
- type: change.type!,
- name: change.name!
+ id: change.config_id ?? "",
+ type: change.type ?? "",
+ name: change.name ?? ""
}
}));
const tailedWithConfig = tailedChanges.map((change) => ({
...change,
config: {
- id: change.config_id!,
- type: change.type!,
- name: change.name!
+ id: change.config_id ?? "",
+ type: change.type ?? "",
+ name: change.name ?? ""
}
}));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const baseChanges = (data?.changes ?? []).map((change) => ({ | |
| ...change, | |
| config: { | |
| id: changes.config_id!, | |
| type: changes.type!, | |
| name: changes.name! | |
| id: change.config_id!, | |
| type: change.type!, | |
| name: change.name! | |
| } | |
| })); | |
| const totalChanges = data?.total ?? 0; | |
| const tailedWithConfig = tailedChanges.map((change) => ({ | |
| ...change, | |
| config: { | |
| id: change.config_id!, | |
| type: change.type!, | |
| name: change.name! | |
| } | |
| })); | |
| const baseChanges = (data?.changes ?? []).map((change) => ({ | |
| ...change, | |
| config: { | |
| id: change.config_id ?? "", | |
| type: change.type ?? "", | |
| name: change.name ?? "" | |
| } | |
| })); | |
| const tailedWithConfig = tailedChanges.map((change) => ({ | |
| ...change, | |
| config: { | |
| id: change.config_id ?? "", | |
| type: change.type ?? "", | |
| name: change.name ?? "" | |
| } | |
| })); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx` around lines 80 - 96,
The mapping for baseChanges and tailedWithConfig is using non-null assertions on
change.config_id, change.type, and change.name which can throw if the API
returns incomplete records; update the mapping logic in ConfigDetailsChangesPage
(functions/vars: baseChanges and tailedWithConfig) to either filter out changes
missing required fields (e.g., drop entries where any of config_id/type/name is
null/undefined) or provide safe fallbacks (e.g., id: change.config_id ??
'unknown', type: change.type ?? 'unknown', name: change.name ?? 'Unnamed')
before constructing the config object so runtime errors are avoided.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2925
Summary by CodeRabbit
Release Notes