[MGNLUI-3637] SystemLanguagesFieldDefinition should not build options in getter Created: 28/Oct/15  Updated: 15/Apr/16  Resolved: 13/Jan/16

Status: Closed
Project: Magnolia UI
Component/s: security app
Affects Version/s: 5.4.2
Fix Version/s: 5.4.4

Type: Bug Priority: Neutral
Reporter: Mikaël Geljić Assignee: Ngoc Nguyenthanh
Resolution: Fixed Votes: 0
Labels: fields
Remaining Estimate: 0d
Time Spent: 3.5d
Original Estimate: 2d

Issue Links:
Relates
relates to MAGNOLIA-6400 Map2Bean tranformer throws NPE if def... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Sprint: Basel 19, Saigon 26
Story Points: 5

 Description   

SystemLanguagesFieldDefinition builds options in getter, using MgnlContext.

Making definitions return contextual info (e.g. currentLocale) defeats the purpose of having a single, valid, reliable configuration for all, and must be avoided. It might lead to errors when accessing definitions outside of any Context (as occurred in linked issue).

  • Definitions should always be stupid POJOs.
  • The logic in #getOptions() should be moved to a custom SelectFieldFactory, overriding #getSelectFieldOptionDefinition()
  • The FieldTypeDefinition should be registered in in JCR config, under security-app module.


 Comments   
Comment by Ngoc Nguyenthanh [ 04/Nov/15 ]

I have fixed by:

  • Move logic to get field options from info.magnolia.security.app.dialog.field.SystemLanguagesFieldDefinition to field factory info.magnolia.security.app.dialog.field.SystemLanguagesFieldFactory.
  • Register the FieldFactory in Version Handler for old version and bootstrap file.
  • Add test method to cover in case upgrade to new version.
  • Update unit-test for FieldFactory.

Due to fix version is 5.4.4 but I have done the ticket when installed version is 5.4.3-SNAPSHOT. So when we do the integration we need to update the version in:

  • Comment for deprecated field in info.magnolia.security.app.dialog.field.SystemLanguagesFieldDefinition#SYSTEM_LANGUAGES_PATH
  • Delta version in info.magnolia.security.setup.SecurityModuleVersionHandler:284
  • Update the unit-test as well: info.magnolia.security.setup.SecurityModuleVersionHandlerTest#updateFrom542ReconfigureFactoryClassOfSystemLanguagesField
Comment by Evzen Fochr [ 11/Jan/16 ]

Preselected language is not cosistent with previsious version.
On demo preselected language for new user (Security app) is user language.
Now it is first language in select box.

Comment by Ngoc Nguyenthanh [ 13/Jan/16 ]

SystemLanguagesFieldFactory inherit from SelectFieldFactory
Before refractoring info.magnolia.ui.form.field.factory.SelectFieldFactory#getSelectFieldOptionDefinition call definition to get options and add selected language into private variable initialSelectedKey. It use to create default option for the field in info.magnolia.ui.form.field.factory.SelectFieldFactory#createDefaultValue.
When I move code to factory, we still received exact selected option but it's not to be used.
I fixed by overriding info.magnolia.security.app.dialog.field.SystemLanguagesFieldFactory#createDefaultValue to return the pre-selected language.

Generated at Mon Feb 12 09:08:38 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.