Skip to content

Comments

Louder warning to discourage permanently changing tenant locale#340

Open
MikeTaylor wants to merge 1 commit intomasterfrom
louder-warning-when-changing-locale
Open

Louder warning to discourage permanently changing tenant locale#340
MikeTaylor wants to merge 1 commit intomasterfrom
louder-warning-when-changing-locale

Conversation

@MikeTaylor
Copy link
Contributor

This keeps happening despite the present warning. Although that warning is clear, it's also verbose, and maybe easy to skip over for people whose first language is not that of the then-prevaling locale -- precisely those who we need to read it.

This commit:

  • Adds a large red banner saying "STOP! Do not do this."
  • Changes the "Change session locale" button into an actual <Button>
  • Adds whitespace below the warning and button

Please let's just get this done instead of agonizing over it, like we do every time this starts happening. We can incrementally improve what I've done here in the future, but leaving it as it is just asks for more trouble.

This keeps happening despite the present warning. Although that
warning is clear, it's also verbose, and maybe easy to skip over for
people whose first language is not that of the then-prevaling locale
-- precisely those who we need to read it.

This commit:
* Adds a large red banner saying "STOP! Do not do this."
* Changes the "Change session locale" button into an actual `<Button>`
* Adds whitespace below the warning and button
@MikeTaylor MikeTaylor requested review from mkuklis and zburke April 5, 2023 15:39
@github-actions
Copy link

github-actions bot commented Apr 5, 2023

Jest Unit Test Statistics

  1 files  ±0  24 suites  ±0   1m 0s ⏱️ -1s
77 tests ±0  73 ✔️ ±0  4 💤 ±0  0 ±0 
79 runs  ±0  75 ✔️ ±0  4 💤 ±0  0 ±0 

Results for commit f73aa4f. ± Comparison against base commit 977e585.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Apr 5, 2023

BigTest Unit Test Statistics

0 tests  ±0   0 ✔️ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ±0 

Results for commit f73aa4f. ± Comparison against base commit 977e585.

♻️ This comment has been updated with latest results.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

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

Choose your own adventure 🗺️ 📖 What would you like to read?

useless, depressing rant on why I haven't approved this, but at least it was cathartic

I've tried many different ways to discourage people from using this setting when they should be using session-locale. None have worked and I eventually gave up. In the end, I concluded that people who don't care ... don't care. Changing the language will make you feel better, but it's unlikely to help anybody else achieve enlightenment. Yay!

actionable information to get this PR approved
  1. I understand the motivation, but I'm not sure about the language. Yeah, I find it super annoying when folks change the tenant locale in the reference environments, but it is a legit thing to do in a production environment (at least on initial setup) and I worry the "Do not do this!" message sends, well, the wrong message. If you're really confident you want this, please write a Jira and ask Magda Zacharska (mzacharska@ebsco.com) for review. User-facing changes generally need to be approved by the PO who owns the module.
  2. Please tag @uladislausamets for review since he's the lead dev on this module. The team-project matrix lists the lead dev for all modules.
  3. I wrote but never merged a PR that would show the warning alert in the destination-locale, so if you're changing from English to Chinese you'll see the "You probably don't want this..." message in Chinese instead of English. LMK if you want me to dig around for it. It might offer an alternative way to provide a strong message.
alternative solutions

In the reference environments, this could be managed with permissions. Just take away ui-tenant-settings.settings.locale from diku_admin. I think this (or some variation on "use permissions") is closest to The Right Solution.

@MikeTaylor
Copy link
Contributor Author

Screw that.

I've wasted an hour trying to make a ubiquitous problem go away only to run into a predictable brick wall. I'm not going to waste another whole day on top of that.

I'm gonna leave this how it is, and let it be someone else's problem.

When will I ever learn?

@MikeTaylor MikeTaylor closed this Apr 5, 2023
@skomorokh
Copy link

An alternative alternative: add a pair of radio buttons beneath the dropdown for locale:

  • temporarily update locale only for current session
  • update system locale for all users

...with the top one selected by default.

@MikeTaylor
Copy link
Contributor Author

Predictably enough, two years later, this keeps happening over and over and over and over again, and we have done absolutely nothing to mitigate it.

@MikeTaylor
Copy link
Contributor Author

I went to do a bit of work on snapshot this afternoon, only to find some has changed to local to something Arabic.
Screenshot 2025-04-12 at 16 53 58

It's ridiculous. Screw it, I'm merging this because it's an incremental improvement that will prevent this idiotic waste of time from continually recurring.

Anyone who doesn't like (meaning mostly you, @zburke) has the following choices:

  • Revert it, making FOLIO unarguably worse; or
  • Do something better.

@MikeTaylor MikeTaylor reopened this Apr 12, 2025
@MikeTaylor MikeTaylor closed this Apr 12, 2025
@MikeTaylor
Copy link
Contributor Author

No, I'm not, because there's a merge conflict that I can't make myself think about on a Saturday afternoon, but HOLY HELL my life would have been better if I'd just merged this back in the day, and so would everyone else's.

@stegherr-ubtum
Copy link

The thing is, this seems to be an issue for more people than @MikeTaylor, otherwise it wouldn't happen. There is a problem that those people might not even be able to read this discussion (otherwise they wouldn't want to change the language in FOLIO anyway). Leaving it as it is is arguable the worst option and trying something else couldn't make it worse imho. I could even imagine that there are threads in forums somewhere, where people rant about in Arabic, why the locale always gets changed back to English. ;) But I can't read those.

@zburke's concern about the language is valid. Some people do enjoy some strong wording and colorful language whereas others get offended easily. The problem here is that we are dealing with non-native English speakers (or non English speakers even), who we should address differently anyway! We should probably use some pictograms or something universally understandable.

Hope my sermon makes some sense. I'm a bit of a language nerd myself, so I'm sorry for banging on about this.

@MikeTaylor
Copy link
Contributor Author

MikeTaylor commented Aug 25, 2025

Oh, look, someone has set snapshot to an Asian locale with a non-Latin character set. I wonder how that could have happened. If only there had been some way to discourage the constant constant recurrence of this very avoidable roadblock.
Screenshot 2025-08-25 at 11 12 50

@MikeTaylor MikeTaylor reopened this Aug 25, 2025
@zburke
Copy link
Member

zburke commented Aug 25, 2025

@MikeTaylor, I continue to believe this is a permissions problem masquerading as a social "if only people knew, then they wouldn't make this change" problem. Setting the locale in production is normal operation that should be restricted to a few people, just like vipw. The approach there is not to grant universal access but with a really big warning so people choose not to corrupt the /etc/passwd file; the solution is to only provide access to the people who need access. On folio-snapshot, the generally-available account should not have access to anything we don't want everybody to be able to change.

Regardless, FOLIO is now running in production in 100s of institutions world-wide and the bad old days of us devs making whatever functional changes they desire are long over, at least outside of our playground in ui-developer. This module has changed ownership since I last wrote; please contact the PO Thomas Trutt (tit1@cornell.edu) and lead-dev @vashjs if you want to get this, or any related user-facing feature, merged.

@MikeTaylor
Copy link
Contributor Author

I don't disagree with any of this.

I hate that:

  1. This make-the-problem-only-10%-as-bad solutions, which was right there, never got merged to make the problem only 10% as bad.
  2. Other, no-doubt better solutions that were not right there have never been created.
  3. So the problem continues to be 100% as a bad, and I and hundreds of other people continue to waste time on it over and over and over again.

I would be be at all surprised if the total effort that has been wasted since I filed this PR is up into the hundreds, maybe even thousands, of person-hours, given how many people (try to) use snapshot, and how many incidents there have been.

I would have liked to eliminate 90% of that pain. Instead, it's remained essentially due to a religious conviction. That is a tragedy for all of us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants