5246 missing translation orders list #5285
Conversation
ghost
requested a review
from 
There was a problem hiding this comment.
This is looking good. :-)
Another code style thing: We like to keep the commit history clean. So if there is a detour like adding to gitignore and then removing it again, you can delete those commits and keep only the change you want. We described that in our wiki: https://github.com/openfoodfoundation/openfoodnetwork/wiki/Making-a-great-pull-request#keep-your-commit-history-clean-tidy-and-up-to-date
There was a problem hiding this comment.
Awesome!
Agree with Maikel. This is how you would do it here:
- git log
- pick the commit key of last commit that is not in your branch (usually the one that says master)
use that key and do: - git rebase -i <last_commit_key_not_in_the_pr>
- replace "pick" with "drop" in the lines of the commits you want to remove.
- force push in to remote
- done :-)
|
Thank you for your approvals and comments! |
|
@rmklaus12 On the staging server, I have observed that translations are still missing. Details in testing notes:https://docs.google.com/document/d/1wc7LWuE24Qye7VldV0tMOt6WEi9B-k2gc-kqdZPornk/edit# |
|
I think the process for testing translations is different, ping @RachL |
|
Thanks @luisramos0 - I'll be sure to follow this discussion as well. |
|
ah, sorry Filipe I didnt see you were involved in this PR. I was not sure you would know the solution so I pinged Rachel 👍 |
|
That was perfect @luisramos0 👍 I was providing support to @Ekow244 on general testing procedures. Pinging Rachel was the right thing to do, as testing translations were new to me as well :-) |
|
So judging by the screenshots this is ready to go :) thanks everyone! |
|
I am happy there is a conflict here because it made me look at the code again and see it is broken. The new keys should work but they were inserted in a place where the keys for general_settings/edit will break. The change need to be shifted one lione below in the en.yml file. |
config/locales/en.yml
Outdated
| shipment_state: "Shipment State" | ||
| email: "Email" | ||
| total: "Total" | ||
|
|
There was a problem hiding this comment.
the shared group has the correct indentation but it cant go between general_settings and edit
|
Ahh! Glad that was caught, I will fix this in the morning.
…On Mon, 27 Apr 2020 at 9:32 pm, Luis Ramos ***@***.***> wrote:
***@***.**** requested changes on this pull request.
requesting changes to the en.yml
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5285 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHG525ORX47MSELZGTFJX5DROVUNLANCNFSM4MN75ZAQ>
.
|
|
Closed by mistake. Reopening to push requested changes to en.yml file. |
ghost
reopened this
|
Will do! And yes, that's a 'duh!' moment, of course it is the full
resolution not just my last change. Thank you :-)
…On Tue, Apr 28, 2020 at 4:43 PM Maikel ***@***.***> wrote:
***@***.**** commented on this pull request.
Your commit description only tells us the last thing you did in response
to Luis' comment. But the commit content is better described as "Add
missing translations on order list page". Can you amend that and push again?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#5285 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHG525KOWEUEVW25GTZOP7LROZ3KXANCNFSM4MN75ZAQ>
.
|
|
@luisramos0 do we need to re-test here? |
|
I think it's better we retest, to make sure the general settings page shows translations :-) |
|
Looking good :)
Moving to ready to go |
What? Why?
Closes #5246
Translation keys were missing on multiple fields in the Orders List. This fix (adding the appropriate keys to the config/locales/en.yml file resolved the missing translation keys issue.
What should we test?
Is translation present on the orders list (number, state, payment_state, shipment_state, email, and total fields) at /admin/orders?q[s]=completed_at+desc
Release notes
Translation now working on the Orders List
Changelog Category: Fixed
Dependencies
Not dependent, but is related (in the resolution) to #4966
Documentation updates
No updates required