fix: orgnr validation #3838
fix: orgnr validation #3838
Conversation
📝 WalkthroughWalkthroughThe changes introduce a helper function refactoring for checksum calculation and expand test coverage. A new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
HauklandJ
added
kind/incident-C
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/layout/OrganisationLookup/validation.ts (2)
45-50: Fix correctly handles the mod11 special case.The refactoring correctly addresses the bug where calculated check digit 11 should be converted to 0. This aligns with the Norwegian organization number validation standard.
One of the new test cases ('974683520') specifically validates this scenario where the weighted sum yields a modulo result requiring the 11→0 conversion.
Optional: Consider adding a brief comment for clarity.
let calculatedCheckDigit = mod11(sum); +// Norwegian org numbers: if check digit calculates to 11, use 0 instead if (calculatedCheckDigit === 11) { calculatedCheckDigit = 0; }
57-58: Helper function is correct and improves maintainability.The
mod11helper correctly implements the modulo 11 check digit algorithm:11 - (value % 11).Optional: Consider placement closer to usage.
While technically correct (the module loads before functions execute), conventional style would place helper functions before their usage or immediately after the function that calls them, rather than after the schema definitions.
src/layout/OrganisationLookup/validation.test.ts (1)
7-39: Consider more descriptive test names to improve maintainability.All 11 new test cases share the same description "should return true when the orgNr is valid". When a test fails, it will be harder to identify which specific organization number caused the failure.
Consider including the org number in each test description:
- it('should return true when the orgNr is valid', () => { + it('should return true for orgNr 974683520', () => { expect(checkValidOrgnNr('974683520')).toBe(true); }); - it('should return true when the orgNr is valid', () => { + it('should return true for orgNr 900010605', () => { expect(checkValidOrgnNr('900010605')).toBe(true); }); // ... and so on for remaining testsAlternatively, use a parameterized test approach:
it.each([ '974683520', '900010605', '123778847', // ... other valid org numbers ])('should return true for valid orgNr %s', (orgNr) => { expect(checkValidOrgnNr(orgNr)).toBe(true); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/layout/OrganisationLookup/validation.test.ts(1 hunks)src/layout/OrganisationLookup/validation.ts(1 hunks)
🧰 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/layout/OrganisationLookup/validation.tssrc/layout/OrganisationLookup/validation.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
In tests, use
renderWithProvidersfromsrc/test/renderWithProviders.tsxto supply required form layout context
Files:
src/layout/OrganisationLookup/validation.test.ts
🧬 Code graph analysis (1)
src/layout/OrganisationLookup/validation.test.ts (1)
src/layout/OrganisationLookup/validation.ts (1)
checkValidOrgnNr(36-53)
⏰ 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)
|
|
✅ Automatic backport successful! A backport PR has been automatically created for the The release branch The cherry-pick was clean with no conflicts. Please review the backport PR when it appears. |
2 participants
Description
Repoted issue with the validation of org number on external slack:
https://digdir-samarbeid.slack.com/archives/CCQEQKGJD/p1762502941835949
This was due to a special case handling of the control number for the case where the calculated check number is 11.
Related Issue(s)
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit
Tests
Refactor