Skip to content

Enhance translation processing to ensure only usable versions are con…#21

Merged
MrHinsh merged 1 commit intomainfrom
fix-for-wrapper-translation-iwth-no-content
Jan 12, 2026
Merged

Enhance translation processing to ensure only usable versions are con…#21
MrHinsh merged 1 commit intomainfrom
fix-for-wrapper-translation-iwth-no-content

Conversation

@MrHinsh
Copy link
Member

@MrHinsh MrHinsh commented Jan 12, 2026

…sidered

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved translation filtering to include only translations with content available online or as a PDF.
    • Enhanced version comparison logic to ensure newer unavailable translations do not overwrite existing usable language versions.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings January 12, 2026 15:59
@MrHinsh MrHinsh enabled auto-merge January 12, 2026 15:59
@coderabbitai
Copy link

coderabbitai bot commented Jan 12, 2026

Walkthrough

A usability filter is introduced that gates translation processing, ensuring only translations with available online or PDF content are included when determining the latest version per language. The existing version-comparison logic remains intact but now operates exclusively on usable translations.

Changes

Cohort / File(s) Summary
Translation usability filtering
module/layouts/_partials/functions/get-guide-translations-list.html
Adds conditional gating to process only translations with available content (online or PDF). Introduces isNewer flag logic to prevent non-usable newer translations from overwriting existing usable ones. Maintains version-compare (YYYY.M) logic for language-specific latest version tracking.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~23 minutes

Poem

📚 Translations filtered with pragmatic care,
Only the usable make it through there,
Versions compared with discipline's hand,
No dead links corrupt the promised land! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Enhance translation processing to ensure only usable versions are con…' directly aligns with the main change: filtering translations to process only usable ones (with online or PDF content available).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-for-wrapper-translation-iwth-no-content

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.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request enhances the translation processing logic to ensure only usable translations (those with online content or PDF available) are considered when determining the latest version for each language.

Changes:

  • Added usability check to filter translations based on ReadOnline or ReadPDF availability
  • Updated comment to clarify that the function finds the "latest usable version" rather than just "latest version"
  • Restructured code to nest version comparison logic within the usability check

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
module/layouts/_partials/functions/get-guide-translations-list.html (3)

36-71: Logic is sound, but consider defensive handling for malformed versions.

The version parsing assumes a well-formed YYYY.M string with exactly two parts. If a version string is malformed (e.g., "2024" without a month, or "2024.1.2"), index $currentParts 1 could yield an empty string or unexpected value, and int on an empty string may produce 0 silently in Hugo.

This isn't a critical issue if upstream data is always well-formed, but defensive coding would be prudent here.

🛡️ Optional: Add a guard before parsing
          {{/* Parse versions for comparison (YYYY.M format) */}}
          {{ $currentParts := split $currentVersion "." }}
          {{ $existingParts := split $existingVersion "." }}

+         {{/* Only compare if both versions have expected format */}}
+         {{ if and (ge (len $currentParts) 2) (ge (len $existingParts) 2) }}
            {{ $currentYear := int (index $currentParts 0) }}
            {{ $currentMonth := int (index $currentParts 1) }}
            {{ $existingYear := int (index $existingParts 0) }}
            {{ $existingMonth := int (index $existingParts 1) }}
            
            {{ $isNewer := false }}
            {{ if gt $currentYear $existingYear }}
              {{ $isNewer = true }}
            {{ else if and (eq $currentYear $existingYear) (gt $currentMonth $existingMonth) }}
              {{ $isNewer = true }}
            {{ end }}

            {{ if $isNewer }}
              {{/* Update with newer version only if it's usable */}}
              {{ $updatedTranslation := merge . (dict "Version" $currentVersion) }}
              {{ $languageMap = merge $languageMap (dict $langCode $updatedTranslation) }}
            {{ end }}
+         {{ end }}

65-69: Comment on line 66 is slightly misleading.

The comment says "Update with newer version only if it's usable", but by this point we're already inside the {{ if $isUsable }} block (line 37), so usability is already guaranteed. The comment is redundant and could cause confusion during future maintenance. Consider simplifying to just "Update with newer version".

📝 Suggested comment fix
          {{ if $isNewer }}
