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

fix: model lsq translations don't fallback to fr (resolves #2014, #2026, #2028, #2029) #2027

Merged
merged 17 commits into from
Dec 12, 2023

Conversation

jobara
Copy link
Collaborator

@jobara jobara commented Nov 29, 2023

Resolves #2014
Resolves #2026
Resolves #2028
Resolves #2029

  • Renames get_written_language_for_signed_language to to_written_language
  • Adds localized_route_for_locale helper
  • Adds route_name helper
  • Makes use of the missingKeyCallback to fallback lsq to fr
  • Fixes rendering of langage change links
  • Fixes output of translatable model properties to match requested translation
  • Updates/expands tests

Prerequisites

If this PR changes PHP code or dependencies:

  • I've run composer format and fixed any code formatting issues.
  • I've run composer analyze and addressed any static analysis issues.
  • I've run php artisan test and ensured that all tests pass.
  • I've run composer localize to update localization source files and committed any changes.

If this PR changes CSS or JavaScript code or dependencies:

  • I've run npm run lint and fixed any linting issues.
  • I've run npm run build and ensured that CSS and JavaScript assets can be compiled.

BEGIN_COMMIT_OVERRIDE
fix: model lsq translations don't fallback to fr (resolves #2014, #2026) (#2027)

fix: rendering of language changer links (resolves #2028, #2029) (#2027)
fix: response time in other languages not saved (#2027)
fix: can't add other engagement languages (#2027)
fix: not displaying content in translation (#2027)
END_COMMIT_OVERRIDE

@jobara
Copy link
Collaborator Author

jobara commented Nov 30, 2023

@greatislander in fixing the failing test I realized that I may have removed some functionality, but I'm also not sure how that part was supposed to work or if it was actually functional on the site. I wonder if you could weigh in. Basically I removed the language bits from the controllers. It looks like you could use that to pass in a query parameter to have an individual or organization public page rendered in an alternative language if one was provided. However, I don't see how a user could have specified that query parameter. Am I understanding this correctly? Have I missed something? Is this still needed?

Copy link

codecov bot commented Nov 30, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (ccaa90c) 97.63% compared to head (bb50619) 97.60%.
Report is 1 commits behind head on dev.

❗ Current head bb50619 differs from pull request most recent head af2731e. Consider uploading reports for the commit af2731e to get more accurate results

Files Patch % Lines
app/helpers.php 80.95% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #2027      +/-   ##
============================================
- Coverage     97.63%   97.60%   -0.03%     
+ Complexity     2105     2101       -4     
============================================
  Files           314      313       -1     
  Lines          9497     9522      +25     
============================================
+ Hits           9272     9294      +22     
- Misses          225      228       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jobara
Copy link
Collaborator Author

jobara commented Nov 30, 2023

@greatislander in fixing the failing test I realized that I may have removed some functionality, but I'm also not sure how that part was supposed to work or if it was actually functional on the site. I wonder if you could weigh in. Basically I removed the language bits from the controllers. It looks like you could use that to pass in a query parameter to have an individual or organization public page rendered in an alternative language if one was provided. However, I don't see how a user could have specified that query parameter. Am I understanding this correctly? Have I missed something? Is this still needed?

I spoke with @greatislander about this. The $language set from the query parameter is to allow those pages to be shown in different languages, if they've been added for them by the user/org. This for example would allow an organization to localize their public page in many languages even though the site only supports a couple. Will need to bring that functionality back. The original work for this was done under #505.

While the written language supports from HasMultimodalTranslations isn't required any more, the rough in for the ASL/LSQ upload options is still requested to remain, in case we implement those features in the future (See: #455).

@jobara
Copy link
Collaborator Author

jobara commented Nov 30, 2023

While reviewing this, @greatislander found two other bugs ( #2029, #2028 ). If possible these should be addressed alongside this one.

@jobara jobara added this to the 1.2.4 milestone Dec 4, 2023
@jobara jobara marked this pull request as draft December 4, 2023 12:44
@jobara jobara modified the milestones: 1.2.4, 1.2.5 Dec 4, 2023
@jobara jobara marked this pull request as ready for review December 8, 2023 13:17
app/helpers.php Outdated
use Illuminate\Support\Arr;
use Illuminate\Support\Facades\Route as FacadesRoute;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd call this RouteFacade instead.

@jobara jobara changed the title fix: model lsq translations don't fallback to fr (resolves #2014, #2026) fix: model lsq translations don't fallback to fr (resolves #2014, #2026, #2028, #2029) Dec 11, 2023
@jobara jobara enabled auto-merge (squash) December 11, 2023 14:42
@jobara jobara merged commit 3220f3f into accessibility-exchange:dev Dec 12, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment