Project

General

Profile

bug #7856

Allow local override for distribution vocabularies

Added by Andreas Müller 12 months ago. Updated 8 months ago.

Status:
Closed
Priority:
Highest
Assignee:
Category:
taxeditor
Target version:
Start date:
10/23/2018
Due date:
% Done:

100%

Severity:
critical
Found in Version:

Description

copied from #7235

Currently local override for TaxDistributionEditor areas allows only to further reduce the vocabularies defined in DB preferences.
However, local override means that preferences can be overriden. So one may choose also other vocabularies, not selected in DB prefs.

So, with local override selected, always all vocabularies should be shown here.

Another question is the behavior if allow override is false. Then the button should be disabled I think. Fine tuning of areas is always possible via the area selection WITHIN the DistributionEditor therefore a second vocabulary filter is not needed.


Related issues

Related to Edit - task #7854: Open issues in TaxDistributionEditor Closed 10/23/2018
Related to Edit - bug #7999: Allow override does not work correctly for distribution areas Closed 01/17/2019
Related to Edit - bug #7880: Implement centralized preference default loading strategy Closed 10/30/2018
Related to Edit - bug #7849: Improve DB Preferences handling and saving in TaxEditor Feedback 10/23/2018
Related to Edit - feature request #8045: Improve handling of default values and override in local preferences Feedback 01/31/2019
Copied from Edit - feature request #7235: Make available area vocabularies a database preference for TaxEditor distribution editor Closed 02/01/2018

Associated revisions

Revision eade8b7d (diff)
Added by Katja Luther 10 months ago

ref #7856: fix allow override for namedArea vocabulary selection for distributions

Revision 7ffc533f (diff)
Added by Katja Luther 10 months ago

ref #7856: fix allow override for namedArea vocabulary selection for distributions

Revision e398666b (diff)
Added by Katja Luther 9 months ago

fix problems in preferences views

Revision f10b2ba0 (diff)
Added by Katja Luther 9 months ago

ref #7856: in admin preference keep distribution editor prefs enabled if the activation is disabled

Revision 0613242b (diff)
Added by Katja Luther 8 months ago

fix #7856: fix NPE

History

#1 Updated by Andreas Müller 12 months ago

  • Copied from feature request #7235: Make available area vocabularies a database preference for TaxEditor distribution editor added

#2 Updated by Andreas Müller 12 months ago

  • Severity changed from normal to critical

NOTE: currently when using the local preferences and deselecting some areas they are not shown anymore when you next time open the selection dialog. Therefore it is not possible to "reselect" (only by restoring to default, but I have not tested this).
This is CRITICAL.

#3 Updated by Andreas Müller 12 months ago

  • Related to task #7854: Open issues in TaxDistributionEditor added

#4 Updated by Katja Luther 10 months ago

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

now it is possible to override the available vocabularies for the distribution editor.please review

#5 Updated by Katja Luther 10 months ago

  • % Done changed from 0 to 50

#6 Updated by Andreas Müller 9 months ago

  • Related to bug #7999: Allow override does not work correctly for distribution areas added

#7 Updated by Andreas Müller 9 months ago

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

#8 Updated by Andreas Müller 9 months ago

See #7880#note-8 , local override or preference does not yet work fully correct

#9 Updated by Andreas Müller 9 months ago

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

#10 Updated by Andreas Müller 9 months ago

Andreas Müller wrote:

See #7880#note-8 , local override or preference does not yet work fully correct

this is about status, not areas, however similar as the issue of this ticket.

#11 Updated by Katja Luther 9 months ago

  • Status changed from Feedback to Resolved

this should be fixed for distribution vocabularies and status.

#12 Updated by Andreas Müller 9 months ago

should I do remove or are there remaining open issues?

#13 Updated by Katja Luther 9 months ago

  • Assignee changed from Katja Luther to Andreas Müller

please review, the other ticket was already closed, so I closed it again and the issue about the problems when there is no db preference is handled in this ticket.

#14 Updated by Andreas Müller 9 months ago

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

Now after selecting another vocabulary (without override flag set, but this is probably not important) and then saving the preferences I got

login : admin
editor version : 5.5.0.201901242349
server : test.e-taxonomy.eu (edit-test) / rem_conf_am
schema version : 5.0.0.0.20180514
os : Windows Server 2012 R2 6.3 amd64
java : 1.8.0_121
java.lang.NullPointerException
    at org.eclipse.core.internal.preferences.EclipsePreferences.put(EclipsePreferences.java:859)
    at org.eclipse.ui.preferences.ScopedPreferenceStore.setValue(ScopedPreferenceStore.java:639)
    at eu.etaxonomy.taxeditor.preference.PreferencesUtil.setStringValue(PreferencesUtil.java:126)
    at eu.etaxonomy.taxeditor.preference.PreferencesUtil.setDisplayStatusInChecklistEditor(PreferencesUtil.java:1366)
    at eu.etaxonomy.taxeditor.preference.ChecklistEditorGeneralPreference.performOk(ChecklistEditorGeneralPreference.java:438)
    at org.eclipse.jface.preference.PreferenceDialog$8.run(PreferenceDialog.java:905)
    at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
    at org.eclipse.ui.internal.JFaceUtil$1.run(JFaceUtil.java:50)
    at org.eclipse.jface.util.SafeRunnable.run(SafeRunnable.java:173)
    at org.eclipse.jface.preference.PreferenceDialog.okPressed(PreferenceDialog.java:889)
    at org.eclipse.jface.preference.PreferenceDialog.buttonPressed(PreferenceDialog.java:230)
    at org.eclipse.jface.dialogs.Dialog$2.widgetSelected(Dialog.java:618)
    at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:249)
    at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
    at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4418)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1079)
    at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4236)
    at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3824)
    at org.eclipse.jface.window.Window.runEventLoop(Window.java:818)
    at org.eclipse.jface.window.Window.open(Window.java:794)
    at eu.etaxonomy.taxeditor.workbench.handler.OpenPreferencesHandler.execute(OpenPreferencesHandler.java:124)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:55)
    at org.eclipse.e4.core.internal.di.InjectorImpl.invokeUsingClass(InjectorImpl.java:282)
    at org.eclipse.e4.core.internal.di.InjectorImpl.invoke(InjectorImpl.java:264)
    at org.eclipse.e4.core.contexts.ContextInjectionFactory.invoke(ContextInjectionFactory.java:132)
    at org.eclipse.e4.core.commands.internal.HandlerServiceHandler.execute(HandlerServiceHandler.java:152)
    at org.eclipse.core.commands.Command.executeWithChecks(Command.java:494)
    at org.eclipse.core.commands.ParameterizedCommand.executeWithChecks(ParameterizedCommand.java:488)
    at org.eclipse.e4.core.commands.internal.HandlerServiceImpl.executeHandler(HandlerServiceImpl.java:210)
    at org.eclipse.e4.ui.workbench.renderers.swt.HandledContributionItem.executeItem(HandledContributionItem.java:433)
    at org.eclipse.e4.ui.workbench.renderers.swt.AbstractContributionItem.handleWidgetSelection(AbstractContributionItem.java:454)
    at org.eclipse.e4.ui.workbench.renderers.swt.AbstractContributionItem$3.handleEvent(AbstractContributionItem.java:482)
    at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
    at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4418)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1079)
    at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4236)
    at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3824)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run(PartRenderingEngine.java:1121)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:336)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1022)
    at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:150)
    at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:693)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:336)
    at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:610)
    at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:148)
    at eu.etaxonomy.taxeditor.Application.start(Application.java:24)
    at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:388)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:243)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:673)
    at org.eclipse.equinox.launcher.Main.basicRun(Main.java:610)
    at org.eclipse.equinox.launcher.Main.run(Main.java:1519)

#15 Updated by Andreas Müller 9 months ago

  • Related to bug #7849: Improve DB Preferences handling and saving in TaxEditor added

#16 Updated by Andreas Müller 9 months ago

Also there is a related issue: if in DB prefs "Activate Distribuiton Editor" does not allow override the complete page is not shown in local prefs. This does not make sense. Maybe it is acceptable if activation is switched off as the page is not relevant then.

A better solution might be to disable those preferences which do not allow override. We can't do this on page level. Maybe needs discussion if there are even better solutions.

Maybe there is a better ticket for this (e.g. #7849). If yes please move.

#17 Updated by Katja Luther 9 months ago

now in admin preferences it is possible to set the distribution editor preferences even if activation is disabled. Locally I think this makes no sense because if the user does not want to use the distribution editor it is not necessary to set the preferences.

#18 Updated by Katja Luther 9 months ago

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

Andreas Müller wrote:

Now after selecting another vocabulary (without override flag set, but this is probably not important) and then saving the preferences I got

login : admin
editor version : 5.5.0.201901242349
server : test.e-taxonomy.eu (edit-test) / rem_conf_am
schema version : 5.0.0.0.20180514
os : Windows Server 2012 R2 6.3 amd64
java : 1.8.0_121
java.lang.NullPointerException
  at org.eclipse.core.internal.preferences.EclipsePreferences.put(EclipsePreferences.java:859)
  at org.eclipse.ui.preferences.ScopedPreferenceStore.setValue(ScopedPreferenceStore.java:639)

is this still reproducable for you? I can not reproduce it

#19 Updated by Andreas Müller 8 months ago

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

Katja Luther wrote:

now in admin preferences it is possible to set the distribution editor preferences even if activation is disabled. Locally I think this makes no sense because if the user does not want to use the distribution editor it is not necessary to set the preferences.

yes I think this is ok. Is it also possible to make the dropdowns grey if disabled? it now looks like they are still enabled (though can't be used). But this is a minor issue.

Also: we have discussed a new handling of "override" for local preferences. I think there is already a ticket for it. Can you link it?

#20 Updated by Andreas Müller 8 months ago

  • % Done changed from 50 to 90

Katja Luther wrote:

Andreas Müller wrote:

Now after selecting another vocabulary (without override flag set, but this is probably not important) and then saving the preferences I got

login : admin
editor version : 5.5.0.201901242349
server : test.e-taxonomy.eu (edit-test) / rem_conf_am
schema version : 5.0.0.0.20180514
os : Windows Server 2012 R2 6.3 amd64
java : 1.8.0_121
java.lang.NullPointerException
    at org.eclipse.core.internal.preferences.EclipsePreferences.put(EclipsePreferences.java:859)
    at org.eclipse.ui.preferences.ScopedPreferenceStore.setValue(ScopedPreferenceStore.java:639)

is this still reproducable for you? I can not reproduce it

I also can't reproduce. If this can't be fixed theoretically (via stacktrace) we may leave it for now.

#21 Updated by Andreas Müller 8 months ago

Except for the minor issues in note-19 and -20 this is fixed now and can be closed.

#22 Updated by Katja Luther 8 months ago

Andreas Müller wrote:

Katja Luther wrote:

now in admin preferences it is possible to set the distribution editor preferences even if activation is disabled. Locally I think this makes no sense because if the user does not want to use the distribution editor it is not necessary to set the preferences.

yes I think this is ok. Is it also possible to make the dropdowns grey if disabled? it now looks like they are still enabled (though can't be used). But this is a minor issue.

Also: we have discussed a new handling of "override" for local preferences. I think there is already a ticket for it. Can you link it?

#8045

#23 Updated by Katja Luther 8 months ago

  • Related to feature request #8045: Improve handling of default values and override in local preferences added

#24 Updated by Katja Luther 8 months ago

  • Status changed from Feedback to Resolved
  • % Done changed from 90 to 50

#25 Updated by Katja Luther 8 months ago

  • Status changed from Resolved to Feedback
  • % Done changed from 50 to 90

Andreas Müller wrote:

Katja Luther wrote:

Andreas Müller wrote:

Now after selecting another vocabulary (without override flag set, but this is probably not important) and then saving the preferences I got

login : admin
editor version : 5.5.0.201901242349
server : test.e-taxonomy.eu (edit-test) / rem_conf_am
schema version : 5.0.0.0.20180514
os : Windows Server 2012 R2 6.3 amd64
java : 1.8.0_121
java.lang.NullPointerException
  at org.eclipse.core.internal.preferences.EclipsePreferences.put(EclipsePreferences.java:859)
  at org.eclipse.ui.preferences.ScopedPreferenceStore.setValue(ScopedPreferenceStore.java:639)

is this still reproducable for you? I can not reproduce it

I also can't reproduce. If this can't be fixed theoretically (via stacktrace) we may leave it for now.

I added a null check to setStringValue, this should fix the potential NPE

#26 Updated by Katja Luther 8 months ago

  • Status changed from Feedback to Closed
  • % Done changed from 90 to 100

#27 Updated by Andreas Müller 8 months ago

But what if the value is null for some reason. Can this happen? Should we handle this somehow (e.g. by converting it to empty string or by throwing an according exception)?

#28 Updated by Katja Luther 8 months ago

Andreas Müller wrote:

But what if the value is null for some reason. Can this happen? Should we handle this somehow (e.g. by converting it to empty string or by throwing an according exception)?

but an empty preference is like no preference at all?

#29 Updated by Andreas Müller 8 months ago

Katja Luther wrote:

Andreas Müller wrote:

But what if the value is null for some reason. Can this happen? Should we handle this somehow (e.g. by converting it to empty string or by throwing an according exception)?

but an empty preference is like no preference at all?

Ok, maybe this is true. But if true, can't we normalize and make all null values to emtpy string. And more important: does deleting preferences still work this way. If you change a preference from an existing object to null is this still reflected with the given implementation? Or is the change simply ignored?

#30 Updated by Katja Luther 8 months ago

Andreas Müller wrote:

Katja Luther wrote:

Andreas Müller wrote:

But what if the value is null for some reason. Can this happen? Should we handle this somehow (e.g. by converting it to empty string or by throwing an according exception)?

but an empty preference is like no preference at all?

Ok, maybe this is true. But if true, can't we normalize and make all null values to emtpy string. And more important: does deleting preferences still work this way. If you change a preference from an existing object to null is this still reflected with the given implementation? Or is the change simply ignored?

yes you are right. so I would set the preference to default?

#31 Updated by Andreas Müller 8 months ago

  • Status changed from Closed to Feedback
  • % Done changed from 100 to 90

Katja Luther wrote:

Andreas Müller wrote:

Katja Luther wrote:

Andreas Müller wrote:

But what if the value is null for some reason. Can this happen? Should we handle this somehow (e.g. by converting it to empty string or by throwing an according exception)?

but an empty preference is like no preference at all?

Ok, maybe this is true. But if true, can't we normalize and make all null values to emtpy string. And more important: does deleting preferences still work this way. If you change a preference from an existing object to null is this still reflected with the given implementation? Or is the change simply ignored?

yes you are right. so I would set the preference to default?

I am not familiar with the local preferences handling so I do not really know. Isn't it possible to completely delete them if they have default value or is this what is automatically doen with default values? We should avoid that an explicit value is saved even if it is the current default value. Default values may change over time. Explicit values don't change the same way while missing values do change together with default values.

#32 Updated by Katja Luther 8 months ago

Andreas Müller wrote:

Katja Luther wrote:

Andreas Müller wrote:

Katja Luther wrote:

Andreas Müller wrote:

But what if the value is null for some reason. Can this happen? Should we handle this somehow (e.g. by converting it to empty string or by throwing an according exception)?

but an empty preference is like no preference at all?

Ok, maybe this is true. But if true, can't we normalize and make all null values to emtpy string. And more important: does deleting preferences still work this way. If you change a preference from an existing object to null is this still reflected with the given implementation? Or is the change simply ignored?

yes you are right. so I would set the preference to default?

I am not familiar with the local preferences handling so I do not really know. Isn't it possible to completely delete them if they have default value or is this what is automatically doen with default values? We should avoid that an explicit value is saved even if it is the current default value. Default values may change over time. Explicit values don't change the same way while missing values do change together with default values.

I would use the setToDefault Method and then it is handled like the preferences in eclipse.

#33 Updated by Katja Luther 8 months ago

I also had a look how it is handled in eclipse preferenceStore and there a null value is handled like nothing. So nothing is set for the preference:

private void setValue(Properties p, String name, String value) {
        Assert.isTrue(p != null && value != null);
        p.put(name, value);
    }

#34 Updated by Andreas Müller 8 months ago

But doesn't Assert throw an exception if one of them is null? This is something else then nothing. Or is the exception caught somehow?

#35 Updated by Katja Luther 8 months ago

No the exception is not caught as long as I can see. But maybe we should handle it the same. Then it is consistent to the eclipse preferences, so null is not allowed.

#36 Updated by Katja Luther 8 months ago

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

I keep it like it was implemented: when the string is null, the default value is used.

#37 Updated by Andreas Müller 8 months ago

  • Status changed from Resolved to Closed
  • Assignee changed from Andreas Müller to Katja Luther
  • Priority changed from New to Highest
  • % Done changed from 90 to 100

Ok, I am not that deep into the code that I could really decide what the best is. I think we can close this ticket. General behavior will be tested with #7849 anyway and further improvements will be done with #8045

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 40 MB)