-           {{/* Update with newer version only if it's usable */}}
+           {{/* Update with newer version */}}
            {{ $updatedTranslation := merge . (dict "Version" $currentVersion) }}
            {{ $languageMap = merge $languageMap (dict $langCode $updatedTranslation) }}
          {{ end }}

98-99: Sorting by Weight alone may yield non-deterministic order for equal weights.

If multiple translations share the same Weight, their relative order is undefined. Consider adding a secondary sort key (e.g., language code) for consistent, reproducible output.

🔄 Suggested secondary sort

Hugo's sort doesn't natively support multiple keys in one call, but you could do:

-{{ $translationsList = sort $translationsList "Weight" "asc" }}
+{{/* Sort by weight, then by language code for deterministic output */}}
+{{ $translationsList = sort $translationsList "Language" "asc" }}
+{{ $translationsList = sort $translationsList "Weight" "asc" }}

Note: Hugo's sort is stable, so sorting by language first, then by weight, will preserve language order within equal weights.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ded7526 and 788248a.

📒 Files selected for processing (1)
  • module/layouts/_partials/functions/get-guide-translations-list.html
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: nkdAgility/HugoGuides PR: 0
File: .github/instructions/module.instructions.md:0-0
Timestamp: 2026-01-07T18:58:56.412Z
Learning: Support PDF-only translations for any guide or version, allowing translated content without requiring a full web version
Learnt from: CR
Repo: nkdAgility/HugoGuides PR: 0
File: .github/instructions/module.instructions.md:0-0
Timestamp: 2026-01-07T18:58:56.412Z
Learning: Allow individual versions of a guide to be translated independently, without requiring other versions or other guides to be translated
📚 Learning: 2026-01-07T18:58:56.412Z
Learnt from: CR
Repo: nkdAgility/HugoGuides PR: 0
File: .github/instructions/module.instructions.md:0-0
Timestamp: 2026-01-07T18:58:56.412Z
Learning: Support PDF-only translations for any guide or version, allowing translated content without requiring a full web version

Applied to files:

  • module/layouts/_partials/functions/get-guide-translations-list.html
📚 Learning: 2026-01-07T18:58:56.412Z
Learnt from: CR
Repo: nkdAgility/HugoGuides PR: 0
File: .github/instructions/module.instructions.md:0-0
Timestamp: 2026-01-07T18:58:56.412Z
Learning: Allow individual versions of a guide to be translated independently, without requiring other versions or other guides to be translated

Applied to files:

  • module/layouts/_partials/functions/get-guide-translations-list.html
📚 Learning: 2026-01-07T18:58:56.412Z
Learnt from: CR
Repo: nkdAgility/HugoGuides PR: 0
File: .github/instructions/module.instructions.md:0-0
Timestamp: 2026-01-07T18:58:56.412Z
Learning: Module should enable organizations and communities to deliver guides in multiple languages

Applied to files:

  • module/layouts/_partials/functions/get-guide-translations-list.html
⏰ 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). (1)
  • GitHub Check: Agent
🔇 Additional comments (2)
module/layouts/_partials/functions/get-guide-translations-list.html (2)

33-34: Usability check aligns with requirements — good.

The $isUsable check correctly gates on either ReadOnline or ReadPDF being true, which matches the learnings about supporting PDF-only translations. The explicit eq .ReadOnline true comparisons are appropriately strict for Hugo's boolean handling.


76-93: The fallback version extraction code is genuinely unreachable dead weight.

All entries in $languageMap get added via exactly two paths (lines 42-43 and 67-68), both of which explicitly merge with dict "Version" $version or dict "Version" $currentVersion. There's no way for an entry to land in the map without a Version field already set. The defensive code at lines 80-92 will never execute.

If you want to keep it for future-proofing (and there's nothing wrong with that), just document it clearly as such. But if it's just hanging there unloved, remove it — dead code breeds confusion.

@MrHinsh MrHinsh added this pull request to the merge queue Jan 12, 2026
Merged via the queue into main with commit 01bebc2 Jan 12, 2026
12 checks passed
@MrHinsh MrHinsh deleted the fix-for-wrapper-translation-iwth-no-content branch January 20, 2026 14:11
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.

1 participant