[Amormio25]

Description

This PR fixes previous issues within the CSV upload service where strings were being returned untranslated to users. It builds upon an abandoned PR #1571, creating:

• server-side helpers for translation based on the attached Accept-Language header in the request object
• a map of translations in English, Spanish, and French

and utilizes the language determined by the helpers (English default) to be passed as a parameter to services for translating strings.

Co-authored-by: Khaterina Senchareune @skhaterina

Fixes #1732, Fixes #703

How changes were tested:

• manual verification running my development environment
• I ran npm run test and some tests failed (e.g. readings for 3d charts) but did not seem to be as a result of these changes.

Screenshots

The above text after the success tag I believe is a Node.js error and not something we can control and translate.

The above shows the translated error and the logged message afterwards.

Type of change

• Note merging this changes the database configuration.
• This change requires a documentation update

Checklist

• I have followed the OED pull request ideas
• I have removed text in ( ) from the issue request
• You acknowledge that every person contributing to this work has signed the OED Contributing License Agreement and each author is listed in the Description section. This checkbox is required.
• By submitting this PR I agree to the OED AI policy specified below and acknowledge that I followed its terms in this submission. This checkbox is required.

Limitations

Note: This PR doesn't address the unformatted toast error messages (examples in the screenshots). That is out of scope although I'd be willing to work on that. I also did not see any existing tests for translations, and so did not implement my own.

AI Disclosure

List all AI tools used including version(s). You can submit without the versions but OED may request further information before accepting the PR.

Composer 2.5, Claude Sonnet 4.6

If used, describe how each AI tool was used including giving the prompts used. You can submit without the prompts but OED may request further information before accepting the PR.

• As my first time contributing, I used Composer 2.5 to help with any problems I had setting up my development environment including:
  • help with docker commands and logging in as a test admin
  • cross-referencing documentation if there were any scripts or dependencies for formatting
• After spending time reading through and understanding the issue, I had Claude Sonnet 4.6 review my implementation plan:

Plan

First want to relay my understanding:

The requested bug fix is translating outputs to user within the scope of the upload csv feature. 

Structure:
src/server/routes/csv.js
src/server/services/csvPipeline/uploadReadings.js
src/server/services/pipeline-in-progress/loadCsvInput.js
src/server/services/pipeline-in-progress/loadArrayInput.js
src/server/services/pipeline-in-progress/processData.js
src/server/services/csvPipeline/uploadMeters.js
src/server/services/csvPipeline/success.js
src/server/services/csvPipeline/saveCsv.js
src/client/app/components/csv/ReadingsCSVUploadComponent.tsx (and MetersCSVUploadComponent.tsx)

Flow from user:

UploadMeter:
Interact with tsx page.
Submit form triggering submitMeters inside UploadCSVApi
Send post request to routes/csv.js with a language header
Using the server-side helpers for translating, get the language, call saveCSV, and upload (success function if successful).
processData called by loadArrayInput called by loadCSVinput called by uploadReadings/uploadMeters services. 

Similar path for readings. 

Current issues:
not using existing patterns with enum or if logic
translate should not use typescript ? to cleanly separate undefined values
t is poor variable naming and could be done away with entirely
nearly every called service with the language parameter is not using the language parameter at all
(saveCsv and uploadMeters and uploadReadings and loadArrayInput and processData and success.js)

So fix would mostly be injecting the new param and utilizing it throughout the services, as well as fixing miscalleneous comment/docs

• After getting most of the fixes in the service files, I tested in my development environment if my changes worked. Reviewing the changes and testing the changes visually, I noticed placeholder variables weren't being translated and some strings remained English, which I saw was because the translate() helper didn't account for placeholder variables at all. I used Sonnet 4.6 to give me the lines of code for the placeholder variable replacement with regular expressions, and had Composer 2.5 help me with debugging why some strings remained English at first (not properly passing all parameters correctly in some functions).

List of GitHub IDs for anyone working on or contributing to this PR that you cannot certify has followed this policy. Any conttributors not listed are being certified by you to have complied with the terms of the OED AI policy shown below.

none

OED AI policy

(This section is to remain unedited in the PR submission and the checkbox above checked to acknowledge conforming to this policy.)

Given the expanding usage of AI and review of submissions to OED, OED is developing an AI policy. OED is working on a more general solution but for now OED has this AI policy and each submission is required to:

• Disclose of all AI tools used including version.
• Disclose how each AI tool was used including giving the prompts used.
• By checking the AI policy checkbox above, you certify that you have the copyright to the submitted work. Given recent court cases, this will require that less than 50% of the code and documentation came from AI. Less is even better. Every AI system you use must be checked that they provide you with copyright to this work that is compatible with the OED project MPL license.
• By checking the AI policy checkbox above, you certify that you have personally reviewed all code and properly tested it. Thus, you are taking personal responsibility for the quality of the submission by guaranteeing you carefully reviewed every aspect in detail.
  If more than one person is involved in a request, then each person on the team needs to certify and provide the information above. If you don't list anyone else in the section above on unknown compliance with this policy then you must have personal knowledge that the other team members provided you with the necessary information and you are faithfully providing or indicate the answers only apply for yourself or a given list of authors (by GitHub ID).

Other items may be added in the future. It is also open to change given input, new information or changes in the AI sphere and you can make comments/suggestions to the OED project.

[huss]

I'm noting I opened two issues related to this PR:

• Issue internationalize all strings returned by OED server #1748 to extend these ideas to all string returned by the server.
• Issue translation of unformatted toast error messages #1749 to address the limitation given in this PR about unformatted toast error messages.

[huss]

@Amormio25 Thank you for your first contribution to OED that updates PR #1571 to address almost all the comments from that PR. I have made comments on this PR for anything I think should be addressed. Thus, I have closed PR #1571 as it is now considered replaced. Now that this PR is getting closer to ready, I spent time trying to carefully look it over and test it (not done with the previous PR); this took some time. Overall, it is good and I appreciate that the many strings were correctly converted to translatable items. Many of the comments seem straightforward. Some relate to changes from PR #1571 but I found them now and it would be good to resolve them. I welcome any thoughts and let me know if anything is not clear. I know there are a number of comments to consider and I appreciate any work on them. Please do not resolve any comments as I use them to track what I need to look at. You are welcome to put a thumbs up on any you think you addressed or put a reply with any information you think I should see.

For the record, I did testing and found this all seems to work correctly. I did it via the UI by changing languages. I also checked script commands via curl where I used the --header "Accept-Language: es" (and en too) to see what language come back.

Update 261102: All of these items are resolved so this comment is now complete.

Here are some comments that could not easily be put on lines of code so I have them here:

• Should the description also say it fixes issue #703 that was the original issue for the older PR?
• You noted some tests failed and I see 17 failing. I have not looked into them all but each one involves a pipeline test (not 3D readings that you mentioned - let me know if that is what you are really seeing). I looked some in more detail by:
  • All work done in VSC and the terminal where I ran the tests.
  • Copy the actual red text of the failure from the console, create an empty new text file andd place the copied text into it.
  • Repeat for the expected green text.
  • In the Explorer (assuming it is set to show Open Editors), select the first new file (red text) with Select for Compare (from the context menu by right clicking on the file in Explorer). For the second new file (green text), use Compare with Selected. A new tab opens that shows all the differences.
  • Here is what I found for a couple of the tests by doing the first ones listed in the error summary:
    • pipe7: Has two differences where one is a missing <br> and one is a missing space.
    • pipe9: Similar with a missing <br> and two missing spaces.
  • Both of these probably relate to the conversion for internationalization that was done as part of this work. I think they should be easy to fix but care should be take to validate that the original way (before the PR changes) are what is expected given what is seen. My hope is that all the failed tests (or at least a lot of them) are due to this type of issue and can be readily resolved.

