#3154 Adding the Data Source name to the deletion confirmation message #3189
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThe changes implement dynamic data source deletion confirmation messages by updating the delete function to accept an element reference and constructing templated confirmation text that includes the data source name and XID. Supporting infrastructure includes new custom tag attributes and translated message keys across 11 language files. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webapp-resources/messages_fr.properties (1)
3456-3456: Character encoding corruption detected.The French text at line 3455 contains corrupted characters (� symbols) replacing accented characters. The text should be:
êtes-vous sûr de vouloir supprimer la source de données {0} (XID : {1}) ?Please correct the file encoding and restore the accented characters.
Suggested fix
-dsList.dsDeleteConfirmNamed=�tes-vous s�r de vouloir supprimer la source de donn�es {0} (XID : {1}) ? +dsList.dsDeleteConfirmNamed=êtes-vous sûr de vouloir supprimer la source de données {0} (XID : {1}) ?
🤖 Fix all issues with AI agents
In @webapp-resources/messages_fi.properties:
- Line 3586: The resource value for key dsList.dsDeleteConfirmNamed contains a
corrupted character ("tietol�hteen"); update the properties entry for
dsList.dsDeleteConfirmNamed to use the correct Finnish character encoding by
replacing the corrupted sequence with the proper escaped Unicode or character
(e.g., "tietol\u00e4hteen" or "tietolähteen") so the string becomes Haluatko
varmasti poistaa tietol\u00e4hteen {0} (XID: {1})?.
In @webapp-resources/messages_pl.properties:
- Line 3597: The property validate.unsupportedDataType currently contains
English text; replace "Data type {0} is not supported" with an accurate Polish
translation (keeping the placeholder {0} intact) so the
validate.unsupportedDataType entry in messages_pl.properties is in Polish.
- Line 3598: The property value for dsList.dsDeleteConfirmNamed contains
corrupted Polish characters; replace the garbled text with proper Unicode
escapes so the value reads "Czy na pewno chcesz usunąć źródło danych {0} (XID:
{1})?" — specifically use \u0105 for 'ą', \u0107 for 'ć', \u017A for 'ź', \u00F3
for 'ó' and \u0142 for 'ł' when updating the dsList.dsDeleteConfirmNamed entry.
🧹 Nitpick comments (3)
webapp-resources/messages_fr.properties (1)
3455-3455: English text in French properties file.This key contains English text in a French translation file. Based on learnings, French is not actively maintained in SCADA-LTS, so English fallback text is acceptable. However, note that this key appears unrelated to the PR's stated objective of adding data source names to deletion confirmations.
Based on learnings, SCADA-LTS maintains only Polish and English translations; other locales may contain English text and are not blocking.
webapp-resources/messages_en.properties (1)
3461-3461: LGTM! The new deletion confirmation message improves user experience.The message template correctly uses placeholders for the data source name {0} and XID {1}, providing clearer context to users before deletion. The grammar and formatting are appropriate.
Optional: Consider improved organization
For better maintainability, consider placing this key near the other
dsList.*keys (around lines 1241-1251) rather than at the end of the file. While the current placement is functional, grouping related keys makes the properties file easier to navigate and maintain.+# Place near line 1244 after dsList.dsDeleteConfirm instead of at the endWebContent/WEB-INF/jsp/dataSourceList.jsp (1)
65-82: Implementation looks correct.The approach of passing the element reference and extracting
data-name/data-xidattributes to build a dynamic confirmation message works well. The fallback to empty string (|| "") handles missing attributes gracefully.Minor observation: There's inconsistent indentation between the variable declarations (lines 66-69, indented 6 spaces) and the
ifblock (lines 70-81, indented 8 spaces). Consider aligning for consistency.🔧 Optional: Consistent indentation
function deleteDataSource(el, dataSourceId) { - var name = el.getAttribute("data-name") || ""; - var xid = el.getAttribute("data-xid") || ""; - var template = "<spring:message code='dsList.dsDeleteConfirmNamed'/>"; - var msg = template.replace("{0}", name).replace("{1}", xid); - if (confirm(msg)) { + var name = el.getAttribute("data-name") || ""; + var xid = el.getAttribute("data-xid") || ""; + var template = "<spring:message code='dsList.dsDeleteConfirmNamed'/>"; + var msg = template.replace("{0}", name).replace("{1}", xid); + if (confirm(msg)) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
WebContent/WEB-INF/jsp/dataSourceList.jspWebContent/WEB-INF/tags/img.tagwebapp-resources/messages_de.propertieswebapp-resources/messages_en.propertieswebapp-resources/messages_es.propertieswebapp-resources/messages_fi.propertieswebapp-resources/messages_fr.propertieswebapp-resources/messages_lu.propertieswebapp-resources/messages_nl.propertieswebapp-resources/messages_pl.propertieswebapp-resources/messages_pt.propertieswebapp-resources/messages_ru.propertieswebapp-resources/messages_zh.properties
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-05T14:39:51.606Z
Learnt from: Limraj
Repo: SCADA-LTS/Scada-LTS PR: 3239
File: webapp-resources/messages_fi.properties:3565-3565
Timestamp: 2026-01-05T14:39:51.606Z
Learning: SCADA-LTS maintains translations only for Polish and English. Do not invest in or expect active maintenance for other languages (e.g., messages_fi.properties, messages_de.properties). If a messages_*.properties file contains English text and is not maintained, it can be left as-is for now. AI translation may be used in the future. During reviews, prioritize Polish and English keys/text; non-maintained locales should not block changes.
Applied to files:
webapp-resources/messages_fi.propertieswebapp-resources/messages_fr.propertieswebapp-resources/messages_de.propertieswebapp-resources/messages_pt.propertieswebapp-resources/messages_pl.propertieswebapp-resources/messages_nl.propertieswebapp-resources/messages_es.propertieswebapp-resources/messages_zh.propertieswebapp-resources/messages_lu.propertieswebapp-resources/messages_en.propertieswebapp-resources/messages_ru.properties
🔇 Additional comments (10)
webapp-resources/messages_nl.properties (1)
3576-3576: LGTM! Translation key added correctly.The new
dsList.dsDeleteConfirmNamedkey is properly formatted with placeholders{0}(data source name) and{1}(XID), consistent with the pattern used in other locales. The Dutch translation appears grammatically correct and aligns with the existing confirmation message at Line 375.Note: Based on learnings, Dutch (messages_nl.properties) is not actively maintained—only Polish and English translations are prioritized. This addition is acceptable and should not block the PR.
webapp-resources/messages_zh.properties (1)
3561-3561: English text in Chinese translation file (acceptable per project policy)The new key
dsList.dsDeleteConfirmNamedcontains English text instead of Chinese translation. While this is inconsistent with the file's purpose, it's acceptable according to the project's translation policy.Based on learnings, SCADA-LTS actively maintains only Polish and English translations. Other language files like
messages_zh.propertiesare not maintained and can contain English text as placeholders. AI translation may be used in the future to provide proper Chinese translations.The key format and placeholders
{0}(name) and{1}(XID) are correct and align with the PR objectives to include the data source name in the deletion confirmation message.webapp-resources/messages_pt.properties (1)
3613-3613: LGTM! Translation looks correct.The new deletion confirmation message key
dsList.dsDeleteConfirmNamedhas been added with proper Portuguese translation and correctly formatted placeholders {0} and {1} for the data source name and XID respectively.Based on learnings, Portuguese is not an actively maintained locale in SCADA-LTS (only Polish and English are prioritized), but the translation appears grammatically correct and aligns with the PR objectives.
webapp-resources/messages_de.properties (1)
3457-3458: LGTM! Changes correctly implement the dynamic deletion confirmation message.The additions implement the feature as expected:
- Line 3457 adds a validation message key (in English, which is acceptable for this non-maintained locale)
- Line 3458 adds the new
dsList.dsDeleteConfirmNamedkey with German translation using correct placeholders {0} for data source name and {1} for XIDBased on learnings, SCADA-LTS maintains translations only for Polish and English. The German locale is not actively maintained and should not block this change.
webapp-resources/messages_ru.properties (1)
3608-3609: All verification checks passed—no blocking issues found.
- ✓
validate.unsupportedDataTypeis not duplicated in the file; appears only once at line 3608.- ✓
dsList.dsDeleteConfirmNamedplaceholders are correct—the JSP caller (dataSourceList.jsp) supplies both{0}(data source name) and{1}(XID) before passing the message toconfirm().- ✓ English text in
messages_ru.propertiesaligns with project practice for non-maintained locales (consistent withmessages_zh.properties).webapp-resources/messages_lu.properties (1)
3474-3474: Character encoding issue is a known pattern in non-maintained translation files.Line 3474 contains corrupted special characters (e.g., s�cher, d�i, l�schen, w�llt instead of proper Luxembourgish characters like sécher, déi, läschen, wëllt). However, this is consistent with a systemic encoding issue affecting multiple non-maintained language files (German, Finnish, French, Spanish, Polish—all showing similar corruption patterns), while properly maintained languages like Portuguese and Dutch display characters correctly.
Per project guidelines, SCADA-LTS actively maintains only Polish and English translations. Other language files are not actively maintained and may be regenerated using AI translation in the future. This encoding issue is expected in those files and does not require changes in this PR.
webapp-resources/messages_es.properties (2)
3500-3500: English text in Spanish translation file (acceptable per project policy).The key
validate.unsupportedDataTypecontains English text instead of Spanish. However, this is acceptable since Spanish translations are not actively maintained in this project.Based on learnings, SCADA-LTS maintains translations only for Polish and English. Non-maintained locales like Spanish can be left as-is until AI translation is implemented in the future.
3501-3501: LGTM! Proper Spanish translation for the new deletion confirmation.The new translation key
dsList.dsDeleteConfirmNamedis correctly translated to Spanish with proper grammar and placeholder usage. The question format follows Spanish conventions with the opening ¿ character, and the placeholders {0} and {1} are appropriately positioned for the data source name and XID.WebContent/WEB-INF/tags/img.tag (1)
28-29: LGTM!The new
dataNameanddataXidattributes are correctly declared and rendered. Good use of<c:out>for XSS protection when outputting attribute values.Also applies to: 38-39
WebContent/WEB-INF/jsp/dataSourceList.jsp (1)
159-164: LGTM!The
tag:imgcorrectly suppliesdataNameanddataXidattributes, and theonclickhandler properly passesthisto provide the element reference. This cleanly integrates with the updateddeleteDataSourcefunction.
| systemSettings.scadaConfCheck=Check configuration | ||
| validate.unsupportedDataType=Data type {0} is not supported No newline at end of file | ||
| validate.unsupportedDataType=Data type {0} is not supported | ||
| dsList.dsDeleteConfirmNamed=Haluatko varmasti poistaa tietol�hteen {0} (XID: {1})? |
There was a problem hiding this comment.
Character encoding issue in Finnish translation.
The text tietol�hteen appears to have a corrupted character. This should likely be tietolähteen (with the proper Finnish 'ä' character encoded as \u00e4):
dsList.dsDeleteConfirmNamed=Haluatko varmasti poistaa tietol\u00e4hteen {0} (XID: {1})?Based on learnings, Finnish is not actively maintained (only Polish and English are), so this is non-blocking.
🤖 Prompt for AI Agents
In @webapp-resources/messages_fi.properties at line 3586, The resource value for
key dsList.dsDeleteConfirmNamed contains a corrupted character ("tietol�hteen");
update the properties entry for dsList.dsDeleteConfirmNamed to use the correct
Finnish character encoding by replacing the corrupted sequence with the proper
escaped Unicode or character (e.g., "tietol\u00e4hteen" or "tietolähteen") so
the string becomes Haluatko varmasti poistaa tietol\u00e4hteen {0} (XID: {1})?.
| systemSettings.scadaConfTitle=Scada-LTS environment settings | ||
| systemSettings.scadaConfCheck=Check configuration | ||
| validate.unsupportedDataType=Data type {0} is not supported No newline at end of file | ||
| validate.unsupportedDataType=Data type {0} is not supported |
There was a problem hiding this comment.
English text in Polish translation file.
This message is in English but placed in the Polish translation file. It should be translated to Polish for consistency.
Suggested Polish translation
-validate.unsupportedDataType=Data type {0} is not supported
+validate.unsupportedDataType=Typ danych {0} nie jest obs\u0142ugiwany📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| validate.unsupportedDataType=Data type {0} is not supported | |
| validate.unsupportedDataType=Typ danych {0} nie jest obs\u0142ugiwany |
🤖 Prompt for AI Agents
In @webapp-resources/messages_pl.properties at line 3597, The property
validate.unsupportedDataType currently contains English text; replace "Data type
{0} is not supported" with an accurate Polish translation (keeping the
placeholder {0} intact) so the validate.unsupportedDataType entry in
messages_pl.properties is in Polish.
| systemSettings.scadaConfCheck=Check configuration | ||
| validate.unsupportedDataType=Data type {0} is not supported No newline at end of file | ||
| validate.unsupportedDataType=Data type {0} is not supported | ||
| dsList.dsDeleteConfirmNamed=Czy na pewno chcesz usun?? ?r�d?o danych {0} (XID: {1})? |
There was a problem hiding this comment.
Character encoding corruption in Polish text.
The Polish characters are corrupted: usun?? should be usunąć and ?r�d?o should be źródło. Use proper Unicode escapes as seen elsewhere in this file (e.g., line 359 uses \u0179r\u00f3d\u0142o for "źródło").
Suggested fix with proper Unicode escapes
-dsList.dsDeleteConfirmNamed=Czy na pewno chcesz usun?? ?r�d?o danych {0} (XID: {1})?
+dsList.dsDeleteConfirmNamed=Czy na pewno chcesz usun\u0105\u0107 \u017ar\u00f3d\u0142o danych {0} (XID: {1})?📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dsList.dsDeleteConfirmNamed=Czy na pewno chcesz usun?? ?r�d?o danych {0} (XID: {1})? | |
| dsList.dsDeleteConfirmNamed=Czy na pewno chcesz usun\u0105\u0107 \u017ar\u00f3d\u0142o danych {0} (XID: {1})? |
🤖 Prompt for AI Agents
In @webapp-resources/messages_pl.properties at line 3598, The property value for
dsList.dsDeleteConfirmNamed contains corrupted Polish characters; replace the
garbled text with proper Unicode escapes so the value reads "Czy na pewno chcesz
usunąć źródło danych {0} (XID: {1})?" — specifically use \u0105 for 'ą', \u0107
for 'ć', \u017A for 'ź', \u00F3 for 'ó' and \u0142 for 'ł' when updating the
dsList.dsDeleteConfirmNamed entry.
Now pop up window with confirmation looks like this:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.