Skip to content

Wizard: prevent duplicate languages in Locale step (HMS-10599)#4527

Open
mgold1234 wants to merge 1 commit into
osbuild:mainfrom
mgold1234:validate_local
Open

Wizard: prevent duplicate languages in Locale step (HMS-10599)#4527
mgold1234 wants to merge 1 commit into
osbuild:mainfrom
mgold1234:validate_local

Conversation

@mgold1234

@mgold1234 mgold1234 commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

replaceLanguage did not guard against duplicates, and LanguageRow
used key={lang} which caused React warnings when duplicates arrived
from an imported blueprint.

  • Add duplicate guard to replaceLanguage
  • Fix list key to handle duplicate entries
  • Add tests for duplicate language validation

Why is this needed?

The duplicate language validation and error message were already
implemented in a previous commit, but had no test coverage.
Additionally, replaceLanguage did not prevent creating duplicates
at the reducer level, and React would log warnings about duplicate
keys when a blueprint with duplicate languages was loaded from the API.

These changes harden the existing validation by preventing duplicates
at the source and ensuring the rendering handles them gracefully.

To reproduce

Create a blueprint via API with duplicate languages:

fetch('/api/image-builder/v1/blueprints', {
  method: 'POST',
  headers: { 'Content-Type': 'application/json' },
  body: JSON.stringify({
    name: "test-duplicate-languages",
    distribution: "rhel-9",
    image_requests: [{ architecture: "x86_64", image_type: "guest-image",
      upload_request: { type: "aws.s3", options: {} } }],
    customizations: { locale: { languages: ["en_US.UTF-8", "en_US.UTF-8", "C.UTF-8"] } }
  })
}).then(r => r.json()).then(console.log)

Then open the blueprint in the wizard — the Locale step shows
"Duplicated languages: en_US.UTF-8" and blocks the Next button.

JIRA: HMS-10599

@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 75.64%. Comparing base (6044553) to head (4216b54).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
...zard/steps/Locale/components/LanguagesDropDown.tsx 83.33% 1 Missing ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4527      +/-   ##
==========================================
+ Coverage   75.62%   75.64%   +0.01%     
==========================================
  Files         250      249       -1     
  Lines        7159     7152       -7     
  Branches     2655     2655              
==========================================
- Hits         5414     5410       -4     
+ Misses       1503     1500       -3     
  Partials      242      242              
Files with missing lines Coverage Δ
...zard/steps/Locale/components/LanguagesDropDown.tsx 92.98% <83.33%> (+4.09%) ⬆️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6044553...4216b54. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mgold1234 mgold1234 marked this pull request as ready for review June 15, 2026 10:25
@mgold1234 mgold1234 requested a review from a team as a code owner June 15, 2026 10:25

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Using index as part of the React key in LanguagesDropDown can still cause subtle rendering bugs when items are reordered or removed; consider using a stable key such as ${lang}-${occurrenceIndex} where occurrenceIndex is derived from counting occurrences of the same language up to that point.
  • In replaceLanguage, the duplicate guard logic is a bit dense; extracting the isDuplicate check into a small helper function (with a short comment explaining the oldLanguage exception) would make the intent clearer and reduce chances of future regressions when this logic is modified.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Using `index` as part of the React key in `LanguagesDropDown` can still cause subtle rendering bugs when items are reordered or removed; consider using a stable key such as `${lang}-${occurrenceIndex}` where `occurrenceIndex` is derived from counting occurrences of the same language up to that point.
- In `replaceLanguage`, the duplicate guard logic is a bit dense; extracting the `isDuplicate` check into a small helper function (with a short comment explaining the `oldLanguage` exception) would make the intent clearer and reduce chances of future regressions when this logic is modified.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/store/slices/wizard/index.ts Outdated
Comment thread src/Components/CreateImageWizard/steps/Locale/tests/Locale.test.tsx
@mgold1234 mgold1234 force-pushed the validate_local branch 4 times, most recently from 3efaeac to 671a7b0 Compare June 17, 2026 11:53
kingsleyzissou
kingsleyzissou previously approved these changes Jun 17, 2026

@kingsleyzissou kingsleyzissou left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good. I added some nitpicks inline, but we could do those in a follow up.

Comment thread src/Components/CreateImageWizard/steps/Locale/tests/Locale.test.tsx Outdated
Comment thread src/Components/CreateImageWizard/steps/Locale/tests/Locale.test.tsx Outdated
Comment thread src/Components/CreateImageWizard/steps/Locale/tests/Locale.test.tsx Outdated
`replaceLanguage` did not guard against duplicates, and `LanguageRow`
used `key={lang}` which caused React warnings when duplicates arrived
from an imported blueprint.

- Add duplicate guard to `replaceLanguage`
- Fix list key to handle duplicate entries
- Add tests for duplicate language validation

To reproduce: create a blueprint via API with
`languages: ["en_US.UTF-8", "en_US.UTF-8"]` and open it in the wizard.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants