feat: instance contextual layout fetching #3818
Conversation
📝 WalkthroughWalkthroughAdds support for instance-scoped layout requests behind a backend feature flag: extends backend feature types, adds an instance-layout URL helper and query, conditionally uses per-instance fetch in the layouts context when flagged and instanceId exists, and updates tests and test mocks. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
Jondyr
added
backport-ignore
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/features/applicationMetadata/types.ts(1 hunks)src/features/form/layout/LayoutsContext.tsx(2 hunks)src/queries/queries.ts(1 hunks)src/utils/urls/appUrlHelper.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Avoid usinganyand unnecessary type casts (as Type) in TypeScript; prefer precise typings and refactor existing casts/anys
For TanStack Query, use objects to manage query keys and functions, and centralize shared options viaqueryOptions
Files:
src/queries/queries.tssrc/utils/urls/appUrlHelper.tssrc/features/applicationMetadata/types.tssrc/features/form/layout/LayoutsContext.tsx
🧠 Learnings (3)
📚 Learning: 2025-08-22T13:53:28.252Z
Learnt from: CR
Repo: Altinn/app-frontend-react PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-22T13:53:28.252Z
Learning: Applies to **/*.test.{ts,tsx} : In tests, use `renderWithProviders` from `src/test/renderWithProviders.tsx` to supply required form layout context
Applied to files:
src/features/form/layout/LayoutsContext.tsx
📚 Learning: 2025-08-22T13:53:28.252Z
Learnt from: CR
Repo: Altinn/app-frontend-react PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-22T13:53:28.252Z
Learning: Applies to src/features/*/Provider.tsx : Place feature-specific context providers as `Provider.tsx` within each feature directory under `src/features/`
Applied to files:
src/features/form/layout/LayoutsContext.tsx
📚 Learning: 2025-08-28T12:00:32.967Z
Learnt from: olemartinorg
Repo: Altinn/app-frontend-react PR: 3645
File: src/features/datamodel/DataModelsProvider.tsx:172-175
Timestamp: 2025-08-28T12:00:32.967Z
Learning: In DataModelsProvider.tsx, the effect that processes referenced data types must run for both stateless and non-stateless apps. The `isFetching` guard is appropriate because stateless apps don't fetch instance data (useInstanceDataQuery is disabled with `enabled: !isStateless`) but still need the data type processing to occur.
Applied to files:
src/features/form/layout/LayoutsContext.tsx
🧬 Code graph analysis (2)
src/queries/queries.ts (4)
src/layout/layout.ts (1)
ILayoutCollection(85-85)src/utils/network/sharedNetworking.ts (1)
httpGet(7-13)src/utils/network/networking.ts (1)
httpGet(10-17)src/utils/urls/appUrlHelper.ts (1)
getLayoutsUrl(188-193)
src/features/form/layout/LayoutsContext.tsx (3)
src/features/instance/InstanceContext.tsx (1)
useLaxInstanceId(58-62)src/features/applicationMetadata/ApplicationMetadataProvider.tsx (1)
useApplicationMetadata(67-67)src/queries/queries.ts (1)
fetchLayouts(293-294)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install
🔇 Additional comments (4)
src/features/applicationMetadata/types.ts (1)
54-56: LGTM! Clean feature flag addition.The new feature flag is well-named and follows the established pattern in the interface. The comment clearly describes its purpose.
src/features/form/layout/LayoutsContext.tsx (1)
9-9: LGTM! Appropriate imports for the new feature.The new imports support instance-aware layout fetching and are correctly sourced.
Also applies to: 16-16
src/queries/queries.ts (1)
293-294: LGTM! Clean function signature extension.The optional
instanceIdparameter is correctly typed and forwarded togetLayoutsUrl. The change maintains backward compatibility.src/utils/urls/appUrlHelper.ts (1)
188-193: LGTM! Clean conditional URL construction.The function correctly constructs instance-scoped URLs when
instanceIdis provided and falls back to the standard API endpoint otherwise. The logic is clear and follows REST conventions.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils/urls/appUrlHelper.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Avoid usinganyand unnecessary type casts (as Type) in TypeScript; prefer precise typings and refactor existing casts/anys
For TanStack Query, use objects to manage query keys and functions, and centralize shared options viaqueryOptions
Files:
src/utils/urls/appUrlHelper.ts
Jondyr
force-pushed
the
feat/instance-contextual-layout-fetching
branch
from
63c92a0 to
559868f
Compare
Magnusrm
left a comment
Magnusrm
left a comment
There was a problem hiding this comment.
Looks good! ⭐
I have a suggestion below that you can apply if you agree with it. The current implementation should also be fine 😊
Jondyr
force-pushed
the
feat/instance-contextual-layout-fetching
branch
from
40079a6 to
df5bac7
Compare
Jondyr
force-pushed
the
feat/instance-contextual-layout-fetching
branch
from
df5bac7 to
4aaddfc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/features/form/layout/LayoutsContext.tsx (1)
40-42: Critical: Query key still missing instanceId, causing cache collisions.This critical issue was raised in a previous review but hasn't been addressed. When
addInstanceIdentifierToLayoutRequestsis enabled and the samelayoutSetIdis used across different instances, React Query will serve stale cached layouts from a previous instance because the query key doesn't distinguish between instances.Apply this diff to include
instanceIdin the query key when the feature flag is enabled:const { fetchLayouts } = useAppQueries(); const instanceId = useLaxInstanceId(); const features = useApplicationMetadata().features ?? {}; + const shouldUseInstanceEndpoint = features.addInstanceIdentifierToLayoutRequests && instanceId; return { - queryKey: ['formLayouts', layoutSetId, enabled], + queryKey: ['formLayouts', layoutSetId, enabled, shouldUseInstanceEndpoint ? instanceId : null], queryFn: layoutSetId ? async () => { - const shouldUseInstanceEndpoint = features.addInstanceIdentifierToLayoutRequests && instanceId; const layouts = shouldUseInstanceEndpointThis ensures different cache entries for each instance when the feature is enabled, preventing stale data from being served.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/features/form/layout/LayoutsContext.tsx(2 hunks)src/queries/queries.ts(2 hunks)src/test/renderWithProviders.tsx(1 hunks)src/utils/urls/appUrlHelper.test.ts(2 hunks)src/utils/urls/appUrlHelper.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/utils/urls/appUrlHelper.test.ts
- src/queries/queries.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Avoid usinganyand unnecessary type casts (as Type) in TypeScript; prefer precise typings and refactor existing casts/anys
For TanStack Query, use objects to manage query keys and functions, and centralize shared options viaqueryOptions
Files:
src/test/renderWithProviders.tsxsrc/features/form/layout/LayoutsContext.tsxsrc/utils/urls/appUrlHelper.ts
🧠 Learnings (6)
📚 Learning: 2025-08-22T13:53:28.252Z
Learnt from: CR
Repo: Altinn/app-frontend-react PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-22T13:53:28.252Z
Learning: Applies to **/*.test.{ts,tsx} : In tests, use `renderWithProviders` from `src/test/renderWithProviders.tsx` to supply required form layout context
Applied to files:
src/test/renderWithProviders.tsxsrc/features/form/layout/LayoutsContext.tsx
📚 Learning: 2025-10-14T09:01:40.985Z
Learnt from: lassopicasso
Repo: Altinn/app-frontend-react PR: 3654
File: src/layout/ImageUpload/ImageUploadSummary2/ImageUploadSummary2.tsx:49-53
Timestamp: 2025-10-14T09:01:40.985Z
Learning: In the Altinn app-frontend-react codebase, when data integrity is guaranteed through business logic and parent components guard rendering (e.g., `storedImage ? <Component /> : undefined`), non-null assertions on guaranteed fields like `storedImage!.data?.filename` are acceptable and preferred over optional chaining with fallbacks.
Applied to files:
src/features/form/layout/LayoutsContext.tsx
📚 Learning: 2025-09-08T10:35:54.904Z
Learnt from: bjosttveit
Repo: Altinn/app-frontend-react PR: 3676
File: .github/scripts/revert.sh:71-72
Timestamp: 2025-09-08T10:35:54.904Z
Learning: In the Altinn app-frontend-react repository, the Azure storage paths in revert.sh use the structure "$AZURE_TARGET_URI/toolkits/$APP_FULL/" and "$AZURE_TARGET_URI/toolkits/$APP_MAJOR/" without requiring "altinn-app-frontend" in the path.
Applied to files:
src/features/form/layout/LayoutsContext.tsx
📚 Learning: 2025-08-22T13:53:28.252Z
Learnt from: CR
Repo: Altinn/app-frontend-react PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-22T13:53:28.252Z
Learning: Applies to src/features/*/Provider.tsx : Place feature-specific context providers as `Provider.tsx` within each feature directory under `src/features/`
Applied to files:
src/features/form/layout/LayoutsContext.tsx
📚 Learning: 2025-08-28T12:00:32.967Z
Learnt from: olemartinorg
Repo: Altinn/app-frontend-react PR: 3645
File: src/features/datamodel/DataModelsProvider.tsx:172-175
Timestamp: 2025-08-28T12:00:32.967Z
Learning: In DataModelsProvider.tsx, the effect that processes referenced data types must run for both stateless and non-stateless apps. The `isFetching` guard is appropriate because stateless apps don't fetch instance data (useInstanceDataQuery is disabled with `enabled: !isStateless`) but still need the data type processing to occur.
Applied to files:
src/features/form/layout/LayoutsContext.tsx
📚 Learning: 2025-09-03T14:23:33.606Z
Learnt from: olemartinorg
Repo: Altinn/app-frontend-react PR: 3645
File: src/features/receipt/FixWrongReceiptType.tsx:16-34
Timestamp: 2025-09-03T14:23:33.606Z
Learning: In FixWrongReceiptType.tsx, brief ping-ponging between receipt routes during instance data loading is acceptable because the instance data is typically already fetched when this component runs. The author considers this a non-issue.
Applied to files:
src/utils/urls/appUrlHelper.ts
🧬 Code graph analysis (1)
src/features/form/layout/LayoutsContext.tsx (3)
src/features/instance/InstanceContext.tsx (1)
useLaxInstanceId(58-62)src/features/applicationMetadata/ApplicationMetadataProvider.tsx (1)
useApplicationMetadata(67-67)src/queries/queries.ts (2)
fetchLayoutsForInstance(296-297)fetchLayouts(294-294)
🔇 Additional comments (4)
src/test/renderWithProviders.tsx (1)
155-155: LGTM! Consistent test mock addition.The
fetchLayoutsForInstancemock follows the same pattern asfetchLayouts(line 154), correctly requiring tests to explicitly override this query when exercising the per-instance layout fetch path.src/utils/urls/appUrlHelper.ts (1)
189-190: LGTM! Clean URL helper implementation.The function follows the established pattern for instance-scoped endpoints in this file and constructs the URL path consistently with other helpers like
getFileUploadUrlandgetValidationUrl.src/features/form/layout/LayoutsContext.tsx (2)
9-9: LGTM! Import additions support the feature correctly.The new imports (
useApplicationMetadata,useLaxInstanceId,fetchLayoutsForInstance) are appropriately scoped and necessary for the instance-contextual layout fetching feature.Also applies to: 16-16, 19-19
37-38: LGTM! Hook usage is appropriately defensive.The
useLaxInstanceId()correctly returnsundefinedwhen there's no instance, and the?? {}fallback on line 38 provides proper safety for the features object.
|
github-project-automation
bot
moved this from 🔎 In review
to ✅ Done
in Team Altinn Studio
2 participants
Description
Adds support for new feature flag
addInstanceIdentifierToLayoutRequestsfrom app-lib-dotnet#1544. If this feature is toggled on, the frontend will request layouts from a new endpoint that allows the app developer to access instance related data (allowing e.g. custom layouts for an instance).Related Issue(s)
related PR: feat: instance contextual layout fetching app-lib-dotnet#1544
closes #{issue number}
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit
New Features
Tests