fix(react): centralize inline query keys into reusable factories - #442
fix(react): centralize inline query keys into reusable factories#442harishsundar-okta wants to merge 7 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change centralizes React Query keys for MFA step-up, member management, organization details, and domains. It adds parameterized key builders, migrates service consumers to shared keys, removes obsolete organization helpers, and expands query-key tests. ChangesMFA query keys
Organization query keys
Domain query keys
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #442 +/- ##
==========================================
- Coverage 90.89% 90.88% -0.01%
==========================================
Files 239 239
Lines 17696 17703 +7
Branches 1995 2595 +600
==========================================
+ Hits 16084 16089 +5
- Misses 1612 1614 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![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 (1)
packages/core/src/services/my-account/mfa/mfa-constants.ts (1)
27-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for the new query-key factory.
Cover the exact outputs of
all,enrollmentFactors(mfaToken), andauthenticators(mfaToken), including distinct tokens producing distinct keys. As per coding guidelines, add unit tests for new hooks/utilities.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/services/my-account/mfa/mfa-constants.ts` around lines 27 - 33, Add unit tests for the mfaStepUpQueryKeys factory, asserting the exact array values returned by all, enrollmentFactors, and authenticators. Include cases with different mfaToken values to verify each token produces a distinct key.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/core/src/services/my-organization/member-management/member-management-constants.ts`:
- Around line 14-25: Add direct unit tests for the memberManagementQueryKeys
factories, covering invitationList, memberList, identityProviders, and the roles
factory referenced as roles (memberRoles in the review). Assert each factory
returns the exact expected readonly key array, including representative
invitation/member parameters and a roles search-independent key.
In
`@packages/react/src/hooks/my-organization/shared/services/use-member-management-service.ts`:
- Line 89: Remove the locally declared memberManagementQueryKeys from the
service and import the shared memberManagementQueryKeys export from
`@auth0/universal-components-core`. Update the query and cache-management calls at
the referenced locations to use that core factory consistently, preserving the
existing key method usage.
---
Nitpick comments:
In `@packages/core/src/services/my-account/mfa/mfa-constants.ts`:
- Around line 27-33: Add unit tests for the mfaStepUpQueryKeys factory,
asserting the exact array values returned by all, enrollmentFactors, and
authenticators. Include cases with different mfaToken values to verify each
token produces a distinct key.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f11e868-449e-45c6-b4a4-8cd3d2b0cc6f
📒 Files selected for processing (5)
packages/core/src/services/my-account/mfa/mfa-constants.tspackages/core/src/services/my-organization/member-management/member-management-constants.tspackages/react/src/hooks/my-organization/shared/__tests__/use-domain-table-service.test.tspackages/react/src/hooks/my-organization/shared/services/use-member-management-service.tspackages/react/src/hooks/shared/use-mfa-requirements.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/core/src/services/my-account/mfa/__tests__/mfa-query-keys.test.ts (1)
5-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
when...descriptions for test conditions.Rename the top-level
describeblocks to condition-oriented names, such aswhen accessing MFA query keysandwhen accessing MFA step-up query keys; keep theitnames action-oriented.As per coding guidelines, Vitest tests must describe conditions with
when...and describe actions in theitname.Also applies to: 16-17
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/services/my-account/mfa/__tests__/mfa-query-keys.test.ts` around lines 5 - 6, Rename the top-level describe blocks in mfaQueryKeys tests to condition-oriented when... descriptions, including the MFA step-up query keys block, while keeping the existing it descriptions action-oriented.Source: Coding guidelines
packages/react/src/hooks/my-organization/__tests__/use-member-management-service.test.ts (1)
74-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse action-oriented names for the Vitest cases.
Rename these
itdescriptions from “should have...” to action-oriented names such as “returns the expected invitation list key” and “returns the expected identity provider key.”As per coding guidelines, Vitest tests should describe conditions with
when...and describe actions in theitname.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react/src/hooks/my-organization/__tests__/use-member-management-service.test.ts` around lines 74 - 106, Rename the affected Vitest descriptions in the memberManagementQueryKeys tests from “should have...” to action-oriented wording that states the returned result, such as “returns the expected invitation list key,” “returns the expected members key,” “returns the expected member list key,” and “returns the expected identity provider key.” Keep the test behavior and assertions unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/core/src/services/my-account/mfa/__tests__/mfa-query-keys.test.ts`:
- Around line 5-6: Rename the top-level describe blocks in mfaQueryKeys tests to
condition-oriented when... descriptions, including the MFA step-up query keys
block, while keeping the existing it descriptions action-oriented.
In
`@packages/react/src/hooks/my-organization/__tests__/use-member-management-service.test.ts`:
- Around line 74-106: Rename the affected Vitest descriptions in the
memberManagementQueryKeys tests from “should have...” to action-oriented wording
that states the returned result, such as “returns the expected invitation list
key,” “returns the expected members key,” “returns the expected member list
key,” and “returns the expected identity provider key.” Keep the test behavior
and assertions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b28ffd95-1659-459e-b120-907621c32f83
📒 Files selected for processing (2)
packages/core/src/services/my-account/mfa/__tests__/mfa-query-keys.test.tspackages/react/src/hooks/my-organization/__tests__/use-member-management-service.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/react/src/hooks/my-organization/shared/services/use-organization-details-query.ts`:
- Around line 16-26: The useOrganizationDetailsQuery hook lacks direct unit
coverage for its query configuration and behavior. Add Vitest tests targeting
useOrganizationDetailsQuery that verify the organizationDetailsQueryKeys.details
cache key, API organizationDetails.get call, OrganizationDetailsMappers.fromAPI
mapping, and the disabled/no-client behavior when coreClient is unavailable.
In
`@packages/react/src/hooks/my-organization/shared/services/use-sso-provider-edit-service.ts`:
- Around line 399-401: Update the detach flow around ensureQueryData to reuse
the existing organization query state or shared organization-details query,
including its queryFn, rather than calling ensureQueryData with only the details
key. Ensure organization details are available when the cache is missing or
evicted, and prevent detach from starting while organizationQuery.isLoading.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c4f5adcf-7a05-433e-9a53-13dcfc85edb4
📒 Files selected for processing (9)
packages/core/src/services/my-organization/idp-management/sso-provider/sso-provider-query-keys.tspackages/core/src/services/my-organization/member-management/member-management-constants.tspackages/react/src/hooks/my-organization/shared/__tests__/use-sso-provider-table-service.test.tspackages/react/src/hooks/my-organization/shared/services/use-member-detail-service.tspackages/react/src/hooks/my-organization/shared/services/use-member-management-service.tspackages/react/src/hooks/my-organization/shared/services/use-organization-details-edit-service.tspackages/react/src/hooks/my-organization/shared/services/use-organization-details-query.tspackages/react/src/hooks/my-organization/shared/services/use-sso-provider-edit-service.tspackages/react/src/hooks/my-organization/shared/services/use-sso-provider-table-service.ts
💤 Files with no reviewable changes (2)
- packages/core/src/services/my-organization/member-management/member-management-constants.ts
- packages/core/src/services/my-organization/idp-management/sso-provider/sso-provider-query-keys.ts
This reverts commit eb98cd7.
…nto fix/reusable-queries # Conflicts: # packages/core/src/services/my-account/user-mfa-management/__tests__/mfa-query-keys.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react/src/hooks/my-organization/shared/services/use-sso-provider-edit-service.ts (1)
83-90: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not cache placeholder organization data under the shared key.
initialData: OrganizationDetailsFactory.create()persists the empty organization object toorganizationDetailsQueryKeys.details(), and this key is shared byuseMemberManagementService,useMemberDetailService, anduseSsoProviderTableService.useSsoProviderTableService.fetchOrganizationDetailscan then get that cached placeholder fromensureQueryDatawhile the API request is pending. RemoveinitialDatafrom the shared query and keep the local return fallback at line 722.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react/src/hooks/my-organization/shared/services/use-sso-provider-edit-service.ts` around lines 83 - 90, Remove the initialData option using OrganizationDetailsFactory.create() from the shared organization details query, so the placeholder is not cached under organizationDetailsQueryKeys.details(). Preserve the local fallback return in useSsoProviderTableService.fetchOrganizationDetails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@packages/react/src/hooks/my-organization/shared/services/use-sso-provider-edit-service.ts`:
- Around line 83-90: Remove the initialData option using
OrganizationDetailsFactory.create() from the shared organization details query,
so the placeholder is not cached under organizationDetailsQueryKeys.details().
Preserve the local fallback return in
useSsoProviderTableService.fetchOrganizationDetails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e10812a-09dc-4020-87ce-0f52eb93d56e
📒 Files selected for processing (4)
packages/react/src/hooks/my-organization/shared/services/use-member-detail-service.tspackages/react/src/hooks/my-organization/shared/services/use-member-management-service.tspackages/react/src/hooks/my-organization/shared/services/use-sso-provider-edit-service.tspackages/react/src/hooks/my-organization/shared/services/use-sso-provider-table-service.ts
…nto fix/reusable-queries
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/services/my-account/user-mfa-management/user-mfa-management-constants.ts (1)
27-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new exported query-key contract.
mfaStepUpQueryKeysis exported from core and consumed by the React layer. Add JSDoc for the namespace and both builders. Include@paramand@returns, or mark the symbol@internalif it is not part of the package public API.As per coding guidelines, document public APIs with JSDoc and use
@module,@internal,@param, and@returnstags where applicable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/services/my-account/user-mfa-management/user-mfa-management-constants.ts` around lines 27 - 33, The exported mfaStepUpQueryKeys contract lacks documentation. Add JSDoc to mfaStepUpQueryKeys and its enrollmentFactors and authenticators builders, including `@param` and `@returns` descriptions for each builder; if this export is not intended as public API, mark the namespace `@internal` instead.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/core/src/services/my-account/user-mfa-management/user-mfa-management-constants.ts`:
- Around line 27-33: The exported mfaStepUpQueryKeys contract lacks
documentation. Add JSDoc to mfaStepUpQueryKeys and its enrollmentFactors and
authenticators builders, including `@param` and `@returns` descriptions for each
builder; if this export is not intended as public API, mark the namespace
`@internal` instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1ac7d178-2eb1-486c-ad2c-9edf6327a39f
📒 Files selected for processing (4)
packages/core/src/services/my-account/user-mfa-management/__tests__/mfa-query-keys.test.tspackages/core/src/services/my-account/user-mfa-management/user-mfa-management-constants.tspackages/react/src/hooks/my-organization/shared/services/use-sso-provider-edit-service.tspackages/react/src/hooks/my-organization/shared/services/use-sso-provider-table-service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/react/src/hooks/my-organization/shared/services/use-sso-provider-edit-service.ts
- packages/react/src/hooks/my-organization/shared/services/use-sso-provider-table-service.ts
2 participants
Summary
Centralizes inline TanStack Query keys into reusable factory functions and extracts a shared organization details query hook to eliminate duplication and cache fragmentation across services.
Why
What
Packages
packages/corepackages/reactexamplesReferences
Testing
How can this be verified? Note anything intentionally not covered by tests and why.
Checklist
Contributing
Summary by CodeRabbit
Refactor
Tests