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: localize notifications properly (resolves #2015, #2012) #2050

Merged
merged 26 commits into from
Dec 7, 2023

Conversation

chosww
Copy link
Collaborator

@chosww chosww commented Dec 6, 2023

Fixes the issues described in #2015 and #2012.

The issue with 2015 was that we notify users differently based on whether they are a member of an organization or the owner of the organization in ManageOrganizationalAccount. Whenever notification is sent to an organization, we loop through each members then use notify on the users, but organization owners get notification to their account. Problem was that User model has preferredLocale set, so they were receiving emails in correct locale, but Organization or RegulatedOrganization model doesn't have preferredLocale set, so they were receiving emails based on the Admin's locale when the admin is performing some action to the organization's account.

Also, we were using built-in messages from Laravel Framework for email verification, and those strings were not localized. This PR customized email verification messages through AuthServiceProvider as instructed in https://laravel.com/docs/10.x/verification.

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.

Copy link

codecov bot commented Dec 6, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (fbd95be) 97.62% compared to head (20538c6) 97.63%.

Additional details and impacted files
@@            Coverage Diff            @@
##                dev    #2050   +/-   ##
=========================================
  Coverage     97.62%   97.63%           
- Complexity     2103     2105    +2     
=========================================
  Files           314      314           
  Lines          9482     9497   +15     
=========================================
+ Hits           9257     9272   +15     
  Misses          225      225           

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

@chosww chosww requested a review from jobara December 6, 2023 16:11
@chosww chosww self-assigned this Dec 6, 2023
@chosww chosww added the bug Something isn't working label Dec 6, 2023
@jobara jobara changed the title fix: localize notifications properly fix: localize notifications properly (resolves #2015, #2012) Dec 7, 2023
@jobara jobara enabled auto-merge (squash) December 7, 2023 16:22
@jobara jobara merged commit d19c4ae into accessibility-exchange:dev Dec 7, 2023
9 checks passed
@michelled michelled added this to the 1.2.5 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
3 participants