UIOptions handling of default model needs to be improved to store exactly changed options
Description
Environment
Activity
Justin Obara March 20, 2014 at 1:56 PM
Merged pull request ( https://github.com/fluid-project/infusion/pull/482 ) into the project repo at b6db5eeb923625594d24aa2fe0fe7392b3989926
Cindy Li March 20, 2014 at 1:05 PM
A pull request is sent to fix this issue: https://github.com/fluid-project/infusion/pull/482
Cindy Li March 19, 2014 at 2:19 PMEdited
This might have been broken by the implementation of the new prefs framework.
The store currently saves the whole rootModel that contains the merged defaults and previously saved prefs. It is causing the problem that all the preferences are saved regardless they are modified or not. The solution is to keep two models: one contains all the default preference values, while the other is the working model that reflects the changes. At save,
1. make a copy of the working model,
2. compare the copy with the default model,
3. remove elements whose values are same as default values,
4. save this copy into the store.
These steps will happen in the preference framework rather than the store.
y z June 12, 2013 at 2:47 PM
Branch with the fix for the issue was committed at: https://github.com/fluid-project/infusion/commit/ef823a4b799854aa84188579f4c7b2164a84b1b4
Michelle D'Souza June 12, 2013 at 2:45 PM
Pull request https://github.com/fluid-project/infusion/pull/323 merged into project repo at ef823a4b799854aa84188579f4c7b2164a84b1b4
UIOptions handling of its default options has always been very brittle. Until the https://fluidproject.atlassian.net/browse/FLUID-4531#icft=FLUID-4531 work, this was held in a central static location as part of the defaults for the SettingsStore component. Even in older days, this caused some problems - when upgrading from one version of UIOptions to one which managed more options, the component would malfunction until the user cleared their cookies - since the defaults record was only used in the case the cookie was empty. Finding any cookie at all would bypass the defaults, presenting the component with some options which were "undefined" and would so not render properly. The component was effectively unusable in this use case.
Just before https://fluidproject.atlassian.net/browse/FLUID-4531#icft=FLUID-4531 as part of FLUID-4607, I (AB) made a change to the store algorithm to always merge in the defaults before returning them. This resolves the issue described above, but introduces a different problem. Now UIOptions stores every option back into the store, including those derived from the defaults, and so in the case the value of a default changes between releases of UIOptions, a user will continue with their originally stored value of the default, even if they didn't make any change expressing a preference.
We need to rethink this architecture before too long, especially since https://fluidproject.atlassian.net/browse/FLUID-4531#icft=FLUID-4531 antification will lead to UIOptions being used in increasingly flexible situations. For now we should just fix up the tests which were invalidated by this change in semantic. A promising architectural scheme for meeting this issue might be to take advantage of the new "source tracking" facility in the ChangeApplier to distinguish which changes are caused by the user and which from a store fetch.