EDIT

AM:

Grundsätzlich ist das Problem nicht ganz trivial, da das Zusammenspiel zwischen Default Werten, geänderten Werten und Cache funktionieren muss.

Wenn ein Wert geändert wurde und somit vom Default abweicht, muss er bei „Apply“ oder „OK“ sowohl in der DB als auch im Cache gespeichert werden.
Was allerdings möglichst nicht passieren sollte ist, dass Werte, die gar nicht geändert wurden auch gespeichert werden. Das war am Anfang der Fall, da hat Katja quasi alle möglichen Werte ausgelesen und in die DB geschrieben, es sollen aber möglichst nur die Abweichungen rein.

Zu diskutieren wäre dann noch, ob Werte die zurück geändert werden auf den Default Wert aus der DB gelöscht werden sollen, oder ob sie drin bleiben sollen, wenn einmal existent (was vermutlich leichter ist zu implementieren).

Ich fände es übrigens auch gut, wenn beim Öffnen des DB Preferences Dialogs grundsätzlich alle Werte aus der DB ausgelesen werden und der Cache gelernt und mit den aktuellsten Werten gefüllt wird. So ist man auf der sichereren Seite, auch, wenn mal mehrere Leute an den Preferences arbeiten.

Das Problem ist aber eher, dass zuviel gespeichert wird.
Wenn man nur den DB Preferences Dialog öffnet und wieder schließt werden bereits 5 Preferences gespeichert.
Unter anderem die, welches common name area Vokabular verwendet werden soll, nämlich KEINS. Override nicht möglich.

Das führt beim Versuch einen Common Name Area auszuwählen dann z.B. zu:

java.lang.NullPointerException
    at eu.etaxonomy.taxeditor.ui.dialog.selection.CommonNameNamedAreaSelectionDialog.getAvailableVocabularies(CommonNameNamedAreaSelectionDialog.java:74)
    at eu.etaxonomy.taxeditor.ui.dialog.selection.CommonNameNamedAreaSelectionDialog.init(CommonNameNamedAreaSelectionDialog.java:64)
    at eu.etaxonomy.taxeditor.ui.dialog.selection.AbstractFilteredCdmResourceSelectionDialog.<init>(AbstractFilteredCdmResourceSelectionDialog.java:103)

Das ist ein grundsätzliches Problem, dass etwas gespeichert wird auch wenn der User überhaupt nichts geändert hat, welches auch zeigt, dass an der Architektur noch etwas nicht stimmt.
Ich glaube, ohne Default Werte zu hinterlegen (wo auch immer), kommen wir da nicht weiter.
Am besten sollten diese gar nicht erst gespeichert werden, aber wenn etwas gespeichert wird, dann sollte wenigstens der hinterlegte Default gespeichert werden, so dass man nicht versehentlich eine Preference anlegt, die man gar nicht wollte.

"Änderungen an ABCD Import werden nicht gespeichert, außer allowOverride, aber auch das wird nicht richtig reloaded beim erneuten Öffnen"

If the NomenclaturalCode is once set it can not be removed anymore in the DB Pref display even if the Preferences is manually removed from the database. It looks like the dropdown uses the local preference for display.
Also the AllowOverride is implemented in the wrong way here. In DB Pref UI there is no checkbox while in local Prefs there is a checkbox saying "use local xxx"

Andreas Müller wrote:

If the NomenclaturalCode is once set it can not be removed anymore in the DB Pref display even if the Preferences is manually removed from the database. It looks like the dropdown uses the local preference for display.

The preferred nomenclatural code is set when the editor is started, in the wizard the actual code is set to this preferred nomenclatural code if the pref does not exist.
But maybe it is better to keep it empty.

Also the AllowOverride is implemented in the wrong way here. In DB Pref UI there is no checkbox while in local Prefs there is a checkbox saying "use local xxx"

Patrick Plitzner wrote:

"Änderungen an ABCD Import werden nicht gespeichert, außer allowOverride, aber auch das wird nicht richtig reloaded beim erneuten Öffnen"

this again was the wrong preference subject

The general DB preferences are moved now to an extra page so they are not stored, if not opened.

??

If override is not set in local prefs the value can be changed but it is not saved with Apply/OK. Is this wanted behavior? It is not intuitive and unexpected for the user. If something can not be changed, it should be disabled. Needs discussion if editing should be allowed in this case. Tested with distribution editor.

With specimen page I tested if this is also the case if no DB pref is set and it looks like if no DB pref exists changes are persisted. This is critical especially because currently default value DB prefs are stored in the DB even if nobody set them on purpose (see above).

Personally I think, the value should always be editable and persisted (except for the case when override is not allowed)

Special case in Specimen page of local prefs: In a fresh database "Zeige specimenbezogene Views und Menüeinträge" is disabled but shouldn't (default is to show specimen menüs).

Also: if the above ceckbox is unchecked the below 4 checkboxes are still shown after reopening the preferences but shouldn't. Behavior when opening the preferences should be same as when changing the value of the above checkbox.

It looks like in the DB prefs specimen page the 4 below prefs are not set to allow override by default. ALL preference should by default allow override.

Andreas Müller wrote:

It looks like in the DB prefs specimen page the 4 below prefs are not set to allow override by default. ALL preference should by default allow override.

this is fixed.

Andreas Müller wrote:

Critical: DB prefs for Name Details View do not seem to exist anymore

this is fixed

Andreas Müller wrote:

Special case in Specimen page of local prefs: In a fresh database "Zeige specimenbezogene Views und Menüeinträge" is disabled but shouldn't (default is to show specimen menüs).

Also: if the above ceckbox is unchecked the below 4 checkboxes are still shown after reopening the preferences but shouldn't. Behavior when opening the preferences should be same as when changing the value of the above checkbox.

this is fixed

Andreas Müller wrote:

If default value is selected and allow override = true the preference is still not deleted from the database. This is not really critical but problematic as it requires that some chooses "override" in the local prefs to make local prefs work, though nobody really set a DB preference (with no DB preference set override should not be required to make local prefs works, otherwise we could remove the "override" from local prefs)

but this should be handled on serverside?

Katja Luther wrote:

Andreas Müller wrote:

If default value is selected and allow override = true the preference is still not deleted from the database. This is not really critical but problematic as it requires that some chooses "override" in the local prefs to make local prefs work, though nobody really set a DB preference (with no DB preference set override should not be required to make local prefs works, otherwise we could remove the "override" from local prefs)

but this should be handled on serverside?

Unfortunately this is more difficult then expected because CdmPreference does only contain a String as Predicate. So the default value is missing. For enum values available on server side this can be handled, but for others it is not possible. Created new ticket for this issue #8062

Andreas Müller wrote:

On Specimen page, when "Show specimen related views" is unchecked but allow override = true the specific preferences like "Collecting areas in general section" should still be visible. This is because if allow override is switched local preference may still define these preferences and then you need to be able to still define certain preferences DB wide.

Same applies for Name Details View (and similar pages in future).

For distribution editor this is already implemented, BUT here we could think about hiding the page if "Enable Distribution Editor" is unchecked and also "Allow override" is unchecked". In this case it does not make sense to allow defining any specific preferences.

Distribution Editor page: "Show rank in..." and "Sort areas by order ..." are local enabled even if allow override is unchecked in DB prefs. This is not correct. For "Display of areas .." it seems to be correct.

Distribution Editor page local prefs: "override" is missing in distribution status selection, seems to be correct in area selection

Andreas Müller wrote:

Distribution Editor page: "Show rank in..." and "Sort areas by order ..." are local enabled even if allow override is unchecked in DB prefs. This is not correct. For "Display of areas .." it seems to be correct.

this should be fixed

Andreas Müller wrote:

Distribution Editor page local prefs: "override" is missing in distribution status selection, seems to be correct in area selection

is fixed

now for specimen, name details and distribution editor the details are only disabled not invisible when the more global preference is unchecked.

I would expect if DB pref distribution status is set with allow override and local prefs are not yet set that when opening the dialog the dialog is prefilled with the DB pref status list.

Katja Luther wrote:

now for specimen, name details and distribution editor the details are only disabled not invisible when the more global preference is unchecked.

for distribution editor preferences are still editable if "Show ..." and "allow override" are both unchecked, but should not be editable

for specimen and name details the specific prefs are still not yet editable if "allow override" is checked but should be editable

I think everything else is fixed. Open issues note-33 (maybe needs discussion) and note-42 and note-43

Andreas Müller wrote:

Katja Luther wrote:

now for specimen, name details and distribution editor the details are only disabled not invisible when the more global preference is unchecked.

for distribution editor preferences are still editable if "Show ..." and "allow override" are both unchecked, but should not be editable
this is fixed

for specimen and name details the specific prefs are still not yet editable if "allow override" is checked but should be editable

the name details can be edited, but first the use of the simplified view has to be checked to be able to edit the details

Do I understand correctly that the specimen prefs all are only depending on the allow override? So if "Show specimen related views" is unchecked the other prefs should still be editable?

Andreas Müller wrote:

I would expect if DB pref distribution status is set with allow override and local prefs are not yet set that when opening the dialog the dialog is prefilled with the DB pref status list.

is implemented

The above is fixed with commits ed86aee4 ff but still should be reviewed by KL.

Again I get "The currently displayed page contains invalid values" for a DB with no DBprefs defined and also no local prefs defined for page "Distribution Editor" on DB prefs. This is critical.

This seems to be the reason:

java.lang.NullPointerException
    at eu.etaxonomy.taxeditor.preference.ChecklistEditorGeneralPreference.createContents(ChecklistEditorGeneralPreference.java:315)
    at eu.etaxonomy.taxeditor.databaseAdmin.preferencePage.ChecklistEditorGeneralAdminPreference.createContents(ChecklistEditorGeneralAdminPreference.java:32)
    at org.eclipse.jface.preference.PreferencePage.createControl(PreferencePage.java:241)

Common name areas: when opening the dialog it is always empty but should show the vocabularies selected before

Andreas Müller wrote:

Common name areas: when opening the dialog it is always empty but should show the vocabularies selected before

Maybe it is not stored correctly in the cache. Even if 1 vocabulary is selected all vocabularies are shown for selection in common name area selection dialog.

Also it is not possible to delete this preference by setting it to default. If no vocabulary is selected an empty string is persisted. Default is null this might be the reason why it is not recognized as default.

Generally we need to discuss how to set the vocabulary/term-list selections to default. Is it by a certain selection, or with a button (in dialog or on preference page)?

Also we need to discuss if an empty list should be allowed at all or if we disable "Finish" if no entry has been selected

Also, if no preference exists yet (default) all vocabularies should be selected in the selection dialog as this is similar to default.

Also for common names there is "Allow override" in the dialog and on the page. Preferred is on the page, but definetely only 1 should exist (or they need to be synchronized but currently they aren't)

The "Restore Default" generally does not seem to work for DB Prefs. We should either disable/remove it or make it working.

Andreas Müller wrote:

Common name areas: when opening the dialog it is always empty but should show the vocabularies selected before

this should be fixed

The predicate "model.editor.NDV" always gets persisted once DB prefs are opened and saved even if NDV page was not opened or touched at all. Additionally "allow override" is set to "false" here. Default should always be "true".
This is critical as it prevents users to define there own reduced name details view.

Nomenclatural Code: Changing allow override does not set "Apply" to enabled.

Name Details View: by default when switching to "simplified" only NameCache, AuthorCache and Approbation is selected. By default I would expect all to be selected.

Name Details View: "Secundum Ref Details and Secundum activated should have identation to make clear that they belong to "Taxon". Also they should have a different order, first Secundum activated then Secundum Ref Details.

Critical: Name Details View: changes in DB prefs have no effect. Unselected controls are still shown in the view.

I did not check the ABCD stuf as I am not so familar. Can you please check yourself?

Only minor issues:

• Nomenclatural Code is missing a label there.
• The "New Provider" field is missing a label, e.g. "New provider".
• Adding a provider leads to an empty line in the first text field if it was the first provider.
• The new provider should be in the first line in this case.
• No URL check is done, you can also enter "aaa".
• The list field is to small (height) if > 1 providers are entered
• "Add provider" button should be disabled if "New provider" text field is empty
• "Remove provider" should be disabled if no provider is selected in provider list

Andreas Müller wrote:

After trying to set values to default values I could not open the preference page anymore:

login : admin
editor version : 5.5.0.201902072350
server : test.e-taxonomy.eu (edit-test) / test1
schema version : 5.0.0.0.20180514
os : Windows Server 2012 R2 6.3 amd64
java : 1.8.0_121
java.lang.IllegalArgumentException: No enum constant eu.etaxonomy.cdm.model.metadata.TermDisplayEnum.Label
  at java.lang.Enum.valueOf(Enum.java:238)
  at eu.etaxonomy.cdm.model.metadata.TermDisplayEnum.valueOf(TermDisplayEnum.java:1)
  at eu.etaxonomy.taxeditor.preference.ChecklistEditorGeneralPreference.createContents(ChecklistEditorGeneralPreference.java:362)
  at eu.etaxonomy.taxeditor.databaseAdmin.preferencePage.ChecklistEditorGeneralAdminPreference.createContents(ChecklistEditorGeneralAdminPreference.java:32)
  at org.eclipse.jface.preference.PreferencePage.createControl(PreferencePage.java:241)
  at eu.etaxonomy.taxeditor.preference.menu.CdmPreferencePage.createControl(CdmPreferencePage.java:57)
  at org.eclipse.jface.preference.PreferenceDialog.createPageControl(PreferenceDialog.java:1430)
  at org.eclipse.jface.preference.PreferenceDialog$9.run(PreferenceDialog.java:1197)
  at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
...

this is fixed.

Andreas Müller wrote:

Critical: Name Details View: changes in DB prefs have no effect. Unselected controls are still shown in the view.

this should be fixed with last commit

Andreas Müller wrote:

The predicate "model.editor.NDV" always gets persisted once DB prefs are opened and saved even if NDV page was not opened or touched at all. Additionally "allow override" is set to "false" here. Default should always be "true".
This is critical as it prevents users to define there own reduced name details view.

this is fixed

Andreas Müller wrote:

Also, if no preference exists yet (default) all vocabularies should be selected in the selection dialog as this is similar to default.

for the presenceAbsence term selection this is critical at the moment because we can't store all term uuids in the db.

Katja Luther wrote:

Andreas Müller wrote:

Also, if no preference exists yet (default) all vocabularies should be selected in the selection dialog as this is similar to default.

for the presenceAbsence term selection this is critical at the moment because we can't store all term uuids in the db.

now for vocabularies it is implemented that all vocs are checked if no preference exist but for terms it is still all unchecked.

Katja Luther wrote:

Andreas Müller wrote:

Also, if no preference exists yet (default) all vocabularies should be selected in the selection dialog as this is similar to default.

for the presenceAbsence term selection this is critical at the moment because we can't store all term uuids in the db.

Hmm, generally this is not related to default values but to long lists, but will be solved with next model change (one reason why I wanted the model changes within this milestone).
And my idea was not to always store the complete list but to say that default is that all terms are shown and therefore opening the dialog the first time it should show the default state which is all terms selected. However, it is difficult then to distinguish that someone selects all values explicitly from simply default mode.
As selecting NO term does not make much sense using "No terms selected" = default = "All terms are available" is maybe easier to handle. If we decide to do it this way we at least should leave an explicit note in the dialog saying that No selection means any existing term is available. Though this is not really intuitive.
Alternatively we could add a checkbox labeled "All" and if the checkbox is checked. The termlist is disabled and all terms/vocabularies are checked. If "All" is unchecked the terms are shown as currently with first state all terms being selected. During save "All" checked is taken as default value => null is stored (with allow override = true it is even deleted), otherwise handling like always.
I prefer the second solution.

Andreas Müller wrote:

Katja Luther wrote:

Andreas Müller wrote:

Also, if no preference exists yet (default) all vocabularies should be selected in the selection dialog as this is similar to default.

for the presenceAbsence term selection this is critical at the moment because we can't store all term uuids in the db.

Hmm, generally this is not related to default values but to long lists, but will be solved with next model change (one reason why I wanted the model changes within this milestone).
And my idea was not to always store the complete list but to say that default is that all terms are shown and therefore opening the dialog the first time it should show the default state which is all terms selected. However, it is difficult then to distinguish that someone selects all values explicitly from simply default mode.
As selecting NO term does not make much sense using "No terms selected" = default = "All terms are available" is maybe easier to handle. If we decide to do it this way we at least should leave an explicit note in the dialog saying that No selection means any existing term is available. Though this is not really intuitive.
Alternatively we could add a checkbox labeled "All" and if the checkbox is checked. The termlist is disabled and all terms/vocabularies are checked. If "All" is unchecked the terms are shown as currently with first state all terms being selected. During save "All" checked is taken as default value => null is stored (with allow override = true it is even deleted), otherwise handling like always.
I prefer the second solution.

Actually it is only the status selection wizard and for this I would prefer to have a message that if nothing is selected all terms are available. For the next release I already implemented that for a not existing preference all terms are checked.

Andreas Müller wrote:

Name Details View: "Secundum Ref Details and Secundum activated should have identation to make clear that they belong to "Taxon". Also they should have a different order, first Secundum activated then Secundum Ref Details.

is implemented

Andreas Müller wrote:

I did not check the ABCD stuf as I am not so familar. Can you please check yourself?

Only minor issues:

• Nomenclatural Code is missing a label there.
• The "New Provider" field is missing a label, e.g. "New provider".
• Adding a provider leads to an empty line in the first text field if it was the first provider.
• The new provider should be in the first line in this case.
• No URL check is done, you can also enter "aaa".
• The list field is to small (height) if > 1 providers are entered
• "Add provider" button should be disabled if "New provider" text field is empty
• "Remove provider" should be disabled if no provider is selected in provider list

move this to an extra ticket #8089

now the setting of defaults is implemented for db and for local preferences.

For setting the term/vocabulary selections to default the button of the preference page has to be used. Maybe we should move this to the selection wizard in next release -> new ticket ( #8092)

Andreas Müller wrote:

Also it is not possible to delete this preference by setting it to default. If no vocabulary is selected an empty string is persisted. Default is null this might be the reason why it is not recognized as default.

Generally we need to discuss how to set the vocabulary/term-list selections to default. Is it by a certain selection, or with a button (in dialog or on preference page)?

Also we need to discuss if an empty list should be allowed at all or if we disable "Finish" if no entry has been selected

This is not yet correctly implemented. The list of common name vocabularies is deleted as preference when no vocabulary is selected. But when reopening all vocabularies are selected as this is considered to be the default value. I added this to the follow up ticket #8045

With empty preferences when only the DistrEditor page is opened without any change afterwards the preferences for displayStatus and area are stored with value "Label". Default value handling seem not yet correct here.

ON DB prefs page Name Details View the taxon sec attributes are disabled if NDV is disabled but they are not grey. Only when uncheck NDV display on the page they are also grey as all others => #8385

On local prefs Name Details View page the "Restore Defaults" button seems to be enabled even if NDV does not allow override on server side. This is unexpected. I did not check if it also has any effect to press the button in this case.

Restore defaults on local prefs restored the defaults stored in the predicates. One could also expect the current DB prefs (which allow override) to be set as default. However, this problem will probably be solved automatically in #8045 with new implementation so it is not so important.

Nomenclatural Code in DB prefs can not be deleted by setting to defaults. Something wrong with the deleting if value is default. => see #8045#note-10

Andreas Müller wrote:

With empty preferences when only the DistrEditor page is opened without any change afterwards the preferences for displayStatus and area are stored with value "Label". Default value handling seem not yet correct here.

seems to be fixed

Andreas Müller wrote:

On local prefs Name Details View page the "Restore Defaults" button seems to be enabled even if NDV does not allow override on server side. This is unexpected. I did not check if it also has any effect to press the button in this case.

seems to be fixed