Skip to content

Comments

check overrides for parent locale when compiling language files#242

Merged
LukeTowers merged 9 commits intodevelopfrom
fix-client-localization-strings-compilation
Sep 28, 2021
Merged

check overrides for parent locale when compiling language files#242
LukeTowers merged 9 commits intodevelopfrom
fix-client-localization-strings-compilation

Conversation

@mjauvin
Copy link
Member

@mjauvin mjauvin commented Jul 19, 2021

  • fallback to parent locale for client localization strings overrides

- fallback to parent locale for client localization strings overrides
@mjauvin mjauvin added this to the v1.1.5 milestone Jul 20, 2021
@mjauvin mjauvin added needs review Issues/PRs that require a review from a maintainer maintenance PRs that fix bugs, are translation changes or make only minor changes labels Jul 20, 2021
@mjauvin mjauvin changed the title compile lang improvements for fallback/parent locales artisan compile lang must check parent locale overrides Jul 20, 2021
@mjauvin mjauvin changed the title artisan compile lang must check parent locale overrides artisan compile lang must check overrides for parent locale Jul 20, 2021
@mjauvin mjauvin changed the title artisan compile lang must check overrides for parent locale check overrides for parent locale when compiling language files Jul 20, 2021
@LukeTowers LukeTowers added needs response Issues/PRs where a maintainer is awaiting a response from the submitter and removed needs review Issues/PRs that require a review from a maintainer labels Jul 20, 2021
@LukeTowers
Copy link
Member

LukeTowers commented Jul 20, 2021

@mjauvin @bennothommo do you guys think it's worth unit testing this or no?

@mjauvin
Copy link
Member Author

mjauvin commented Jul 20, 2021

I guess so, yeah.

@mjauvin
Copy link
Member Author

mjauvin commented Jul 20, 2021

Any idea on how to approach this @LukeTowers ?

@LukeTowers
Copy link
Member

Probably would need to break it into a separate method to be more testable? Isn't there anything preexisting in the lang class that can handle getting and merging the messages appropriately?

@bennothommo
Copy link
Member

@LukeTowers:

@mjauvin @bennothommo do you guys think it's worth unit testing this or no?

Yes! All the test cases!

@mjauvin:

Any idea on how to approach this @LukeTowers ?

You should be able to test this method direct, if you use the callProtectedMethod helper function in the TestCase class. Since the test cases will have a full copy of Winter CMS when running the tests, you could simply make some modifications to the necessary lang files to test different scenarios. Ultimately, you would be looking at the resulting JS for the test, I would imagine.

@github-actions
Copy link

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this is still being worked on, please respond and we will re-open this pull request.
If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

@github-actions github-actions bot added the stale Issues/PRs that have had no activity and may be archived label Sep 20, 2021
@LukeTowers LukeTowers removed the stale Issues/PRs that have had no activity and may be archived label Sep 20, 2021
@LukeTowers LukeTowers self-assigned this Sep 23, 2021
@jaxwilko
Copy link
Member

@mjauvin @bennothommo had a bit of time free this morning so I've added a test for lang compiling. Let me know what you guys think :)

@LukeTowers LukeTowers added Status: Completed and removed needs response Issues/PRs where a maintainer is awaiting a response from the submitter labels Sep 28, 2021
@LukeTowers
Copy link
Member

LGTM, thanks @mjauvin and @jaxwilko!

@LukeTowers LukeTowers merged commit 4f43dfd into develop Sep 28, 2021
@LukeTowers LukeTowers deleted the fix-client-localization-strings-compilation branch September 28, 2021 22:50
LukeTowers added a commit that referenced this pull request Nov 13, 2021
* develop: (25 commits)
  Support embedded data URIs in the list image column type
  Make some adjustments to the readme content
  Update banner in readme
  Add new GitHub banner
  Documentation with icons (#347)
  Limit options shown in group filter, apply scope when retrieving filtered options
  Add Exception on wrong relation type in relation formwidget (#334)
  Redesigned color picker widget (#324)
  Add winter:test command (#202)
  Use the correct backend timezone config key (#337)
  Get changelog only of the current branch
  Fix Markdown editor sizing issue on Chrome.
  Check overrides for parent locale when compiling language files (#242)
  Fixing commas in English translation files (#305)
  Added Latvian translations for Allowed IP messages (#304)
  Add missing filter translations (#303)
  Clean up newlines
  Update Russian language (#302)
  Fix issue present in overriding RelationController partials using the default code
  Maintenance Allowed IP list (#147)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance PRs that fix bugs, are translation changes or make only minor changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants