Skip to content

feat: remove sorting from signing document list#3775

Open
vxkc wants to merge 1 commit intomainfrom
feat/signing-document-list-ordering
Open

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

Conversation

@vxkc
Copy link
Contributor

@vxkc vxkc commented Oct 9, 2025

Sorting is done on the backend in Altinn/app-lib-dotnet#1511

Description

Remove the sorting of the rows in SigningDocumentList. Sorting is done on the backend in Altinn/app-lib-dotnet#1511.

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
  • 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

    • Document list now respects the backend's provided ordering when supported; otherwise items are sorted by filename.
  • Bug Fixes

    • Signing Document list ordering is now deterministic and aligned with backend capabilities, so item order may differ from prior releases.
  • Tests

    • Added tests to verify ordering behavior across backend capability scenarios.

@vxkc vxkc marked this pull request as draft October 9, 2025 13:59
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025

📝 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 added the kind/product-feature Pull requests containing new features 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 This PR is a new feature and should not be cherry-picked onto release branches 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
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

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

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

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

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

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

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/product-feature Pull requests containing new features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Component: "SigningDocumentList" - customize sequence of documents shown

3 participants