OED icon indicating copy to clipboard operation
OED copied to clipboard

Internationalize new CSV strings

Open JesseVarGar opened this issue 3 years ago • 7 comments

Description

Our team internationalized strings on CSV files. We were able to internationalize strings by adding two new files on the server side named data.js and translation.js. data.js held our string ids with their translations and translation.js held the function that enabled us to translate strings. We used @format.js/intl to translate our strings.

Fixes #703

Type of change

  • [ ] Note merging this changes the node modules
  • [ ] Note merging this changes the database configuration.
  • [ ] This change requires a documentation update

Checklist

  • [x] I have followed the OED pull request ideas
  • [x] I have removed text in ( ) from the issue request

Limitations

No issues.

JesseVarGar avatar Feb 19 '23 05:02 JesseVarGar

Thanks to this team for submitting this PR. OED appreciates your contribution. From the records I see, @davidNicolas-cecs has not agreed to the OED CLA. The project needs that before we can accept your contribution. Please let me know if you have any questions or if you think I missed your CLA. @hyperupcall and @JesseVarGar have CLA records. Thanks.

huss avatar Feb 19 '23 23:02 huss

Good afternoon, I just submitted the form, sorry about that! Thank you have a good day @huss

davidNicolas-cecs avatar Feb 20 '23 00:02 davidNicolas-cecs

Good afternoon, I just submitted the form, sorry about that! Thank you have a good day @huss

Thanks to @davidNicolas-cecs for doing the CLA quickly. I've started looking at the PR but need more time to properly consider it. Let me know if this is an issue.

huss avatar Feb 20 '23 02:02 huss

I've been reviewing the code but think it is best to step back to discuss a higher-level consideration. Your solution resides on the server-side. It uses the site default language to decide on the translated strings to use. There is a second language choice option in OED where the user chooses a language for their interaction and this overrides the site default. Seeing your solution made me realize this distinction for these translations. So far I've come up with these three ways to deal with this:

  1. Do what your code currently does and use the site default. This will mean that the translation language can differ from other client-side ones. If possible, I think OED should avoid this.
  2. Use your server-side solution but route the preferred language to the server. I think this means that each route that wants to do a translation needs to add this to the route. This isn't hard to do but does require touching multiple files/route and must be done for each new route.
  3. Continue to use a client-side solution. The issue had this as the second idea where the keys are passed back for translation. We need to decide the best technique and test it to be sure it works.

There are some other tradeoffs but I don't think they are too important for this level of discussion.

I think OED needs to decide which of the choices it wants to use. I'm leaning somewhat toward option 3 as it may require the least change in code (since it does not touch the routes) but I welcome the thoughts of others, including other options. Once this is decided we can then move forward with this PR.

The work in this PR was invaluable in clarifying these ideas in my mind and I very much appreciate the work. No matter what the decision, this will be the basis for the final solution.

huss avatar Feb 20 '23 15:02 huss

Thank you for your comment! Yeah, it might be best to talk about this a bit higher-level before implementing further, as you have made some pretty good points.

I was thinking about 3), and we weren't sure how to implement it without bringing in the whole frontend. Some of these errors are visible with a single API call where no JavaScript is executed:

$ curl -XPOST http://localhost:3000/api/csv
<h1>FAILURE</h1>CSVPipelineError: No csv file was uploaded. A csv file must be submitted via the csvfile parameter.

I could be misunderstanding something, that's why we went with a server-side solution.

I notice what you mentioned in 2) about syncing the language preference by routing the preferred language to the server. One downside, as you mention, is that "each route that wants to do a translation needs to add this to the route".

It might be possible to implement 2), but in another way. Maybe middleware can be added on the server that looks for the Accept-Language header, and then extracts it to some per-session state object. This object can then be passed down to be used by the translate() function to translate to the correct language. That way, clients like cURL can still see the proper translations if the proper headers are added.

And then lastly, to hook this up for the frontend, we can configure ApiBackend.ts to fire requests with the proper Accept-Language, whenever the language is changed on the frontend.

I think the downside to this approach is that the "state object" has to be passed around all the way until it reaches translate(), which may be a lot of code changes. It may not be so bad, if the variable was called ctx, which can be used in the future if there are other per-session variables that need to be stored.

hyperupcall avatar Feb 20 '23 23:02 hyperupcall

Thanks to @hyperupcall for the insightful comments. Let me begin with the API calls that are direct via curl that I had not thought about before your message. These calls should be done by a script/program that is interacting with OED. Thus, it is less important that the message be translated. Having said that, the same routes are used by the web interface where those same messages do need to be translated. I don't know if we could easily tell the difference between these two uses to handle the messages differently. Either way, this complicates the idea of doing it on the client side. Not sure exactly how it would be dealt with.

For the third way, I was thinking of trying to use template literals/strings where items in ${ } are replaced within the string. I believe these could be a call to the translate function with the key as done elsewhere in the client. I was hoping the route could pass them back as a regular string and then somehow apply the template with backtick but I have not tried it. If not, something could be coded. What is now unclear is how these would be translated on the server. Could we read the data.js file from the client and use them to translate into the default language on the server in a way similar to what you did? And, as noted above, it is not yet clear how we know to do that rather than rely on the client.

So, I don't yet have one that I clearly prefer (unlike what I said before). I would appreciate the thoughts of others with the hope of figuring out the best path.

huss avatar Feb 21 '23 16:02 huss

This is good work but we need to resolve the direction before moving forward. I am sorry it was sat while the project is focused on v1.0 release. I have decided this will wait until after that release and then should move forward.

huss avatar Apr 22 '23 16:04 huss

This is good work but it has sat for a while. When someone is willing to work on this it can be reopened.

huss avatar Jul 16 '24 16:07 huss