MBS-13117: Prevent localizing admin-only messages #3091
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mwiencek
reviewed
Nov 15, 2023
10 tasks
yvanzo
force-pushed
the
mbs-13117-l-admin
branch
5 times, most recently
from
c6f2654
to
73d3fce
Compare
reosarevok
reviewed
Nov 27, 2023
mwiencek
reviewed
Nov 27, 2023
mwiencek
reviewed
Nov 27, 2023
po/Makefile
Outdated
| test_no_admin: $(ADMINPERL) $(ADMINTT) $(ADMINJS) $(wildcard extract_pot_templates) | ||
| @echo "Testing that no admin files contain any translatable messages (MBS-13117)"; | ||
| @count=0; \ | ||
| for file in $(ADMINPERL); do \ |
There was a problem hiding this comment.
Would it be worth moving test_no_admin to its own bash script in order to avoid all the newline escapes?
(Not sure specifying $(ADMINPERL) $(ADMINTT) $(ADMINJS) in the Makefile as dependencies is strictly needed, since this rule is only invoked once by the tests.)
There was a problem hiding this comment.
Good idea, moved in commit 6e0619e
yvanzo
force-pushed
the
mbs-13117-l-admin
branch
4 times, most recently
from
6e0619e
to
465c4cd
Compare
reosarevok
approved these changes
Nov 28, 2023
There was a problem hiding this comment.
This seems good with the caveat that assuming we merge #2480 first, which would seem the most sensible order, it will need some rebasing to "un-delete" some of the translation calls for the files that are no longer admin only (any forms would still be admin-only IIRC but since they won't be under /admin maybe they should use l_admin instead of nothing).
Part of MBS-13117
There is no non-module file **.pl under lib/ anyway, but even if there was any it would be for admin only.
Implement all eventually needed functions to prevent translating messages destinated to admin in both admin and non-admin pages, as part of MBS-13117.
Prevent translating messages destinated to admin in non-admin pages, as part of MBS-13117.
It is equivalent to expand2react(cleanMsgid) but it makes it explicit that it is intentionally to not translate messages destinated to admin, as part of MBS-13117.
To prevent any further accidental localization in admin pages, this patch makes CI tests to exit with error with a non-empty list of lines calling localization functions (l, ln, lp, N_l, N_ln, N_lp).
There will never be any context needed for not translatable messages.
yvanzo
force-pushed
the
mbs-13117-l-admin
branch
from
465c4cd
to
7e2e74e
Compare
|
Just rebased on latest |
yvanzo
force-pushed
the
mbs-13117-l-admin
branch
from
280947f
to
2311fd9
Compare
The column labels for languages and scripts were exposed to admins only but still translated. These messages would be removed from translations after the previous commit 05960f8 from the following files: - root/admin/attributes/Attribute.js - root/admin/attributes/Language.js - root/admin/attributes/Script.js However those will likely be needed again after resolving MBS-11178. This patch preserves column labels and their translations by temporarily defining these in a unused file until MBS-11178 gets resolved.
yvanzo
force-pushed
the
mbs-13117-l-admin
branch
from
2311fd9
to
e76ec04
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem MBS-13117
Messages intended for admins only are needlessly extracted for translation.
Indeed admins are required to be English-speaking anyway.
So it is a waste of time for translators so far.
Solution
((t)exp).(N_)l[np]_adminfunctions for admin-only messages in other pages.It makes explicit that some messages are intentionally not translated because these are intended for admin.
This approach works well in JavaScript, should we follow the same in Perl (under
libonly)?Draft progress checklist
po/mb_server_potafter the singular/plural issue has been resolved.Action