Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename "entreprise" in i18n keys to "enterprise" #2625

Conversation

kristinalim
Copy link
Member

What? Why?

Closes #2443

Inconsistent use between the two could cause confusion and possibly bugs.

$ git grep entreprise | grep ^config/locales/en.yml | wc -l
0
$ git grep entreprise | grep -v config/locales/ | wc -l
0

What should we test?

This does not involve changes to how the software works, but needs some actions per language on Transifex.

Smoke test of customer purchasing items, looking at places where there seem to be missing translations. Best if this can be tested in another language so missing translation keys would be more obvious.

Release notes

  • Rename translation keys that use "entreprise" to use "enterprise" instead

Changelog Category: Changed

@kristinalim kristinalim self-assigned this Aug 31, 2018
Copy link
Contributor

@luisramos0 luisramos0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I love clean ups!

@luisramos0 luisramos0 added the pr-staged-fr staging.coopcircuits.fr label Sep 3, 2018
@luisramos0
Copy link
Contributor

Staged here: http://staging.openfoodfrance.org/

@RachL RachL self-assigned this Sep 3, 2018
@RachL
Copy link
Contributor

RachL commented Sep 3, 2018

@luisramos0 @myriamboure I've tried to create a new order cycle for Enterprise1, Enterprise 2 and Enterprise test France VAT. Each time I get the same error message:
image
I guess it is not linked to that PR, is everything good with the staging server? Also, is that error message always in English? If yes I will create an issue.

@myriamboure
Copy link
Contributor

myriamboure commented Sep 3, 2018

Ok so I can confirm that there is a big bug here. I have some clue that can help understanding the issue: I can update an existing OC but not create a new one with exactly the same parameters.
Updated OC:
capture du 2018-09-03 17-10-25
New OC:
capture du 2018-09-03 17-10-36
@kristinalim can it be linked to that PR? If not it means there is some bigger issue on the master on which this PR is based, that we need to fix before release @luisramos0 ...

@luisramos0
Copy link
Contributor

Before anything else, let me check the staging environment. Most probably is an issue with staging. I'll let you know what I find. Maybe only tomorrow morning.

@kristinalim
Copy link
Member Author

I double-checked the changes introduced by this commit, and cannot find a possible cause of the problem. I tested with my local DB too - creating and editing enterprises seem to be working.

I also suspect this is an issue with the staging server. @luisramos0 Just let me know if I could help look into this. I've yet to request access to the staging servers.

@luisramos0
Copy link
Contributor

ok, lets have a look.

first, the easy one, @RachL yes, "Failed to create order cycle" is hard coded and needs an issue to be fixed. (the bug is in app/assets/javascripts/admin/order_cycles/services/order_cycle.js.coffee:159 and 174)

@luisramos0
Copy link
Contributor

There were two issues with the FR staging server:

  1. the DB was in an inconsistent state (the db deploy job didnt error out but the db was not in the right state after it). I am sure this is correct now.
  2. delayed job was not running (I found this as I was reloading the sample data). the job is now running but it wasn't started with ofn-install. @pacodelaluna will be looking into this.

I tried to create an order cycle before and I could see the error. Now it's working.
Can you retry your test @RachL ?

@RachL
Copy link
Contributor

RachL commented Sep 5, 2018

@luisramos0 what superadmin login did you used? Spree@example is not working for me anymore... I will try and open a new enterprise in the meantime

@luisramos0
Copy link
Contributor

Sorry, I forgot to mention that. it's [email protected] spree123 on this server.

@RachL
Copy link
Contributor

RachL commented Sep 5, 2018

@luisramos0 thanks it's working 🎉 I will finish testing this PR now

@RachL
Copy link
Contributor

RachL commented Sep 5, 2018

And ready to go !
Testing notes:
https://docs.google.com/document/d/163BGw8XyeWqIudz4m8Y-TxHot5JsedKFVvwpVPBQaPo/edit#

@RachL RachL removed the pr-staged-fr staging.coopcircuits.fr label Sep 5, 2018
@sauloperez sauloperez merged commit 0a05e69 into openfoodfoundation:master Sep 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants