Project

General

Profile

bug #7849

Improve DB Preferences handling and saving in TaxEditor

Added by Andreas Müller 3 months ago. Updated 7 days ago.

Status:
Resolved
Priority:
New
Category:
taxeditor
Target version:
Start date:
10/23/2018
Due date:
% Done:

0%

Severity:
normal
Found in Version:

Description

Currently it often happens that DB preferences are not (correctly) saved if pressing OK or Apply in DB preferences dialog.

We need a framework that makes it easy to always handle save of preferences correctly.

The framework may include:

  • a registry which returns a list of all available pref.predicates, including the expected default values (for all predicates allow override = true must be part of the default value)
  • when opening TaxEditor and when opening the DB preferences view all registered values are freshly read from DB
    • when opening pref view all values are written to the UI including values for allow override
    • when saving cache is emptied and reloaded for registered list of predicates, all values are read from the UI (if available there), only those values NOT yet handled via UI must NOT be deleted from cache (theses might be experimental feature with no configuration UI available yet)
    • when saving all predicates of the registration are saved to database
      • if value+allowOverride = default value => delete
      • otherwise: save value (API will decide if create or update is needed

====

This ticket should also include a check for all existing UI pages if they work and throw no exceptions (especially with a DB with empty CdmPreference table), see also #7848

====

Further improvements:

  • buttons "Apply" and "OK" should be disabled if no changes are available since last save, this also holds for the local prefs dialog

Related issues

Related to Edit - feature request #7810: DB preference to make all Specimen functionality/menu items invisible Feedback 10/08/2018
Related to Edit - bug #7848: Exceptions for some cdm preference pages Closed 10/23/2018
Related to Edit - feature request #7901: Make PreferencePredicates an interface to allow multiple implementations (e.g. for client apps, for grouping, for data type specific instances, etc.) Closed 11/12/2018
Related to Edit - bug #7236: Make available distribution status a database preference for TaxEditor distribution editor Feedback 02/01/2018
Related to Edit - bug #7880: Implement centralized preference default loading strategy Closed 10/30/2018
Blocked by Edit - feature request #7902: Allow default values for PreferencePredicates In Progress 11/12/2018

Associated revisions

Revision 53d9aad0 (diff)
Added by Patrick Plitzner 3 months ago

fix #7810 Fix saving of specimen DB preferences

  • failed to save when initally no preferences were set

Revision d3ec20ed (diff)
Added by Patrick Plitzner 3 months ago

ref #7849 String externalization and label adjustment

Revision 5e3a2c94 (diff)
Added by Patrick Plitzner 3 months ago

ref #7849 Set override flag to true if no pref was found

Revision cdad4890 (diff)
Added by Patrick Plitzner 3 months ago

ref #7849 Set allowOverride to true

Revision e974354a (diff)
Added by Patrick Plitzner 3 months ago

ref #7849 Fix potential NPE

Revision 39a72cef (diff)
Added by Patrick Plitzner 3 months ago

ref #7849 Always show override checkbox in name details admin prefs

Revision 986b34c8 (diff)
Added by Patrick Plitzner 3 months ago

ref #7849 Temporarily removed general DB preferences

Revision 923fbf1e (diff)
Added by Katja Luther 3 months ago

ref #7849: save ABCD configuration with correct preference subject

Revision bf0c676a (diff)
Added by Patrick Plitzner 3 months ago

ref #7849 code refactoring

Revision b123c0b7 (diff)
Added by Katja Luther 3 months ago

ref #7849: move general db preferences to its own tab to avoid saving if not edited

Revision 432276e4 (diff)
Added by Katja Luther 2 months ago

ref #7849: harmonize admin and local preferences for distribution editor and use drop down instead of radio buttons

Revision c130c5fe (diff)
Added by Katja Luther 2 months ago

ref #7849: do not save preferences if page was not edited

Revision a51915c2 (diff)
Added by Katja Luther 2 months ago

ref #7849: handle exception if value of preference not in Enum for areaDisplay

Revision 185eb33b (diff)
Added by Katja Luther 2 months ago

ref #7849: improve DB preference handling and use editorpreferencePredicates keys also for local preferences

Revision 9181d281 (diff)
Added by Katja Luther about 2 months ago

ref #7849: continue adapt db and local preference pages

Revision 12d75812 (diff)
Added by Katja Luther about 2 months ago

ref #7849: remove OVERRIDE constants from IPreferenceKeys and handle it by method

Revision fbd932cc (diff)
Added by Katja Luther about 1 month ago

ref #7849: disable apply button until something was changed in preference page

History

#1 Updated by Andreas Müller 3 months ago

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.

#2 Updated by Andreas Müller 3 months ago

  • Description updated (diff)

#3 Updated by Andreas Müller 3 months ago

  • Related to feature request #7810: DB preference to make all Specimen functionality/menu items invisible added

#4 Updated by Andreas Müller 3 months ago

  • Description updated (diff)

#5 Updated by Andreas Müller 3 months ago

  • Related to bug #7848: Exceptions for some cdm preference pages added

#6 Updated by Andreas Müller 3 months ago

  • Description updated (diff)

#7 Updated by Andreas Müller 3 months ago

  • Description updated (diff)

#8 Updated by Andreas Müller 3 months ago

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.

#9 Updated by Patrick Plitzner 3 months ago

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

#10 Updated by Andreas Müller 3 months ago

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"

#11 Updated by Katja Luther 3 months ago

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"

#12 Updated by Katja Luther 3 months ago

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

#13 Updated by Andreas Müller 3 months ago

  • Assignee changed from Patrick Plitzner to Katja Luther
  • Target version changed from Release 5.4 to Release 5.5

#14 Updated by Katja Luther 3 months ago

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

#15 Updated by Andreas Müller 2 months ago

#16 Updated by Andreas Müller 2 months ago

  • Related to feature request #7901: Make PreferencePredicates an interface to allow multiple implementations (e.g. for client apps, for grouping, for data type specific instances, etc.) added

#17 Updated by Andreas Müller 2 months ago

  • Related to bug #7236: Make available distribution status a database preference for TaxEditor distribution editor added

#18 Updated by Andreas Müller 7 days ago

  • Related to bug #7880: Implement centralized preference default loading strategy added

#19 Updated by Andreas Müller 7 days ago

  • Status changed from New to Resolved
  • Assignee changed from Katja Luther to Andreas Müller

Can this be reviewed?

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 40 MB)