Skip to content

Conversation

olemartinorg
Copy link
Contributor

@olemartinorg olemartinorg commented Sep 30, 2025

Description

The functionality added in #3641 updates initial validation, but the backend returns new incremental validations. This fixes the issue by re-using the functionality we had for updating incremental navigation. It also adds a functionality test in Cypress that also regression-tests the problem discovered in #3742.

Related Issue(s)

Verification/QA

  • Manual functionality testing
    • I have tested these changes manually
    • Creator of the original issue (or service owner) has been contacted for manual testing (or will be contacted when released in alpha)
    • No testing done/necessary
  • Automated tests
    • Unit test(s) have been added/updated
    • Cypress E2E test(s) have been added/updated
    • No automatic tests are needed here (no functional changes/additions)
    • I want someone to help me make some tests
  • UU/WCAG (follow these guidelines until we have our own)
    • I have tested with a screen reader/keyboard navigation/automated wcag validator
    • No testing done/necessary (no DOM/visual changes)
    • I want someone to help me perform accessibility testing
  • User documentation @ altinn-studio-docs
    • Has been added/updated
    • No functionality has been changed/added, so no documentation is needed
    • I will do that later/have created an issue
  • Support in Altinn Studio
    • Issue(s) created for support in Studio
    • This change/feature does not require any changes to Altinn Studio
  • Sprint board
    • The original issue (or this PR itself) has been added to the Team Apps project and to the current sprint board
    • I don't have permissions to do that, please help me out
  • Labels
    • I have added a kind/* and backport* label to this PR for proper release notes grouping
    • I don't have permissions to add labels, please help me out

Summary by CodeRabbit

  • New Features

    • Incremental validation updates for attachments and form data, giving faster, more responsive feedback.
    • Smoother tag editing on attachments with immediate validation updates.
  • Bug Fixes

    • Validation messages now reliably appear/disappear when adding or removing attachment tags.
    • Prevents stale errors after partial saves and when revisiting edited entries.
  • Performance

    • Reduced unnecessary validation recalculations for snappier interactions.
  • Tests

    • Added end-to-end coverage for attachment tags validation, including multi-file uploads and form submission.
  • Refactor

    • Consolidated validation handling to a single incremental update mechanism.

Ole Martin Handeland added 3 commits September 30, 2025 12:21
…d adding it as a separate hook. This should work better than updating initial validations, which broke navigation later in the app.
@olemartinorg olemartinorg added kind/bug Something isn't working backport-ignore This PR is a new feature and should not be cherry-picked onto release branches labels Sep 30, 2025
Copy link

coderabbitai bot commented Sep 30, 2025

📝 Walkthrough

Walkthrough

Implements an incremental backend validation flow. Replaces previous initial-validation updates on attachment tag changes with a hook-driven incremental update. Refactors backend validation initialization and updates, adjusts form-data save mutation output mapping, exposes a static selector in validation context, and adds Cypress tests plus a new page object selector.

Changes

Cohort / File(s) Summary of changes
Backend validation incremental update refactor
src/features/validation/backendValidation/BackendValidation.tsx, src/features/validation/backendValidation/useUpdateIncrementalValidations.ts, src/features/validation/validationContext.tsx
Replaces deep-compare incremental logic with useUpdateIncrementalValidations; restructures initial vs. incremental update effects; removes unused mapper; adds new hook implementing cached-initial vs. last-save comparison and update; exposes useStaticSelector and switches updates to use it.
Attachments tags update path
src/features/attachments/AttachmentsStorePlugin.tsx
Swaps backend validation utilities and context usage for useUpdateIncrementalValidations; updates validations only when issues exist; simplifies SetTags payload/update flow; removes DataModelsProvider/legacy mappers reliance.
Form data save mutation mapping
src/features/formData/FormDataWrite.tsx
Returns useMutation(...) directly; maps validationIssues via backendValidationIssueGroupListToObject; imports related types/constants from validation.
E2E: attachment tags validation
test/e2e/integration/expression-validation-test/tags-validation.ts, test/e2e/pageobjects/app-frontend.ts
Adds Cypress suite covering attachment tag validation behavior across edits and submissions; introduces expressionValidationTest.cvUploader selector.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The PR includes modifications to FormDataWrite.tsx and validationContext.tsx that are unrelated to the incremental attachment validation bug and its tests, introducing refactors outside the scope of issue #3742. Remove or relocate the unrelated FormDataWrite and validationContext changes into a separate refactoring PR to keep this fix focused on the attachment validation issue.
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely describes the primary change, which is fixing the navigation blockage that occurred after validating attachment tags, directly reflecting the main bug being addressed without including extraneous details.
Linked Issues Check ✅ Passed The changes implement the linked issue’s objectives by handling incremental validations correctly (replacing the initial validation update flow), reusing existing navigation update logic, and adding a Cypress regression test for attachment tag validation, fulfilling all coding tasks outlined for issue #3742.
Description Check ✅ Passed The pull request description follows the repository template by summarizing the change in the Description section, linking the related issue (#3742), and providing a complete Verification/QA checklist covering manual testing, automated tests, accessibility, documentation, support, sprint board status, and labels.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bug/tag-validation

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 036a298 and 5fe0d79.

📒 Files selected for processing (7)
  • src/features/attachments/AttachmentsStorePlugin.tsx (4 hunks)
  • src/features/formData/FormDataWrite.tsx (3 hunks)
  • src/features/validation/backendValidation/BackendValidation.tsx (1 hunks)
  • src/features/validation/backendValidation/useUpdateIncrementalValidations.ts (1 hunks)
  • src/features/validation/validationContext.tsx (2 hunks)
  • test/e2e/integration/expression-validation-test/tags-validation.ts (1 hunks)
  • test/e2e/pageobjects/app-frontend.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Avoid using any and 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 via queryOptions

Files:

  • test/e2e/pageobjects/app-frontend.ts
  • test/e2e/integration/expression-validation-test/tags-validation.ts
  • src/features/validation/backendValidation/BackendValidation.tsx
  • src/features/validation/backendValidation/useUpdateIncrementalValidations.ts
  • src/features/attachments/AttachmentsStorePlugin.tsx
  • src/features/validation/validationContext.tsx
  • src/features/formData/FormDataWrite.tsx
🧬 Code graph analysis (5)
test/e2e/integration/expression-validation-test/tags-validation.ts (1)
test/e2e/pageobjects/app-frontend.ts (1)
  • AppFrontend (3-372)
src/features/validation/backendValidation/BackendValidation.tsx (5)
src/features/validation/validationContext.tsx (1)
  • Validation (320-375)
src/features/datamodel/DataModelsProvider.tsx (1)
  • DataModels (394-435)
src/features/validation/backendValidation/backendValidationUtils.ts (4)
  • useShouldValidateInitial (30-36)
  • mapBackendIssuesToTaskValidations (110-127)
  • mapBackendValidationsToValidatorGroups (129-147)
  • mapValidatorGroupsToDataModelValidations (177-198)
src/features/validation/backendValidation/backendValidationQuery.ts (1)
  • useBackendValidationQuery (129-172)
src/features/validation/backendValidation/useUpdateIncrementalValidations.ts (1)
  • useUpdateIncrementalValidations (18-47)
src/features/validation/backendValidation/useUpdateIncrementalValidations.ts (5)
src/features/validation/validationContext.tsx (1)
  • Validation (320-375)
src/features/datamodel/DataModelsProvider.tsx (1)
  • DataModels (394-435)
src/features/validation/backendValidation/backendValidationQuery.ts (1)
  • useGetCachedInitialValidations (29-40)
src/features/validation/index.ts (1)
  • BackendValidationIssueGroups (99-101)
src/features/validation/backendValidation/backendValidationUtils.ts (3)
  • mapBackendValidationsToValidatorGroups (129-147)
  • mapBackendIssuesToFieldValidations (55-105)
  • mapValidatorGroupsToDataModelValidations (177-198)
src/features/attachments/AttachmentsStorePlugin.tsx (2)
src/features/validation/backendValidation/useUpdateIncrementalValidations.ts (1)
  • useUpdateIncrementalValidations (18-47)
src/features/validation/index.ts (1)
  • backendValidationIssueGroupListToObject (111-115)
src/features/formData/FormDataWrite.tsx (1)
src/features/validation/index.ts (1)
  • backendValidationIssueGroupListToObject (111-115)
⏰ 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: Install
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Type-checks, eslint, unit tests and SonarCloud
🔇 Additional comments (15)
test/e2e/pageobjects/app-frontend.ts (1)

353-353: LGTM!

The new cvUploader selector follows the existing pattern and will be used by the new attachment tag validation test.

src/features/validation/validationContext.tsx (2)

147-147: LGTM!

Exposing useStaticSelector enables consumers to access stable references without triggering re-renders, which is appropriate for the validation update functions.


363-364: LGTM!

Switching to useStaticSelector for these update functions is correct, as they are stable references that don't need to trigger re-renders when accessed.

test/e2e/integration/expression-validation-test/tags-validation.ts (1)

1-82: LGTM!

The test comprehensively covers the attachment tag validation flow, including:

  • Uploading multiple files and assigning tags
  • Editing tags and verifying incremental validation updates
  • Ensuring validations appear and disappear correctly
  • Complete form submission workflow

This provides excellent regression coverage for issue #3742.

src/features/validation/backendValidation/useUpdateIncrementalValidations.ts (1)

1-47: LGTM!

The hook provides a clean, centralized approach to updating incremental validations. The logic:

  • Fetches cached initial validations as baseline
  • Clones and applies new validation groups
  • Uses deep equality to prevent unnecessary updates
  • Properly updates backend validation state

The implementation follows React best practices with correct dependency tracking.

src/features/formData/FormDataWrite.tsx (3)

25-29: LGTM!

Importing the centralized validation conversion utility aligns with the PR's objective to consolidate incremental validation handling.


181-181: LGTM!

Directly returning the mutation result is a minor refactor with no behavioral change.


312-312: LGTM!

Using the centralized backendValidationIssueGroupListToObject converter is correct here, as the multipatch endpoint returns an array format that needs conversion to the object format expected by the caller.

src/features/validation/backendValidation/BackendValidation.tsx (4)

1-12: LGTM! Clean refactor to centralized incremental validation hook.

The import of useUpdateIncrementalValidations consolidates the incremental validation logic, replacing the previous manual deep-equality-based approach.


20-21: LGTM! Clear naming and centralized hook usage.

The rename from isFetching to isFetchingInitial improves clarity. The useUpdateIncrementalValidations hook centralizes the incremental validation update logic.


24-31: LGTM! Initial validation flow is correctly implemented.

The initial validation effect properly computes task validations and validator groups, then updates backend validations. The dependency array is correctly updated to include all required dependencies.


33-38: LGTM! Excellent simplification of incremental validation handling.

The refactor consolidates complex incremental validation logic into a single hook call. This directly addresses the bug where initial validations were incorrectly updated when the backend returned incremental validations (issue #3742). The simplified approach is much easier to maintain and reason about.

src/features/attachments/AttachmentsStorePlugin.tsx (3)

26-27: LGTM! Necessary imports for incremental validation handling.

The imports of backendValidationIssueGroupListToObject and useUpdateIncrementalValidations enable the attachment update flow to properly handle incremental backend validations.


413-420: LGTM! Tag update flow is correct.

The updateTags mutation call properly passes the dataElementId and tags. The mutation's onSuccess handler (lines 800-804) now correctly processes incremental validation updates from the backend response.


785-806: LGTM! Correctly handles incremental validations from attachment tag updates.

This change directly addresses the bug in issue #3742. The mutation now properly updates incremental validations (not initial validations) when the backend returns validation issues after attachment tag changes. The conditional check ensures updates only occur when validation issues are present.


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.

❤️ Share
🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copy link
Contributor

@framitdavid framitdavid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@olemartinorg olemartinorg merged commit 2e6c896 into main Oct 3, 2025
18 checks passed
@olemartinorg olemartinorg deleted the bug/tag-validation branch October 3, 2025 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-ignore This PR is a new feature and should not be cherry-picked onto release branches kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validation on attachment upload may block subsequent navigation
2 participants