Cleans up and collocates versions and features #3640
Conversation
📝 WalkthroughWalkthroughCentralizes version-based feature checks into a new module Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
cammiida
force-pushed
the
chore/versioning-cleanup
branch
from
afb7716 to
fb47c1b
Compare
cammiida
added
kind/other
![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 (3)
src/features/applicationMetadata/VersionErrorOrChildren.tsx (1)
23-23: Avoid hard-coding the minimum backend version string. Source it from the central versioning module.Hard-coding 'v8.0.0' risks drift when minimum version changes. Pull a label from versions.ts instead.
Apply in this file:
+import { minimumApplicationVersionLabel } from 'src/utils/versioning/versions'; @@ - params={['v8.0.0']} + params={[minimumApplicationVersionLabel()]}And add this helper in src/utils/versioning/versions.ts (or equivalent), keeping label formatting consistent:
export function minimumApplicationVersionLabel(): string { // Adjust if your map stores a prefixed value const ver = FEATURE_VERSION_MAP.MINIMUM_APPLICATION_VERSION; return ver.startsWith('v') ? ver : `v${ver}`; }I can wire this up if you prefer.
src/features/validation/backendValidation/backendValidationQuery.ts (1)
16-16: Centralized feature-gate usage looks good; minor readability nit.Deconstruct altinnNugetVersion before use to avoid nesting a hook call inside the argument list.
-import { appSupportsIncrementalValidationFeatures } from 'src/utils/versioning/versions'; +import { appSupportsIncrementalValidationFeatures } from 'src/utils/versioning/versions'; @@ - const hasIncrementalValidationFeatures = appSupportsIncrementalValidationFeatures( - useApplicationMetadata().altinnNugetVersion, - ); + const { altinnNugetVersion } = useApplicationMetadata(); + const hasIncrementalValidationFeatures = appSupportsIncrementalValidationFeatures(altinnNugetVersion);Also applies to: 135-137
src/utils/versioning/versions.ts (1)
14-26: Consider introducing an “unknown” state instead of defaulting to false for undefined versionsWe verified that every altinnNugetVersion value in tests, mocks and e2e (e.g. ‘8.5.0.157’, ‘8.5.0.141’, ‘8.0.0.0’, ‘8.1.0.115’) and all entries in FEATURE_VERSION_MAP use a consistent four‐segment format, so isAtLeastVersion will always compare like–for–like. However, when currentNugetVersion is undefined, isFeatureSupported unconditionally returns false, causing call sites (e.g. PDFPreviewButtonComponent.tsx, useProcessNext.tsx) to treat “unknown” as “unsupported” and potentially surface errors or hide features prematurely.
To refine this behavior, you can:
• Change isFeatureSupported (src/utils/versioning/versions.ts) to return boolean | undefined—where undefined means “version unknown”—and update the typed feature‐support helpers accordingly.
• In each consumer (e.g. appSupportsPdfPreviewButton, appSupportsUnlockingOnProcessNextFailure), use strict checks (=== false) to trigger unsupported flows and treat undefined as “neutral” (no UI change).
• If you’d rather keep the helpers boolean‐only, audit all call sites and guard on currentNugetVersion before invoking support checks, ensuring you never display an “unsupported” message when version metadata is simply missing.This refactor is optional but will prevent unintended UI errors when version metadata isn’t yet available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
src/features/applicationMetadata/ApplicationMetadataProvider.tsx(2 hunks)src/features/applicationMetadata/VersionErrorOrChildren.tsx(1 hunks)src/features/attachments/AttachmentsStorePlugin.tsx(3 hunks)src/features/attachments/tools.ts(0 hunks)src/features/instance/useProcessNext.tsx(5 hunks)src/features/language/LanguageProvider.tsx(2 hunks)src/features/validation/backendValidation/backendValidationQuery.ts(2 hunks)src/features/validation/backendValidation/backendValidationUtils.ts(0 hunks)src/layout/PDFPreviewButton/PDFPreviewButtonComponent.tsx(2 hunks)src/utils/versioning/versionCompare.test.ts(1 hunks)src/utils/versioning/versions.ts(1 hunks)
💤 Files with no reviewable changes (2)
- src/features/validation/backendValidation/backendValidationUtils.ts
- src/features/attachments/tools.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/features/validation/backendValidation/backendValidationQuery.tssrc/features/applicationMetadata/VersionErrorOrChildren.tsxsrc/features/applicationMetadata/ApplicationMetadataProvider.tsxsrc/utils/versioning/versionCompare.test.tssrc/utils/versioning/versions.tssrc/layout/PDFPreviewButton/PDFPreviewButtonComponent.tsxsrc/features/language/LanguageProvider.tsxsrc/features/attachments/AttachmentsStorePlugin.tsxsrc/features/instance/useProcessNext.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
In tests, use
renderWithProvidersfromsrc/test/renderWithProviders.tsxto supply required form layout context
Files:
src/utils/versioning/versionCompare.test.ts
🧬 Code graph analysis (7)
src/features/validation/backendValidation/backendValidationQuery.ts (2)
src/utils/versioning/versions.ts (1)
appSupportsIncrementalValidationFeatures(61-63)src/features/applicationMetadata/ApplicationMetadataProvider.tsx (1)
useApplicationMetadata(67-67)
src/features/applicationMetadata/ApplicationMetadataProvider.tsx (1)
src/utils/versioning/versions.ts (1)
isMinimumApplicationVersion(28-30)
src/utils/versioning/versions.ts (1)
src/utils/versioning/versionCompare.ts (1)
isAtLeastVersion(13-32)
src/layout/PDFPreviewButton/PDFPreviewButtonComponent.tsx (4)
src/layout/layout.ts (1)
NodeValidationProps(99-102)src/features/instance/InstanceContext.tsx (1)
useStrictInstanceId(75-82)src/features/applicationMetadata/ApplicationMetadataProvider.tsx (1)
useApplicationMetadata(67-67)src/utils/versioning/versions.ts (2)
appSupportsPdfPreviewButton(32-34)FEATURE_VERSION_MAP(3-10)
src/features/language/LanguageProvider.tsx (1)
src/utils/versioning/versions.ts (1)
appSupportsFetchAppLanguagesInAnonymous(36-38)
src/features/attachments/AttachmentsStorePlugin.tsx (1)
src/utils/versioning/versions.ts (1)
appSupportsNewAttachmentAPI(44-46)
src/features/instance/useProcessNext.tsx (2)
src/features/applicationMetadata/ApplicationMetadataProvider.tsx (1)
useApplicationMetadata(67-67)src/utils/versioning/versions.ts (2)
appSupportsUnlockingOnProcessNextFailure(40-42)appSupportsIncrementalValidationFeatures(61-63)
⏰ 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). (4)
- GitHub Check: Cypress Run Internal (4)
- GitHub Check: Cypress Run Internal (2)
- GitHub Check: check-label
- GitHub Check: Type-checks, eslint, unit tests and SonarCloud
🔇 Additional comments (11)
src/utils/versioning/versionCompare.test.ts (1)
1-1: Import path migration verified – no old imports remainRan searches across
.ts/.tsxfiles and confirmed there are no imports fromsrc/utils/versionCompare. All instances reference the new pathsrc/utils/versioning/versionComparein:
src/utils/versioning/versions.tssrc/utils/versioning/versionCompare.test.tsNo further changes needed.
src/features/applicationMetadata/ApplicationMetadataProvider.tsx (1)
13-13: No stale versioning imports detected – changes approvedRan the scan for any remaining imports of
MINIMUM_APPLICATION_VERSIONorisAtLeastVersionacross all.tsand.tsxfiles and found only their original definitions in:
src/utils/versioning/versions.tssrc/features/applicationMetadata/minVersion.tsThere are no external consumers still importing from the old locations. All usage now goes through
isMinimumApplicationVersion.src/features/attachments/AttachmentsStorePlugin.tsx (1)
31-31: No remaining imports ofappSupportsNewAttachmentAPIVerified via
rg -nC1 "appSupportsNewAttachmentAPI" --glob "*.ts" --glob "*.tsx"that the only occurrences are:
src/utils/versioning/versions.ts(definition)src/features/attachments/AttachmentsStorePlugin.tsx(import at line 31)No other modules import
appSupportsNewAttachmentAPIfrom any old location.src/features/language/LanguageProvider.tsx (2)
10-10: Good move: centralized feature gate importSwitching to appSupportsFetchAppLanguagesInAnonymous consolidates version checks and improves consistency.
96-96: Confirm semantics of userIsAuthenticatedThis relies on useIsAllowAnonymous(false). If that hook returns “allow anonymous” rather than “is authenticated”, the variable name could mislead and the OR-condition might behave opposite of intent. Please confirm.
src/layout/PDFPreviewButton/PDFPreviewButtonComponent.tsx (1)
10-10: Import change aligns with new versioning moduleUsing appSupportsPdfPreviewButton and FEATURE_VERSION_MAP is consistent with PR objectives.
src/utils/versioning/versions.ts (1)
3-10: Feature map looks solid and typedConstants are frozen with as const; keys align with usages across the PR.
src/features/instance/useProcessNext.tsx (4)
19-22: Centralized version gates: good refactorUsing the shared versions helpers improves cohesion and reduces duplication.
48-48: LGTM: derive altinnNugetVersion onceClear and avoids passing full metadata around.
78-81: Unlock-on-failure check used correctly hereConditionally swallowing the 500 PDF generation error when supported is correct.
104-104: Correct inversion for incremental validation featurePassing !appSupportsIncrementalValidationFeatures(...) to toggle legacy behavior is correct.
cammiida
force-pushed
the
chore/versioning-cleanup
branch
from
fb47c1b to
c88f1f4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/features/applicationMetadata/VersionErrorOrChildren.tsx (1)
22-24: Double-check whether the 'v' prefix is part of the translationIf the i18n string already includes the 'v' prefix, passing 'v8.0.0' (or a name that includes 'v') can yield "vv8.0.0". Prefer passing the raw semantic version (e.g., 8.0.0) and letting the translation format it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
src/features/applicationMetadata/ApplicationMetadataProvider.tsx(2 hunks)src/features/applicationMetadata/VersionErrorOrChildren.tsx(1 hunks)src/features/attachments/AttachmentsStorePlugin.tsx(3 hunks)src/features/attachments/tools.ts(0 hunks)src/features/instance/useProcessNext.tsx(5 hunks)src/features/language/LanguageProvider.tsx(2 hunks)src/features/validation/backendValidation/backendValidationQuery.ts(2 hunks)src/features/validation/backendValidation/backendValidationUtils.ts(0 hunks)src/layout/PDFPreviewButton/PDFPreviewButtonComponent.tsx(2 hunks)src/utils/versioning/versionCompare.test.ts(1 hunks)src/utils/versioning/versions.ts(1 hunks)
💤 Files with no reviewable changes (2)
- src/features/attachments/tools.ts
- src/features/validation/backendValidation/backendValidationUtils.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/utils/versioning/versionCompare.test.ts
- src/features/validation/backendValidation/backendValidationQuery.ts
- src/utils/versioning/versions.ts
- src/features/applicationMetadata/ApplicationMetadataProvider.tsx
- src/features/attachments/AttachmentsStorePlugin.tsx
- src/features/instance/useProcessNext.tsx
- src/features/language/LanguageProvider.tsx
- src/layout/PDFPreviewButton/PDFPreviewButtonComponent.tsx
🧰 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/features/applicationMetadata/VersionErrorOrChildren.tsx
⏰ 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). (3)
- GitHub Check: Type-checks, eslint, unit tests and SonarCloud
- GitHub Check: Install
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
src/features/applicationMetadata/VersionErrorOrChildren.tsx (1)
22-24: Confirm and replace hard-coded versionI wasn’t able to automatically verify the presence of
MINIMUM_APPLICATION_VERSIONin your versioning module, so please double‐check and swap out the literal string once confirmed.• Ensure that
src/utils/versioning/versions.tsexports aMINIMUM_APPLICATION_VERSIONconstant (for example:export const MINIMUM_APPLICATION_VERSION = { name: 'v8.0.0', /* … */ } ```) • Replace the hard‐coded `'v8.0.0'` in `src/features/applicationMetadata/VersionErrorOrChildren.tsx` with the imported constant: ```diff - <Lang - id="version_error.min_backend_version" - params={["v8.0.0"]} - /> + <Lang + id="version_error.min_backend_version" + params={[MINIMUM_APPLICATION_VERSION.name]} + />• Add the import alongside your other module imports:
import { MINIMUM_APPLICATION_VERSION } from "src/utils/versioning/versions";
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/utils/versioning/versions.ts (1)
3-11: Centralization looks good; enforce 4-part version typing and de-drift the min app version.Tie the display name and NuGet threshold together and statically validate version shape to avoid future drift and typos.
Apply:
-export const MINIMUM_APPLICATION_VERSION_NAME = 'v8.0.0'; -export const FEATURE_VERSION_MAP = { - MINIMUM_APPLICATION_VERSION: '8.0.0.108', +export type FourPartVersion = `${number}.${number}.${number}.${number}`; +export const MINIMUM_APPLICATION_VERSION_NAME = 'v8.0.0'; +export const MINIMUM_APPLICATION_VERSION_NUGET: FourPartVersion = '8.0.0.108'; +export const FEATURE_VERSION_MAP = { + MINIMUM_APPLICATION_VERSION: MINIMUM_APPLICATION_VERSION_NUGET, UNLOCKING_ON_PROCESS_NEXT_FAILURE: '8.1.0.115', INCREMENTAL_VALIDATION: '8.5.0.141', NEW_ATTACHMENTS_API: '8.5.0.153', PDF_PREVIEW_BUTTON: '8.5.0.157', APP_LANGUAGES_IN_ANONYMOUS: '8.5.6.180', -} as const; +} as const satisfies Record<string, FourPartVersion>;Note: If TS < 4.9, drop the
satisfiesclause but keepFourPartVersionandMINIMUM_APPLICATION_VERSION_NUGET.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/features/applicationMetadata/VersionErrorOrChildren.tsx(2 hunks)src/features/applicationMetadata/minVersion.ts(0 hunks)src/utils/versioning/versions.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/features/applicationMetadata/minVersion.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/applicationMetadata/VersionErrorOrChildren.tsx
🧰 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/versioning/versions.ts
🧬 Code graph analysis (1)
src/utils/versioning/versions.ts (1)
src/utils/versioning/versionCompare.ts (1)
isAtLeastVersion(13-32)
⏰ 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). (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Install
- GitHub Check: Type-checks, eslint, unit tests and SonarCloud
🔇 Additional comments (1)
src/utils/versioning/versions.ts (1)
29-47: Wrappers read clean and keep callers honest.Nice thin delegates to the centralized checker; names are self-explanatory.
|
|
Cypress-testen som feiler feiler i main også, så ingenting som er introdusert her. Merger denne og ser på å få fikset testen separat. |
github-project-automation
bot
moved this from 🔎 In review
to ✅ Done
in Team Altinn Studio
Description
Related Issue(s)
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit