Feature/dutch nl nl language support#10304
Conversation
Add complete Dutch translations for all WSO2 Identity Server user-facing portals and the React i18n module: - modules/i18n: nl-NL locale (meta, index, common, common-users, myaccount) - authentication-portal: Resources_nl_NL.properties + LanguageOptions entry - recovery-portal: Resources_nl_NL.properties + LanguageOptions entry - accounts portal: Resources_nl_NL.properties + LanguageOptions entry - x509-certificate-authentication-portal: Resources_nl_NL.properties + LanguageOptions entry Resolves: wso2/product-is#27782
Add Dutch (nl-NL) to the locale dropdown used in the self-registration locale attribute input (recovery-portal/LocaleOptions.properties). Completes the nl-NL surface coverage so Dutch users can select Nederlands (Nederland) as their preferred locale when self-registering.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDutch language support added across identity platform portals. Portal language switchers and locale configuration updated, backend translation property files created, React i18n TypeScript modules introduced for metadata and portal strings, and comprehensive tests and validation scripts provided. ChangesDutch (nl-NL) locale implementation
Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
modules/i18n/src/__tests__/nl-NL-completeness.test.ts (1)
23-23: 💤 Low valueConsider adding explicit type annotations.
Variables
myaccountandcommonUserslack explicit type annotations. As per coding guidelines, variables should have explicit types even when the type is obvious.✨ Suggested improvement
- const myaccount = NL_NL.resources.portals.myAccount; + const myaccount: Record<string, unknown> = NL_NL.resources.portals.myAccount as Record<string, unknown>;- const commonUsers = NL_NL.resources.portals.commonUsers; + const commonUsers: Record<string, unknown> = NL_NL.resources.portals.commonUsers as Record<string, unknown>;As per coding guidelines, explicit type annotations should be used everywhere.
Also applies to: 30-30
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/i18n/src/__tests__/nl-NL-completeness.test.ts` at line 23, The variables `myaccount` and `commonUsers` are missing explicit type annotations; update their declarations to include the appropriate type (e.g., the portal resource type or a matching interface) instead of relying on type inference—locate the assignments that use `NL_NL.resources.portals.myAccount` and `NL_NL.resources.portals.commonUsers` and annotate them with the correct type (same type used across other portal resource tests or the portal resource interface) so the declarations are explicitly typed.scripts/check-nl-properties.js (1)
31-40: 💤 Low valueConsider adding error handling for file operations.
The
fs.readFileSynccall on line 32 will throw an uncaught exception if the file doesn't exist or can't be read. While these files should exist, adding error handling would make the script more robust and provide clearer error messages.🛡️ Suggested improvement
langFiles.forEach((file) => { + try { const content = fs.readFileSync(path.resolve(file), "utf8"); const hasNl = content.includes("lang.switch.nl_NL"); console.log(`${hasNl ? "✓" : "✗"} ${file.split("/").slice(-2).join("/")} has nl_NL entry`); if (hasNl) { passed++; } else { failed++; } + } catch (error) { + console.log(`✗ ${file.split("/").slice(-2).join("/")} - Error reading file: ${error.message}`); + failed++; + } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-nl-properties.js` around lines 31 - 40, Wrap the file read and inspect logic inside the langFiles.forEach callback (where fs.readFileSync is called) with a try/catch so IO errors are caught; on error, log a clear message including the file name and the caught error, increment failed (so the script reflects the problem) and continue to the next file, otherwise proceed to compute hasNl and update passed/failed as now; ensure you reference the existing symbols langFiles.forEach, fs.readFileSync, hasNl, passed and failed when making the change.modules/i18n/src/__tests__/nl-NL-registration.test.ts (1)
14-14: ⚡ Quick winReplace
anywith proper typing.The map callback uses
(bundle: any)which violates the coding guideline: "Never useany; use proper types orunknownwith type guards." Consider usingunknownwith proper type checking.♻️ Suggested improvement
it("nl-NL is re-exported from the translation barrel", () => { - const codes = Object.values(translations).map((bundle: any) => bundle.meta?.code); + const codes: (string | undefined)[] = Object.values(translations).map( + (bundle: unknown): string | undefined => + typeof bundle === "object" && bundle !== null && "meta" in bundle + ? (bundle as { meta?: { code?: string } }).meta?.code + : undefined + ); expect(codes).toContain("nl-NL"); });As per coding guidelines,
anyshould never be used.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/i18n/src/__tests__/nl-NL-registration.test.ts` at line 14, The map callback for computing `codes` currently uses `(bundle: any)`—replace `any` with `unknown` and add a runtime type guard for `bundle` to ensure it's an object with a `meta` property that contains `code`; update the callback signature to `(bundle: unknown)` and inside the mapper check `typeof bundle === 'object' && bundle !== null && 'meta' in bundle` (or implement a reusable type predicate like `isTranslationBundle`) before accessing `bundle.meta.code`, returning the code or undefined accordingly so the types are safe without using `any`.modules/i18n/src/__tests__/nl-NL-no-english-bleed.test.ts (1)
4-8: ⚡ Quick winReplace
as anywith proper typing.The test assertions use
(common as any)which violates the coding guideline: "Never useany; use proper types orunknownwith type guards." Consider properly typing thecommonobject or using a type-safe accessor pattern.♻️ Suggested improvement
+interface CommonTranslations { + cancel: string; + save: string; + delete: string; + search: string; + logout: string; +} + describe("nl-NL common spot-check Dutch values", () => { - it("cancel is Dutch", () => expect((common as any).cancel).toBe("Annuleren")); - it("save is Dutch", () => expect((common as any).save).toBe("Opslaan")); - it("delete is Dutch", () => expect((common as any).delete).toBe("Verwijderen")); - it("search is Dutch", () => expect((common as any).search).toBe("Zoeken")); - it("logout is Dutch", () => expect((common as any).logout).toBe("Afmelden")); + const typedCommon: CommonTranslations = common as CommonTranslations; + + it("cancel is Dutch", () => expect(typedCommon.cancel).toBe("Annuleren")); + it("save is Dutch", () => expect(typedCommon.save).toBe("Opslaan")); + it("delete is Dutch", () => expect(typedCommon.delete).toBe("Verwijderen")); + it("search is Dutch", () => expect(typedCommon.search).toBe("Zoeken")); + it("logout is Dutch", () => expect(typedCommon.logout).toBe("Afmelden")); });As per coding guidelines,
anyshould never be used.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/i18n/src/__tests__/nl-NL-no-english-bleed.test.ts` around lines 4 - 8, The tests use an untyped cast `(common as any)` — replace this with a proper type for `common` (e.g., declare or import an interface like `CommonStrings` with properties cancel, save, delete, search, logout) and cast/annotate `common` to that type (or use `unknown` + a runtime type guard) so each assertion reads e.g. `expect((common as CommonStrings).cancel).toBe("Annuleren")`; update the test file’s top to import/declare `CommonStrings` and remove all `as any` occurrences, or add a short type guard function to safely assert properties before asserting values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@identity-apps-core/apps/recovery-portal/src/main/resources/org/wso2/carbon/identity/mgt/recovery/endpoint/i18n/Resources_nl_NL.properties`:
- Line 227: Fix the typo in the property value for key
If.you.specify.tenant.domain.you.registered.under.super.tenant by replacing
"supertenannt" with the correct word "supertenant" so the user-facing Dutch
string reads "Als u geen tenantdomein opgeeft, wordt u geregistreerd onder de
supertenant".
- Line 457: The encoded SCIM key `VW5sb2rIFRpbWU_` is malformed and won't match
runtime lookups for "Unlock Time"; replace that key with the correct Base64
encoding `VW5sb2NrIFRpbWU=` while keeping the translation value `Ontgrendeltijd`
unchanged so the entry becomes `VW5sb2NrIFRpbWU=:{Ontgrendeltijd}` (i.e., update
the left-hand key only).
---
Nitpick comments:
In `@modules/i18n/src/__tests__/nl-NL-completeness.test.ts`:
- Line 23: The variables `myaccount` and `commonUsers` are missing explicit type
annotations; update their declarations to include the appropriate type (e.g.,
the portal resource type or a matching interface) instead of relying on type
inference—locate the assignments that use `NL_NL.resources.portals.myAccount`
and `NL_NL.resources.portals.commonUsers` and annotate them with the correct
type (same type used across other portal resource tests or the portal resource
interface) so the declarations are explicitly typed.
In `@modules/i18n/src/__tests__/nl-NL-no-english-bleed.test.ts`:
- Around line 4-8: The tests use an untyped cast `(common as any)` — replace
this with a proper type for `common` (e.g., declare or import an interface like
`CommonStrings` with properties cancel, save, delete, search, logout) and
cast/annotate `common` to that type (or use `unknown` + a runtime type guard) so
each assertion reads e.g. `expect((common as
CommonStrings).cancel).toBe("Annuleren")`; update the test file’s top to
import/declare `CommonStrings` and remove all `as any` occurrences, or add a
short type guard function to safely assert properties before asserting values.
In `@modules/i18n/src/__tests__/nl-NL-registration.test.ts`:
- Line 14: The map callback for computing `codes` currently uses `(bundle:
any)`—replace `any` with `unknown` and add a runtime type guard for `bundle` to
ensure it's an object with a `meta` property that contains `code`; update the
callback signature to `(bundle: unknown)` and inside the mapper check `typeof
bundle === 'object' && bundle !== null && 'meta' in bundle` (or implement a
reusable type predicate like `isTranslationBundle`) before accessing
`bundle.meta.code`, returning the code or undefined accordingly so the types are
safe without using `any`.
In `@scripts/check-nl-properties.js`:
- Around line 31-40: Wrap the file read and inspect logic inside the
langFiles.forEach callback (where fs.readFileSync is called) with a try/catch so
IO errors are caught; on error, log a clear message including the file name and
the caught error, increment failed (so the script reflects the problem) and
continue to the next file, otherwise proceed to compute hasNl and update
passed/failed as now; ensure you reference the existing symbols
langFiles.forEach, fs.readFileSync, hasNl, passed and failed when making the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 456ad181-4c5e-4f20-89ac-e1ff457a68ba
📒 Files selected for processing (21)
.changeset/add-dutch-nl-NL-language-support.mdidentity-apps-core/apps/accounts/src/main/resources/LanguageOptions.propertiesidentity-apps-core/apps/accounts/src/main/resources/org/wso2/carbon/identity/application/accounts/endpoint/i18n/Resources_nl_NL.propertiesidentity-apps-core/apps/authentication-portal/src/main/resources/LanguageOptions.propertiesidentity-apps-core/apps/authentication-portal/src/main/resources/org/wso2/carbon/identity/application/authentication/endpoint/i18n/Resources_nl_NL.propertiesidentity-apps-core/apps/recovery-portal/src/main/resources/LanguageOptions.propertiesidentity-apps-core/apps/recovery-portal/src/main/resources/LocaleOptions.propertiesidentity-apps-core/apps/recovery-portal/src/main/resources/org/wso2/carbon/identity/mgt/recovery/endpoint/i18n/Resources_nl_NL.propertiesidentity-apps-core/apps/x509-certificate-authentication-portal/src/main/resources/LanguageOptions.propertiesidentity-apps-core/apps/x509-certificate-authentication-portal/src/main/resources/org/wso2/carbon/identity/application/authentication/endpoint/i18n/Resources_nl_NL.propertiesmodules/i18n/src/__tests__/nl-NL-completeness.test.tsmodules/i18n/src/__tests__/nl-NL-no-english-bleed.test.tsmodules/i18n/src/__tests__/nl-NL-registration.test.tsmodules/i18n/src/translations/index.tsmodules/i18n/src/translations/nl-NL/index.tsmodules/i18n/src/translations/nl-NL/meta.tsmodules/i18n/src/translations/nl-NL/portals/common-users.tsmodules/i18n/src/translations/nl-NL/portals/common.tsmodules/i18n/src/translations/nl-NL/portals/index.tsmodules/i18n/src/translations/nl-NL/portals/myaccount.tsscripts/check-nl-properties.js
Co-authored-by: Pavindu Lakshan <pavindulakshan@gmail.com>
pavinduLakshan
left a comment
pavinduLakshan
left a comment
There was a problem hiding this comment.
Let's add the license header to all newly added files, and address the review comments from coderabbit.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/i18n/src/__tests__/nl-NL-registration.test.ts`:
- Line 14: The test uses `any` and lacks an explicit type for `codes`; replace
`any` with a proper type and annotate `codes` explicitly: define or import an
interface like `TranslationBundle { meta?: { code?: string } }` (or use
`unknown` with a type guard) then change the map to
Object.values(translations).map((bundle: TranslationBundle) =>
bundle.meta?.code) and declare const codes: (string | undefined)[] = ... so
`bundle` is strongly typed and `codes` has an explicit type referencing
`meta?.code`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 994b700f-15db-4031-8c80-7f155d426517
📒 Files selected for processing (3)
modules/i18n/src/__tests__/nl-NL-completeness.test.tsmodules/i18n/src/__tests__/nl-NL-no-english-bleed.test.tsmodules/i18n/src/__tests__/nl-NL-registration.test.ts
✅ Files skipped from review due to trivial changes (2)
- modules/i18n/src/tests/nl-NL-no-english-bleed.test.ts
- modules/i18n/src/tests/nl-NL-completeness.test.ts
|
There is a typescript build failure due to the recently added new i18n. shall we include those as well in nl translations? |
|
completed the new push with the latest minor changes requested
Sorry for the minor issues. I am new to the WS02 workspace, i did this for the internship opportunity for SE. |
Thanks, I will take a look.
No worries, you're already well ahead of the level of most external contributions we typically receive :) |
Co-authored-by: Pavindu Lakshan <pavindulakshan@gmail.com>
|
Hi @Shazaanashraff, could you check and resolve https://github.com/wso2/identity-apps/pull/10304/changes#r3293825732 as well? Once done, we are good to merge. |
|
PS: I did some minor changes to the PR myself, so please take a pull before pushing your changes. |
|
The typecheck job is failing with the following errors.
This occurs because the nl translation doesn't have i18n keys for
Due to missing translations in NL translation file for
This occurs because the nl translation doesn't have i18n keys for |
|
changes:
all typechecks were passed locally |
Typecheck job still fails with the following error, due to missing translations for
Once you fix the issues locally, you can run |
|
ran the typecheck after the change |
|
brotherrrr. |
Nice!! Let me retrigger the CI and see. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10304 +/- ##
=======================================
Coverage 55.70% 55.70%
=======================================
Files 42 42
Lines 1016 1016
Branches 246 254 +8
=======================================
Hits 566 566
- Misses 416 450 +34
+ Partials 34 0 -34
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
hope this works |
pavinduLakshan
left a comment
pavinduLakshan
left a comment
There was a problem hiding this comment.
All tests passed, and local verification was successful as well, so I’m proceeding with the merge.
Thank you very much, @Shazaanashraff, for this contribution. This is one of the most requested features in the WSO2 Identity Server, and we really appreciate your contribution. Keep up the great work!
put up a word for me for that internship...... |
2 participants
Purpose
Adds complete out-of-the-box Dutch (
nl-NL) language support to all WSO2 Identity Server user-facing portals. Before this PR, Dutch was not available in the language switcher. This is a full re-implementation of #10273, addressing all gaps flagged in that review.What this PR does in plain terms:
modules/i18n/src/translations/nl-NL/.Resources_nl_NL.propertiesto each of the 4 portals — these are simple key=value text files that Java loads at runtime to serve translated strings.LanguageOptions.propertiesfile.LocaleOptions.propertiesin the recovery portal (controls the locale attribute dropdown in self-registration).NL_NLlocale export inmodules/i18n/src/translations/index.ts.Screenshots: Attached (language switcher showing Dutch, login page in Dutch, My Account dashboard in Dutch).
Related Issues
OOTB Support for Dutch language in Identity Server Frontend applications wso2/product-is#27782
Related PRs
#10273 (prior attempt — closed; this PR addresses all gaps flagged in that review)
Checklist
Manual test summary:
Downloaded WSO2 IS 7.3.0 from GitHub releases. Configured
deployment.tomlwith CORS and myaccount callback URL, deployed the built portal WARs, started IS, then ran the React dev server (pnpm --filter @wso2is/myaccount start). Logged in athttps://localhost:9000/myaccountwithadmin/admin.Verified:
@wso2is/i18n) ✓Unit tests added:
modules/i18n/src/__tests__/nl-NL-registration.test.ts— verifiesNL_NLexport exists with correct meta and is included insupportedI18nLanguagesmodules/i18n/src/__tests__/nl-NL-completeness.test.ts— verifies common and myAccount namespaces exist and matchen-UStop-level key structuremodules/i18n/src/__tests__/nl-NL-no-english-bleed.test.ts— spot-checks that key values are Dutch (Annuleren, Opslaan, Verwijderen, Zoeken, Afmelden)scripts/check-nl-properties.js— Node script confirming all 4Resources_nl_NL.propertiesfiles exist and all 4LanguageOptions.propertiescontain thenl_NLentryAll tests passed locally (
pnpm --filter @wso2is/i18n test).Security checks
Note for reviewer: A native Dutch speaker review is requested before merge to verify translation quality. No behavioural changes — this is purely additive (new locale files only). CI regression suite should pass unchanged.