Hello! It doesn't seem this is a bug, so I'll mark it as a feature request.
Allowing sitewide alerts to be translatable results in a weird behavior where using the language selector when creating a new alert isn't respected and the alert is displayed on all languages. So, if I were to create an alert in french (select french in the language selector), save it and then navigate to English, the french alert would be shown.
I'll update this issue as I venture down the rabbit hole.
Comment | File | Size | Author |
---|---|---|---|
#13 | 3293384-respect-selected-language-13.patch | 7.99 KB | andre.bonon |
#10 | MR24-CorrectWay.png | 19.89 KB | michelecris |
#10 | branch-2-x-alert-inPortuguese.png | 17.4 KB | michelecris |
#10 | screen-test.png | 58.59 KB | michelecris |
#7 | screen-13-38-50.png | 12.3 KB | michelecris |
Issue fork sitewide_alert-3293384
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
erikbrgn CreditAttribution: erikbrgn at Axis Communications AB commentedComment #4
michelecris CreditAttribution: michelecris at CI&T commentedHi!
I will try to review this one.
Comment #5
michelecris CreditAttribution: michelecris at CI&T commentedHi @erickbrgn
I reviewed your code and your test all passed!
But when I try to reproduce the alerts on drupal, I realize that shows just the default language. If I try to translate other language inside the alert, this isn't showing up when I select the correspondent language. Just if I create a new alert and select default the language I want to show. So I will try to fix that.
Comment #7
michelecris CreditAttribution: michelecris at CI&T commentedHello!
I made some changes in the code, and now its showing all the messages saved from the current language inside the alert not just the default message that was before. I made some screenshots from alerts module view now. If someone could review that I will aprecciate.
Thanks!
Comment #8
erikbrgn CreditAttribution: erikbrgn at Axis Communications AB commentedHello @michelecris.
Thanks for the review/feedback!
I think it'd be easier if you'd continued the existing fork/MR instead of creating your own with my changes committed as yours.
I've added a test for the scenario you mentioned, which made me change the approach a little by utilizing
EntityQuery
andEntityRepository
to retrieve alerts instead of relying fully on the storage.Comment #9
michelecris CreditAttribution: michelecris at CI&T commentedHi @erikbrgn
Im sorry about that, Im new here and still learning the rules and how I can help the Drupal community. So I could review your code and see if resolve the problem that I mentioned before here.
Comment #10
michelecris CreditAttribution: michelecris at CI&T commentedHello again @erikbrgn!
I reviewed your new aproach in #8 and now its working right!
I run the test and all passed too. And I print my drupal page showing how it was before in 2.x branch and now in your branch MR24 with your new changes and return all correct. So I will change the status for RTBC.
Thanks!
Comment #11
michelecris CreditAttribution: michelecris at CI&T commentedComment #12
erikbrgn CreditAttribution: erikbrgn at Axis Communications AB commentedExcellent. Thanks for reviewing, @michelecris :)
Comment #13
andre.bonon CreditAttribution: andre.bonon at ImageX commentedAdding a "snapshot" patch from #8, as the MR's Diff might change.
Comment #15
ChrisSnyder