Fixing attachment removed in PATCH when process ends #3953

olemartinorg · 2026-01-16T13:26:10Z

Description

This should fix a case where attachments were attempted to be removed from the form data when transitioning from a data task -> receipt. The root cause for this was StoreAttachmentsInNodeWorker building a list of attachments for a node, and since mapAttachments() (called from useNodeAttachments()) skips attachments where taskId is not the current one it would skip these attachments since the process data has been updated.

Instead of a localized fix, the issue at hand is that NodesContext (which has these 'store attachments' effects) should be reset when navigating to another task. It used to do that properly by checking the current layouts, but process data is updated before we get a response from the layouts query, thus it's an earlier indicator that NodesContext should reset itself.

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

Bug Fixes
- Prevents unintended context regeneration during task transitions, reducing UI flicker and inconsistent state when moving between workflow steps.
- Improves stability and navigation reliability by temporarily blocking background updates while a task transition is in progress.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

…receipt (i.e. changing taskId), since we get the process back earlier than we get new layouts. This should prevent node context effects from running while transitioning to a new task (essentially resetting NodesContext earlier than before).

coderabbitai · 2026-01-16T13:26:25Z

📝 Walkthrough

Walkthrough

Adds task-transition awareness to the NodesContext module: introduces an internal hook to detect task transitions from navigation params, and updates the global context regeneration condition to account for in-transition state while guarding against specific end-state false positives.

Changes

Cohort / File(s)	Change Summary
Task Transition Detection `src/utils/layout/NodesContext.tsx`	Added `useIsInTaskTransition` internal hook to detect task transitions by comparing current task elementId with taskId from URL/navigation params; updated context regeneration logic to include `isInTaskTransition` in its condition; added guards for `ProcessEnd` / `CustomReceipt` to avoid false positives.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main bug fix—preventing attachment removal during task/process transitions—and directly relates to the changeset modifications in NodesContext.tsx.
Description check	✅ Passed	The description comprehensively covers the bug, root cause, and solution; includes related Slack references; and properly documents verification steps with appropriate checkboxes completed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch bug/receipt-removed-attachment

🧹 Recent nitpick comments

src/utils/layout/NodesContext.tsx (1)
344-354: Avoid type casting by using direct equality checks.

