Conversation
📝 WalkthroughWalkthroughReplaces useMemo with useRef to persist fetched data, consolidates activation logic into an isActive flag sourced from meta or dataRef.current, introduces shouldPoll to control polling, and simplifies conditional rendering for certificate issuance and service setup (keeps existing UI branches). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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 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 |
![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
🤖 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/modules/reverse-proxy/table/ReverseProxyStatusCell.tsx`:
- Around line 34-41: The poll flag currently stops as soon as certificateIssued
is true, which can freeze the UI if the service hasn't reached ACTIVE; fix by
computing shouldPoll using the fetched ReverseProxy status so polling continues
until status === 'ACTIVE'. Concretely, call
useFetchApi(`/reverse-proxies/services/${serviceId}`, ...) with polling enabled,
then compute shouldPoll = !!enabled && ( !certificateIssued || data?.status !==
'ACTIVE' ) (or move the hook so you can derive shouldPoll from data), and pass
that flag (or stop polling only when data.status === 'ACTIVE') so the cell keeps
refreshing until the service becomes ACTIVE; reference symbols: shouldPoll,
certificateIssued, useFetchApi, ReverseProxy, serviceId, POLL_INTERVAL_MS.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ebbb7752-b36e-4192-8944-c36f3ec17a71
📒 Files selected for processing (1)
src/modules/reverse-proxy/table/ReverseProxyStatusCell.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/modules/reverse-proxy/table/ReverseProxyStatusCell.tsx`:
- Around line 26-34: The readiness checks use mixed snapshots (meta and
dataRef.current?.meta) which can combine to a false-positive ready state; fix by
selecting one coherent metadata object (e.g., const currentMeta = meta ??
dataRef.current?.meta) and then compute isActive and certificateIssued from that
single currentMeta (use ReverseProxyStatus.ACTIVE for isActive and check
certificate_issued_at for certificateIssued), and update shouldPoll accordingly;
apply the same change to the analogous check around the other occurrence
referenced (the second computation at line ~46).
- Line 44: The code unconditionally assigns dataRef.current = data which can
overwrite the last-known status with undefined when fetching is disabled; change
the update to only overwrite the ref when a real value is present (e.g., check
data !== undefined/null) so dataRef.current retains the previous cached status
when data is undefined; update the assignment used in ReverseProxyStatusCell
(the dataRef and data variables in that component) to guard the write
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 292774ae-d6de-4a2c-8556-3c674ab714d6
📒 Files selected for processing (1)
src/modules/reverse-proxy/table/ReverseProxyStatusCell.tsx
| const isActive = | ||
| meta?.status === ReverseProxyStatus.ACTIVE || | ||
| dataRef.current?.meta?.status === ReverseProxyStatus.ACTIVE; | ||
|
|
||
| const certificateIssued = | ||
| !!meta?.certificate_issued_at || | ||
| !!dataRef.current?.meta?.certificate_issued_at; | ||
|
|
||
| const shouldPoll = !!enabled && !(isActive && certificateIssued); |
There was a problem hiding this comment.
Use one coherent metadata source for readiness checks
isActive and certificateIssued are currently computed with separate ORs across meta and dataRef.current. That can produce a synthetic (ACTIVE && issued) state from different snapshots, which may stop polling and hide the cell too early.
🔧 Proposed fix
- const isActive =
- meta?.status === ReverseProxyStatus.ACTIVE ||
- dataRef.current?.meta?.status === ReverseProxyStatus.ACTIVE;
-
- const certificateIssued =
- !!meta?.certificate_issued_at ||
- !!dataRef.current?.meta?.certificate_issued_at;
+ const effectiveMeta = dataRef.current?.meta ?? meta;
+ const isActive = effectiveMeta?.status === ReverseProxyStatus.ACTIVE;
+ const certificateIssued = !!effectiveMeta?.certificate_issued_at;Also applies to: 46-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/reverse-proxy/table/ReverseProxyStatusCell.tsx` around lines 26 -
34, The readiness checks use mixed snapshots (meta and dataRef.current?.meta)
which can combine to a false-positive ready state; fix by selecting one coherent
metadata object (e.g., const currentMeta = meta ?? dataRef.current?.meta) and
then compute isActive and certificateIssued from that single currentMeta (use
ReverseProxyStatus.ACTIVE for isActive and check certificate_issued_at for
certificateIssued), and update shouldPoll accordingly; apply the same change to
the analogous check around the other occurrence referenced (the second
computation at line ~46).
| if (data && data?.meta) return !!data?.meta?.certificate_issued_at; | ||
| return certificateIssued; | ||
| }, [data]); | ||
| dataRef.current = data; |
There was a problem hiding this comment.
Preserve last known data instead of overwriting ref with undefined
Unconditionally assigning dataRef.current = data can clear cached status when fetching is disabled, which can reintroduce stale UI state on later renders.
🔧 Proposed fix
- dataRef.current = data;
+ if (data !== undefined) {
+ dataRef.current = data;
+ }📝 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.
| dataRef.current = data; | |
| if (data !== undefined) { | |
| dataRef.current = data; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/reverse-proxy/table/ReverseProxyStatusCell.tsx` at line 44, The
code unconditionally assigns dataRef.current = data which can overwrite the
last-known status with undefined when fetching is disabled; change the update to
only overwrite the ref when a real value is present (e.g., check data !==
undefined/null) so dataRef.current retains the previous cached status when data
is undefined; update the assignment used in ReverseProxyStatusCell (the dataRef
and data variables in that component) to guard the write accordingly.
2 participants
Summary by CodeRabbit