fix: normalize created proxy rule status in console#3023
Conversation
Greptile SummaryThis PR centralizes proxy-rule status handling into a new Confidence Score: 4/5Safe to merge; changes are a clean refactor with a correctness fix and no regressions identified. All P2 findings — no logic errors or runtime failures found. The two observations are about intentional behavioral changes (log visibility for src/lib/components/domains/status.ts — verify Important Files Changed
Reviews (1): Last reviewed commit: "fix: normalize created proxy rule status..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export function isProxyRuleLogsViewable(proxyRule: Models.ProxyRule): boolean { | ||
| return ( | ||
| Boolean(proxyRule.logs?.length) && | ||
| normalizeProxyRuleStatus(proxyRule.status) !== ProxyRuleStatus.Verified | ||
| ); |
There was a problem hiding this comment.
created status
The old code whitelisted only 'verifying' and 'unverified' for log viewing. The new normalizedStatus !== Verified predicate also includes created (before normalization) and any unrecognized future statuses. Since the PR intentionally treats created as a failure state equivalent to unverified, this is probably correct — but worth confirming that exposing logs on a freshly-created (never-attempted) rule is the desired UX, in case the API returns created for brand-new rules that have no meaningful logs yet.
| export function getProxyRuleUpdatedPrefix(status: ProxyRuleStatusValue): string { | ||
| const normalizedStatus = normalizeProxyRuleStatus(status); | ||
|
|
||
| if (normalizedStatus === ProxyRuleStatus.Verifying) { | ||
| return 'Updated'; | ||
| } | ||
|
|
||
| if (normalizedStatus === ProxyRuleStatus.Unverified) { | ||
| return 'Failed'; | ||
| } | ||
|
|
||
| return ''; |
There was a problem hiding this comment.
created timestamp label changed from 'Checked' to 'Failed'
The old updatedLabel emitted 'Checked <time>' for created status. After normalization, created maps to unverified which now returns 'Failed', so the label becomes 'Failed <time>'. This is arguably more accurate, but it's a visible user-facing string change. If "Checked" had a distinct meaning (e.g., "DNS was checked but certificate generation hasn't started"), dropping it may remove useful context for users diagnosing their setup.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
2 participants
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)