fix(admin): fix Org Security loading state UI#1738
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughDomainsList now accepts ChangesDomains list retry UI
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
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
🧹 Nitpick comments (1)
web/sdk/admin/views/organizations/details/security/domains-list.tsx (1)
157-165: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRetry button has no in-flight/disabled state.
Clicking Retry triggers
onRetry()(parent'srefetch()), but there's no indication that a refetch is in progress and nothing prevents multiple rapid clicks from firing duplicate requests while the previous one is still pending.Consider threading an
isFetching/isRetryingflag down to disable the button (and optionally show a loading label) while a refetch is outstanding.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1335a2e4-b2b2-4faf-8b48-5ba5442cb8d4
📒 Files selected for processing (2)
web/sdk/admin/views/organizations/details/security/domains-list.tsxweb/sdk/admin/views/organizations/details/security/index.tsx
| if (!isLoading && error) { | ||
| return ( | ||
| <Flex | ||
| justify="between" | ||
| align="center" | ||
| className={styles["domains-list"]} | ||
| style={{ padding: "var(--rs-space-5) var(--rs-space-7)" }} | ||
| > | ||
| <Text size="small" variant="secondary"> | ||
| Couldn't load email domains | ||
| </Text> | ||
| <Button | ||
| variant="outline" | ||
| color="neutral" | ||
| size="small" | ||
| onClick={() => onRetry()} | ||
| data-test-id="retry-domains-button" | ||
| > | ||
| Retry | ||
| </Button> | ||
| </Flex> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file with line numbers
sed -n '1,240p' web/sdk/admin/views/organizations/details/security/domains-list.tsx | cat -n
# Find the query hook and related state usage in this file
rg -n "useQuery|isLoading|error|domains|onRetry|refetch|fetch" web/sdk/admin/views/organizations/details/security/domains-list.tsx
# Locate the query source / hook definition if it exists nearby
fd -a "domains-list.tsx|*.ts|*.tsx" web/sdk/admin/views/organizations/details/security | head -n 50Repository: raystack/frontier
Length of output: 8294
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where DomainsList is used
rg -n "DomainsList" web/sdk/admin -g '*.tsx' -g '*.ts'
# Inspect the parent component that fetches domains
fd -a "domains*.tsx|*security*.tsx|*domain*.tsx" web/sdk/admin/views/organizations/details/security web/sdk/admin/views/organizations/details | head -n 50
# Search for the listOrganizationDomains query usage and its loading/error/data mapping
rg -n "listOrganizationDomains|useQuery\\(|error\\s*:|isLoading\\s*:|domains\\s*=" web/sdk/admin -g '*.tsx' -g '*.ts'Repository: raystack/frontier
Length of output: 984
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the parent component that supplies isLoading/domains/error/onRetry
sed -n '1,220p' web/sdk/admin/views/organizations/details/security/index.tsx | cat -n
# Search for the exact query state mapping in this directory
rg -n "useQuery|isLoading|isFetching|error|data|refetch|DomainsList" web/sdk/admin/views/organizations/details/security -g '*.tsx' -g '*.ts'Repository: raystack/frontier
Length of output: 8666
Gate the error state on missing data. A refetch failure after domains have already loaded will replace the visible list with the retry screen. Show this branch only when there’s no existing data to keep on screen, e.g. !domains.length.
Coverage Report for CI Build 28860476387Coverage remained the same at 44.879%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
2 participants
Summary
The Org → Security page had a buggy loading state — the "Allowed email domains" loader showed up as an unstyled empty box, and the whole section would vanish or behave oddly depending on the data. This PR fixes the loader and makes the loading, empty, and error states consistent and clear.
Changes
react-loading-skeletonto Apsara'sSkeleton(self-styled, no extra CSS needed).Technical Details
react-loading-skeletonSkeletonrelies onreact-loading-skeleton/dist/skeleton.css, which is never imported anywhere inweb/sdk/adminorweb/apps/admin. The app's<SkeletonTheme>only provides color variables, not the base skeleton styles — so the skeleton rendered as an invisible/empty box. Apsara'sSkeletonships its own styles, so it works out of the box (same approach assession-skeleton.tsxandapis/details-dialog.tsx).null). Thenullempty case made the bordered box disappear, causing the "abrupt close" jump. All four states (loading/list/empty/error) now share the same.domains-listcontainer for a stable layout.error+onRetryfrom thelistOrganizationDomainsquery inindex.tsx; Retry calls the query'srefetch(). The existing error toast is kept.Test Plan
ListOrganizationDomainsrequest in DevTools) and confirmed the error state + Retry button appear; Retry re-fetches and recovers.tsc --noEmitclean for the changed files)SQL Safety (if your PR touches
*_repository.goorgoqu.*)N/A