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

Added possibility to set backend locale per every admin user #3087

Merged
merged 7 commits into from
Apr 5, 2023
Merged

Added possibility to set backend locale per every admin user #3087

merged 7 commits into from
Apr 5, 2023

Conversation

fballiano
Copy link
Contributor

This PR adds the possibility to set a locale for the backend for every backend user, adding a column to the admin_user table and the UI components to use this new column.

This is the new field added to the "System -> My Account" form:

Screenshot 2023-03-14 alle 22 39 50

Manual testing scenarios (*)

  1. login to the backend
  2. click "system -> my account"
  3. set the backend locale using the new field
  4. save
  5. logout
  6. login to the backend again
  7. the locale of the backend should be the one selected

@github-actions github-actions bot added Component: Adminhtml Relates to Mage_Adminhtml Component: Admin Relates to Mage_Admin translations Relates to app/locale labels Mar 14, 2023
@fballiano
Copy link
Contributor Author

PHPStan fails for unrelated issues with the 20.0 branch

@luigifab
Copy link
Contributor

No tested, but is there the possibility to leave the field empty?

@fballiano
Copy link
Contributor Author

@luigifab there is

@elidrissidev
Copy link
Member

Have you thought about saving the locale in the extra field instead of a new column?

@fballiano
Copy link
Contributor Author

@elidrissidev ah! noup, didn't even notice it existed. It's interesting. I guess it makes sense.

Is "extra" supposed to be used by the "system" or by normal developers? I ask this because I'm wondering if who manipulates the data in that field knows that some more stuff can be added by the core and that they should act accordingly.

@kiatng
Copy link
Contributor

kiatng commented Mar 17, 2023

The extra field is to save the state in the System Configuration:

/**
* Save state of configuration field sets
*
* @param array $configState
* @return bool
*/
protected function _saveState($configState = [])
{
$adminUser = Mage::getSingleton('admin/session')->getUser();
if (is_array($configState)) {
$extra = $adminUser->getExtra();
if (!is_array($extra)) {
$extra = [];
}
if (!isset($extra['configState'])) {
$extra['configState'] = [];
}
foreach ($configState as $fieldset => $state) {
$extra['configState'][$fieldset] = $state;
}
$adminUser->saveExtra($extra);
}
return true;
}

So, if you use it, it may be overwritten on lines 322-325. Using it for locale is not what it is intended for. It's better with a new column.

@fballiano
Copy link
Contributor Author

@kiatng thank you!

@elidrissidev
Copy link
Member

I know it's used for user config state, but it's put inside array key configState to allow storing additional data in there. Anyways, adding a new column doesn't hurt :)

m-overlund
m-overlund previously approved these changes Mar 29, 2023
Copy link
Contributor

@m-overlund m-overlund left a comment

Choose a reason for hiding this comment

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

Tested without issues

@fballiano fballiano changed the base branch from 20.0 to main April 4, 2023 17:35
@fballiano fballiano dismissed stale reviews from m-overlund and FredericMartinez April 4, 2023 17:35

The base branch was changed.

@fballiano
Copy link
Contributor Author

I'm merging cause we have 1 green check and 1 gray check but 2 more gray checks were dismissed only cause of the branch change, without code change (and the branch is still v20 at the end of the day)

@fballiano fballiano merged commit c20371d into OpenMage:main Apr 5, 2023
@fballiano fballiano deleted the user_locale_backend branch April 5, 2023 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Admin Relates to Mage_Admin Component: Adminhtml Relates to Mage_Adminhtml translations Relates to app/locale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants