feat: update report name label and add BIRT translation (WEB-821)#3383
Conversation
|
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:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Reports Component Template src/app/reports/reports.component.html |
Changed the "reportName" column header translation from labels.inputs.name to labels.text.BIRT and adjusted header content/line breaks. |
Locale translations (added/updated keys) src/assets/translations/en-US.json, src/assets/translations/cs-CS.json, src/assets/translations/de-DE.json, src/assets/translations/es-CL.json, src/assets/translations/es-MX.json, src/assets/translations/fr-FR.json, src/assets/translations/it-IT.json, src/assets/translations/ko-KO.json, src/assets/translations/lt-LT.json, src/assets/translations/lv-LV.json, src/assets/translations/ne-NE.json, src/assets/translations/pt-PT.json, src/assets/translations/sw-SW.json |
Added labels.BIRT (usually mapping to "Mifos Reporting Plugin for Eclipse Birt"), sometimes added labels["Mifos Reporting Plugin for Eclipse Birt"], and updated labels.Pentaho from an empty string to "Mifos Reporting Plugin for Pentaho" across locales. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
- WEB-797 Implement Eclipse BIRT reporting support #3279 — overlaps with BIRT UI text and translations added here.
- WEB-756 Fix the web application to remove text values from pentaho fixed fields #3185 — modifies the same
labels.Pentahotranslation keys across locales. - WEB-521 fix(i18n): add missing translations and apply translate pipes across web-app #2934 — recent localization/template changes touching report UI translations.
Suggested reviewers
- IOhacker
- gkbishnoi07
🚥 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 clearly describes the main changes: updating the report name label and adding BIRT translation, with ticket reference WEB-821. |
| 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
📝 Coding Plan
- Generate coding plan for human review comments
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 help to get the list of available commands and usage tips.
Tip
CodeRabbit can approve the review once all CodeRabbit's comments are resolved.
Enable the reviews.request_changes_workflow setting to automatically approve the review once all CodeRabbit's comments are resolved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/assets/translations/en-US.json`:
- Line 292: Add the new translation key "Mifos Reporting Plugin for Eclipse
Birt" to every locale JSON (cs-CS, de-DE, es-CL, es-MX, fr-FR, it-IT, ko-KO,
lt-LT, lv-LV, ne-NE, pt-PT, sw-SW) so non-en-US users see the intended label
used in reports.component.html; either provide proper localized strings or copy
the en-US value for now, and keep the key name identical to the one added in
en-US.json. Also verify the CustomMissingTranslationHandler still behaves as
expected (it currently strips labels.catalogs. and returns the key) and if you
prefer runtime fallback to en-US instead, implement that fallback in
CustomMissingTranslationHandler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4643684d-daea-413f-8722-725ce56a4ea9
📒 Files selected for processing (2)
src/app/reports/reports.component.htmlsrc/assets/translations/en-US.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/assets/translations/ko-KO.json`:
- Line 291: The translation for the key "Mifos Reporting Plugin for Eclipse
Birt" is still in English; replace its value with an appropriate Korean
translation (e.g., a concise Korean phrase conveying "Mifos Reporting Plugin for
Eclipse BIRT") in src/assets/translations/ko-KO.json by updating the value for
that exact key, then run Prettier (npx prettier --write .) to ensure formatting
before committing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 870a9f3b-378a-410b-bf7f-e94194626b6f
📒 Files selected for processing (12)
src/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
✅ Files skipped from review due to trivial changes (3)
- src/assets/translations/de-DE.json
- src/assets/translations/it-IT.json
- src/assets/translations/es-CL.json
vaishnavirrapolu-06
force-pushed
the
feat/web-821-report-details
branch
from
f54acb2 to
7c5b3de
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/assets/translations/en-US.json (1)
292-292: Consider removing the redundant top-level BIRT string key.Line 292 adds
labels["Mifos Reporting Plugin for Eclipse Birt"], but current report rendering useslabels.text.BIRT/labels.text.<reportType>. If there’s no consumer for this top-level key, removing it would avoid translation-path drift.♻️ Suggested cleanup
- "Mifos Reporting Plugin for Eclipse Birt": "Mifos Reporting Plugin for Eclipse Birt",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/en-US.json` at line 292, Remove the redundant top-level translation key "Mifos Reporting Plugin for Eclipse Birt" from the en-US JSON and ensure the canonical report key is only present under the nested path used by rendering (e.g., labels.text.BIRT / labels.text.<reportType>); delete the top-level "Mifos Reporting Plugin for Eclipse Birt" entry and, if needed, consolidate its value under labels.text.BIRT so there’s a single source of truth for the BIRT label.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/assets/translations/en-US.json`:
- Line 292: Remove the redundant top-level translation key "Mifos Reporting
Plugin for Eclipse Birt" from the en-US JSON and ensure the canonical report key
is only present under the nested path used by rendering (e.g., labels.text.BIRT
/ labels.text.<reportType>); delete the top-level "Mifos Reporting Plugin for
Eclipse Birt" entry and, if needed, consolidate its value under labels.text.BIRT
so there’s a single source of truth for the BIRT label.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f4fe97c9-b1f7-45c2-a7dc-6e0d89fc412f
📒 Files selected for processing (14)
src/app/reports/reports.component.htmlsrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (10)
- src/assets/translations/fr-FR.json
- src/assets/translations/es-MX.json
- src/assets/translations/ne-NE.json
- src/assets/translations/es-CL.json
- src/app/reports/reports.component.html
- src/assets/translations/sw-SW.json
- src/assets/translations/ko-KO.json
- src/assets/translations/lt-LT.json
- src/assets/translations/de-DE.json
- src/assets/translations/lv-LV.json
|
Hi @IOhacker , I've completed the requested changes. All checks are now green, including the Single Commit requirement. I have:
Ready for final review! |
|
@vaishnavirrapolu-06 kindly review my comments |
|
@lohacker, Thank you very much for reviewing my PR and for the feedback! I have noted your 4 comments. I will be at college during the day tomorrow, but I will implement all the requested changes and update the PR as soon as I return in the evening. Looking forward to getting this perfected! 😊 |
|
@vaishnavirrapolu-06 thank you for your response |
vaishnavirrapolu-06
force-pushed
the
feat/web-821-report-details
branch
from
7c5b3de to
3cbff39
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/app/reports/reports.component.html (1)
20-22:⚠️ Potential issue | 🟠 MajorUse a generic header for
reportNameat Line 21.Line 21 hardcodes a BIRT-specific label for a generic report-name column, which can mislead users when rows include Pentaho/other report types.
Suggested fix
- {{ 'labels.text.BIRT' | translate }} + {{ 'labels.inputs.name' | translate }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/reports/reports.component.html` around lines 20 - 22, The header for the report-name column is using a BIRT-specific translation key ('labels.text.BIRT') in the <th mat-header-cell *matHeaderCellDef mat-sort-header> element, which should be a generic label; update that translation key to a neutral one (e.g. 'labels.text.reportName' or 'labels.text.report') so the header reflects any report type and keep the existing mat-sort-header and *matHeaderCellDef on the same <th> element (locate the <th mat-header-cell *matHeaderCellDef mat-sort-header> / reportName column in reports.component.html and replace the specific key with the generic key).src/assets/translations/es-MX.json (1)
291-291:⚠️ Potential issue | 🟡 MinorLocalize the ES-MX value at Line 291.
Line 291 still keeps the value in English, so this key can show mixed-language UI in the
es-MXlocale.Suggested fix
- "Mifos Reporting Plugin for Eclipse Birt": "Mifos Reporting Plugin for Eclipse Birt", + "Mifos Reporting Plugin for Eclipse Birt": "Mifos Plugin para generar informes usando Eclipse Birt",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-MX.json` at line 291, The ES-MX translation for the key "Mifos Reporting Plugin for Eclipse Birt" is still in English; update the value for that JSON key to a proper Spanish (Mexico) localization (for example: "Complemento de informes de Mifos para Eclipse BIRT") so the es-MX locale does not show mixed-language UI.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/app/reports/reports.component.html`:
- Around line 20-22: The header for the report-name column is using a
BIRT-specific translation key ('labels.text.BIRT') in the <th mat-header-cell
*matHeaderCellDef mat-sort-header> element, which should be a generic label;
update that translation key to a neutral one (e.g. 'labels.text.reportName' or
'labels.text.report') so the header reflects any report type and keep the
existing mat-sort-header and *matHeaderCellDef on the same <th> element (locate
the <th mat-header-cell *matHeaderCellDef mat-sort-header> / reportName column
in reports.component.html and replace the specific key with the generic key).
In `@src/assets/translations/es-MX.json`:
- Line 291: The ES-MX translation for the key "Mifos Reporting Plugin for
Eclipse Birt" is still in English; update the value for that JSON key to a
proper Spanish (Mexico) localization (for example: "Complemento de informes de
Mifos para Eclipse BIRT") so the es-MX locale does not show mixed-language UI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e48f50e9-6db9-4eee-8a9c-71c969f1e993
📒 Files selected for processing (14)
src/app/reports/reports.component.htmlsrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (8)
- src/assets/translations/ne-NE.json
- src/assets/translations/cs-CS.json
- src/assets/translations/lt-LT.json
- src/assets/translations/lv-LV.json
- src/assets/translations/es-CL.json
- src/assets/translations/pt-PT.json
- src/assets/translations/ko-KO.json
- src/assets/translations/en-US.json
|
Hi @lohacker! I've updated the logic in reports.component.html to dynamically handle both BIRT and Pentaho report types. I've also updated es-MX.json with the Spanish translations you provided for both types. The PR is updated (still a single commit). Ready for your final review! 😊 |
2 participants
Hi @IOhacker Sir!
Following your feedback, I have moved this task to the correct openMF/web-app repository.
Summary of changes:
Thank you for your guidance!
Summary by CodeRabbit
Style
Chores (Localization)