Comments

feat: remove sorting from signing document list#3775

Open

vxkc wants to merge 1 commit intomain from

feat/signing-document-list-ordering

Open

feat: remove sorting from signing document list #3775
vxkc wants to merge 1 commit intomain from
feat/signing-document-list-ordering

Conversation

@vxkc

@vxkc vxkc marked this pull request as draft

October 9, 2025 13:59

@coderabbitai

Copy link

Contributor

coderabbitai bot commented Oct 9, 2025 •

edited

Loading

📝 Walkthrough

Walkthrough

Document list ordering is now conditional on backend capability: a new version check (altinnNugetVersion) determines whether to trust backend ordering or apply client-side filename sorting. Added versioning utility, updated hook signature and component usage, and expanded tests to cover both ordering behaviours.

Changes

Cohort / File(s)	Summary
SigningDocumentList API `src/layout/SigningDocumentList/api.ts`	Added `altinnNugetVersion` parameter; compute `backendHandlesOrdering` via new version helper; include it in queryKey; return `dataElements` as-is when backend supports ordering, otherwise sort by filename.
Component usage / metadata `src/layout/SigningDocumentList/SigningDocumentListComponent.tsx`, `src/layout/SigningDocumentList/SigningDocumentListComponent.test.tsx`	Component now reads `altinnNugetVersion` from `useApplicationMetadata` and passes it to `useDocumentList`; tests updated mocks to include `altinnNugetVersion`.
Hook tests `src/layout/SigningDocumentList/api.test.ts`	New unit tests for `useDocumentList` verifying ordering behaviour for versions >= 8.9.0.225 (preserve backend order) and < 8.9.0.225 or undefined (client sorts by filename).
Versioning utility `src/utils/versioning/versions.ts`	Added `BACKEND_ORDERED_SIGNING_DOCUMENTS` feature version (8.9.0.225) and exported helper `backendSupportsSigningDocumentOrdering(currentNugetVersion)`.
Package / manifest `package.json`	Minor manifest change (small churn noted in diff).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: removing client-side sorting from the SigningDocumentList component as it's now handled by the backend.
Description check	✅ Passed	The PR description follows the template with all required sections completed: description provided, related issue closed, verification/QA checklist mostly filled with testing confirmation, documentation updated, and labels applied.
Linked Issues check	✅ Passed	The PR addresses the version-based fallback mechanism to maintain backwards compatibility with older backends (pre-8.9.0.225) while leveraging backend-side sorting when available, satisfying the compatibility concerns raised in linked discussions.
Out of Scope Changes check	✅ Passed	All changes are scoped to implementing version detection and conditional sorting behavior: adding feature version tracking, updating API signatures, and adding comprehensive unit tests. No unrelated modifications detected.

_{✏️ 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 feat/signing-document-list-ordering

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

src/layout/SigningDocumentList/api.test.ts (1)

60-72: Testing approach is fragile — tightly coupled to useQuery internals.

The test extracts queryFn from useQuery's mock call args and invokes it directly. This bypasses react-query's execution model and is tightly coupled to the internal call signature. If the hook migrates to queryOptions pattern (as suggested by the coding guidelines for centralized TanStack Query config), or if useQuery is wrapped, these tests will break silently.

Consider testing via a QueryClientProvider wrapper so useQuery actually executes, or at minimum add a comment noting the coupling.
src/layout/SigningDocumentList/api.ts (1)
48-53: Minor: filename fallback ?? '' in sort is unreachable after Zod transform.

After the .transform() on line 25 (it.filename ?? it.dataType), filename in the output is always a non-null string. The ?? '' fallback on line 53 is dead code. Not a bug, just a nit.
Proposed simplification
- return dataElements.toSorted((a, b) => (a.filename ?? '').localeCompare(b.filename ?? ''));
+ return dataElements.toSorted((a, b) => a.filename.localeCompare(b.filename));

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.}

@vxkc vxkc mentioned this pull request

Oct 9, 2025

feat: change signing data elements ordering Altinn/altinn-studio-docs#2408

Draft

@vxkc vxkc added the kind/product-feature label

Oct 9, 2025

@vxkc vxkc force-pushed the feat/signing-document-list-ordering branch from 761347f to e4683f5 Compare

October 9, 2025 14:12