[Amormio25]

Hello @huss , I appreciate the really thorough review and will work on addressing them especially the test cases, which I might've gone over a bit too quickly.

I know you created two separate issues for project-wide translation and unformatted toast errors, so just to clarify revising this PR would still close #1732 and the original #703?

I just wasn't sure if the purpose was to revise this with the goal to address the larger issue of project-wide translation for example, since addressing #1732 and #703 seem to now be "sub-issues" of a larger issue.

[huss]

@Amormio25 Thank you for your comment/question.

I know you created two separate issues for project-wide translation and unformatted toast errors, so just to clarify revising this PR would still close #1732 and the original #703?

The current PR addresses those two issues. Since it can stand alone without doing the work to extend it and it address a desire within OED, I think it should be merged when ready and close the original two issues. The two newer issues can be addressed in future work. From painful experience, I know it is tempting to keep extending work but that delays it being introduced into the code and risks more complications from the delay. Thus, unless you want to advocate for waiting to combine with newer work to generalize, I think this is the best course of action.

[Amormio25]

Hey @huss, I left some replies to some of the inline comments, mainly asking for direction on some possible changes I could make. I'll continue working to address the full PR review as well.

[huss]

@Amormio25

I left some replies to some of the inline comments, mainly asking for direction on some possible changes I could make.

I think I have addressed them all. Let me know if anything is not clear, I missed something or you have any thoughts. Thanks for asking for input.

[Amormio25]

I believe I've addressed all the changes. Running the tests this time I saw 15 pipeline tests failing, most of which were due to incorrect formats in translation strings like missing break tags and punctuation, which I saw using the very neat Select for Compare method you mentioned. Let me know if there are any things I still possibly missed.

As for an earlier comment you had regarding automatic translation, I'm willing to do research on this part, specifically something compatible with the size of the project and preferably maintainable. I've worked with something called Weblate before on the Zulip project, but I'm unsure how difficult these sort of automatic translation pipelines are to set up, or whether they are worth it for OED as opposed to the manual translations.

[huss]

As for an earlier comment you had regarding automatic translation, I'm willing to do research on this part, specifically something compatible with the size of the project and preferably maintainable. I've worked with something called Weblate before on the Zulip project, but I'm unsure how difficult these sort of automatic translation pipelines are to set up, or whether they are worth it for OED as opposed to the manual translations.

OED does not add that many strings so I think the question in my mind was if we should start to used automatic translation that is then checked at some point. I'm open to ideas on doing this and how OED could validate that the strings in a PR are reasonable before accepting. Any thoughts or experience is welcome input.

[huss]

@Amormio25 Thank you for addressing my previous comments. My review found this is good and clearly converging to a final solution. I made a few new comments for your consideration.

I came to realize some of the failing tests probably represented cases where the string isn't optimal (you correctly mirrored it in your changes). Given they are already public I think it is best to leave them but at some point OED needs to decide if it will fix any.

Please let me know any thoughts or if anything is not clear.

[huss]

@Amormio25 Thank you for the updated code and comments about my previous comments. I made several comment but most are about issues with my comments and that they are resolved. There is one item (2 comments) about a TS error I see in the terminal when I start OED. I welcome your ideas and let me know if you want me involved in any way.

[huss]

This is to note that issue #1753 was opened to address the CodeQL failures found while scanning this PR. It is unclear it started with this PR and is probably better addressed outside this PR. As such, it is not holding up this PR from being merged.

[huss]

@Amormio25 Thank you for the updated code that addressed my last comments. Review and testing found this works as desired. Per another comment, I am approving even though there are CodeQL concerns. Congratulations on having your first contribution accepted by OED and we appreciate your work.

[Amormio25]

Thanks a lot for the help throughout this process @huss, I hope to continue contributing as well. From my understanding the CodeQL concerns are due to user input, specifically the language param not being validated before usage. I might try to take on that issue in the future, but for now I'll focus on #1749.