The as TaskKeys cast on line 348 can be eliminated by using direct equality comparisons instead of Array.includes(). This aligns with the coding guidelines and improves type safety.
♻️ Suggested refactor
 function useIsInTaskTransition() {
   const currentTask = useProcessQuery().data?.currentTask?.elementId;
   const taskIdFromUrl = useNavigationParam('taskId');

-  if ([TaskKeys.ProcessEnd, TaskKeys.CustomReceipt].includes(taskIdFromUrl as TaskKeys) && !currentTask) {
+  if ((taskIdFromUrl === TaskKeys.ProcessEnd || taskIdFromUrl === TaskKeys.CustomReceipt) && !currentTask) {
     // Receipt indicates that the process ended - it cannot be compared directly with the taskId
     return false;
   }

   return currentTask !== taskIdFromUrl;
 }

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 277c59a and b688beb.

📒 Files selected for processing (1)

src/utils/layout/NodesContext.tsx

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Avoid using any type or type casting (as type) in TypeScript code; improve typing by avoiding casts and anys when refactoring
Use objects for managing query keys and functions, and queryOptions for sharing TanStack Query patterns across the system for central management

Files:

src/utils/layout/NodesContext.tsx

{**/*.module.css,**/*.{ts,tsx}}

📄 CodeRabbit inference engine (CLAUDE.md)

Use CSS Modules for component styling and leverage Digdir Design System components when possible

Files:

src/utils/layout/NodesContext.tsx

🧠 Learnings (3)

📚 Learning: 2025-08-28T12:25:27.289Z

Learnt from: olemartinorg
Repo: Altinn/app-frontend-react PR: 3645
File: src/components/wrappers/ProcessWrapper.tsx:107-110
Timestamp: 2025-08-28T12:25:27.289Z
Learning: In ProcessWrapper.tsx, when taskType === ProcessTaskType.Archived and taskId !== TaskKeys.CustomReceipt, the component shows a loader with reason 'redirect-to-receipt' but does not perform explicit navigation. This is intentional - other parts of the system handle the actual navigation closer to where user interactions occur.

Applied to files:

src/utils/layout/NodesContext.tsx

📚 Learning: 2025-09-03T14:26:18.627Z

Learnt from: olemartinorg
Repo: Altinn/app-frontend-react PR: 3645
File: src/components/wrappers/ProcessWrapper.tsx:78-83
Timestamp: 2025-09-03T14:26:18.627Z
Learning: In ProcessWrapper.tsx, the useIsRunningProcessNext() hook intentionally uses a non-reactive pattern with queryClient.isMutating and local state instead of useIsMutating hook. This design choice is deliberate and should not be changed to a reactive pattern.

Applied to files:

src/utils/layout/NodesContext.tsx

📚 Learning: 2025-11-25T12:53:54.399Z

Learnt from: CR
Repo: Altinn/app-frontend-react PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T12:53:54.399Z
Learning: Applies to **/*.test.{ts,tsx} : Use `renderWithProviders` from `src/test/renderWithProviders.tsx` when testing components that require form layout context

Applied to files:

src/utils/layout/NodesContext.tsx

🧬 Code graph analysis (1)

src/utils/layout/NodesContext.tsx (2)

src/features/instance/useProcessQuery.ts (1)

useProcessQuery (23-26)

src/hooks/navigation.ts (1)

useNavigationParam (51-55)

⏰ 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: Type-checks, eslint, unit tests and SonarCloud
GitHub Check: Install

🔇 Additional comments (3)

src/utils/layout/NodesContext.tsx (3)

16-16: LGTM!

The new imports are correctly organized and necessary for the task-transition detection logic.

Also applies to: 24-25

285-285: LGTM!

The hook is correctly invoked at the component top level, and the extended condition properly blocks node generation during task transitions. This addresses the race condition where process data updates before layouts, preventing premature attachment removal.

Also applies to: 321-324

340-343: Good documentation of the hook's purpose.

The comment clearly explains why this hook exists and when it returns true. This helps future maintainers understand the task-transition timing issue being addressed.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

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

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

sonarqubecloud · 2026-01-16T13:52:54Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
88.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

* mapAttachments() hook returned no attachments when navigating to the receipt (i.e. changing taskId), since we get the process back earlier than we get new layouts. This should prevent node context effects from running while transitioning to a new task (essentially resetting NodesContext earlier than before). * This should fix the tests that broke (this was a bit sloppy) --------- Co-authored-by: Ole Martin Handeland <git@olemartin.org>

github-actions · 2026-01-19T15:59:07Z

✅ Automatic backport successful!

A backport PR has been automatically created for the release/v4.24 release branch.

The release branch release/v4.24 already existed and was updated.

The cherry-pick was clean with no conflicts. Please review the backport PR when it appears.

* mapAttachments() hook returned no attachments when navigating to the receipt (i.e. changing taskId), since we get the process back earlier than we get new layouts. This should prevent node context effects from running while transitioning to a new task (essentially resetting NodesContext earlier than before). * This should fix the tests that broke (this was a bit sloppy) --------- Co-authored-by: Ole Martin Handeland <git@olemartin.org>

olemartinorg self-assigned this Jan 16, 2026

olemartinorg added kind/bug Something isn't working backport This PR should be cherry-picked onto older release branches taskforce/next Issues that belongs to the named task-force labels Jan 16, 2026

olemartinorg added this to Team Altinn Studio Jan 16, 2026

olemartinorg moved this to 👷 In progress in Team Altinn Studio Jan 16, 2026

This should fix the tests that broke (this was a bit sloppy)

b688beb

cammiida approved these changes Jan 19, 2026

View reviewed changes

olemartinorg merged commit 2214c4c into main Jan 19, 2026
16 checks passed

olemartinorg deleted the bug/receipt-removed-attachment branch January 19, 2026 15:58

github-project-automation bot moved this from 👷 In progress to ✅ Done in Team Altinn Studio Jan 19, 2026

github-actions bot mentioned this pull request Jan 19, 2026

[Backport] Fixing attachment removed in PATCH when process ends #3955

Merged

olemartinorg mentioned this pull request Jan 22, 2026

Backporting: Fixing attachment removed in PATCH when process ends (#3953) #3964

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing attachment removed in PATCH when process ends #3953

Fixing attachment removed in PATCH when process ends #3953

Uh oh!

olemartinorg commented Jan 16, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jan 16, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Uh oh!

sonarqubecloud bot commented Jan 16, 2026

Uh oh!

Uh oh!

github-actions bot commented Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fixing attachment removed in PATCH when process ends #3953

Fixing attachment removed in PATCH when process ends #3953

Uh oh!

Conversation

olemartinorg commented Jan 16, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

Verification/QA

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

sonarqubecloud bot commented Jan 16, 2026

Quality Gate passed

Uh oh!

Uh oh!

github-actions bot commented Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

olemartinorg commented Jan 16, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jan 16, 2026 •

edited

Loading