@vxkc vxkc added the backport-ignore label

Oct 9, 2025

@sonarqubecloud

Copy link

sonarqubecloud bot commented Oct 9, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 45%)

See analysis details on SonarQube Cloud

@vxkc vxkc marked this pull request as ready for review

October 10, 2025 06:40

bjorntore

bjorntore approved these changes

Oct 10, 2025

View reviewed changes

olemartinorg

olemartinorg requested changes

Oct 10, 2025

View reviewed changes

Copy link

Contributor

@olemartinorg olemartinorg left a comment

Choose a reason for hiding this comment

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

App-frontend has to be backwards-compatible for all versions down to (and including) v8.0.0. Even if sorting now is done backend, I don't think we can remove it from the frontend? In case the sorting is different on backend vs. frontend, we either would have to introduce an opt-out config, or delay this change to the next major version.

If you want to delay this to the next major version only, open a PR in the monorepo instead: https://github.com/Altinn/altinn-studio/tree/main/src/App/frontend

@vxkc vxkc marked this pull request as draft

October 10, 2025 12:40

@bjorntore

Copy link

Contributor

bjorntore commented Oct 13, 2025

App-frontend has to be backwards-compatible for all versions down to (and including) v8.0.0. Even if sorting now is done backend, I don't think we can remove it from the frontend? In case the sorting is different on backend vs. frontend, we either would have to introduce an opt-out config, or delay this change to the next major version.

If you want to delay this to the next major version only, open a PR in the monorepo instead: https://github.com/Altinn/altinn-studio/tree/main/src/App/frontend

Is it easy to check what backend version the frontend is talking to, and sort in frontend if it's before the backend release related to this? Or is that a bad idea?

We're also considering having some flag in the response to tell the frontend to skip sorting, but feels like a hack tbh.

@olemartinorg

Copy link

Contributor

olemartinorg commented Oct 13, 2025

Is it easy to check what backend version the frontend is talking to, and sort in frontend if it's before the backend release related to this?

Yup, that would work. Look in src/utils/versioning/versions.ts for that functionality. Beware that we need both the major.minor.patch version (8.X.X) and the build version. So the functionality needs to arrive in an app-lib-dotnet release before we can know for sure which version sorts this on the backend.

@bjorntore

Copy link

Contributor

bjorntore commented Oct 15, 2025

Is it easy to check what backend version the frontend is talking to, and sort in frontend if it's before the backend release related to this?

Yup, that would work. Look in src/utils/versioning/versions.ts for that functionality. Beware that we need both the major.minor.patch version (8.X.X) and the build version. So the functionality needs to arrive in an app-lib-dotnet release before we can know for sure which version sorts this on the backend.

Is it a fair solution in your opinion? Or should we rethink even more?

@olemartinorg

Copy link

Contributor

olemartinorg commented Oct 15, 2025

I think that's a good enough solution. In v5/next we'll remove that code anyway, as the dynamics of frontend/backend compatibility there will work differently.

@vxkc


 feat: respect signing documents order from backend after 8.9.0

7606e48

@vxkc vxkc force-pushed the feat/signing-document-list-ordering branch from e4683f5 to 7606e48 Compare

February 11, 2026 13:11

@vxkc vxkc marked this pull request as ready for review

February 11, 2026 13:14

@vxkc vxkc requested a review from olemartinorg

February 11, 2026 13:15

@sonarqubecloud

Copy link

sonarqubecloud bot commented Feb 11, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

Labels

backport-ignore kind/product-feature

3 participants

@vxkc @bjorntore @olemartinorg

Comments

Conversation

@vxkc vxkc commented Oct 9, 2025 • 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 Oct 9, 2025 • 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 Oct 9, 2025

Quality Gate Failed Quality Gate failed

Uh oh!

@olemartinorg olemartinorg left a comment

Choose a reason for hiding this comment

Uh oh!

bjorntore commented Oct 13, 2025

Uh oh!

olemartinorg commented Oct 13, 2025

Uh oh!

bjorntore commented Oct 15, 2025

Uh oh!

olemartinorg commented Oct 15, 2025

Uh oh!

sonarqubecloud bot commented Feb 11, 2026

Quality Gate Failed Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

@vxkc vxkc commented Oct 9, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 9, 2025 •

edited

Loading