Project

General

Profile

bug #8111

User selection dialog for groups does not filter out existing users

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

Status:
Closed
Priority:
Priority14
Assignee:
Category:
taxeditor
Target version:
Start date:
02/19/2019
Due date:
% Done:

50%

Severity:
normal
Found in Version:

Description

... but shows all users by "*". Same holds for Group selection dialog for Users.

Associated revisions

Revision cbc83794 (diff)
Added by Katja Luther 8 months ago

fix #8111: filter existing users/groups for member/group selection

Revision 2040fc2c (diff)
Added by Katja Luther 7 months ago

ref #8111: fix selection of user

Revision 92749440 (diff)
Added by Katja Luther 7 months ago

fix #8111: fix saving problem for members in group

Revision ea46d0e4 (diff)
Added by Katja Luther 7 months ago

ref #8111: handle event when removing user in group

History

#1 Updated by Katja Luther 8 months ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 50

#2 Updated by Katja Luther 8 months ago

  • Assignee changed from Katja Luther to Andreas Müller

please review

#3 Updated by Andreas Müller 7 months ago

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

WIth test/rem_conf_am I could not open the dialog at all, but got

java.lang.NullPointerException
    at eu.etaxonomy.taxeditor.ui.dialog.selection.UserSelectionDialog.<init>(UserSelectionDialog.java:66)
    at eu.etaxonomy.taxeditor.ui.dialog.selection.UserSelectionDialog.select(UserSelectionDialog.java:48)
    at eu.etaxonomy.taxeditor.ui.dialog.selection.SelectionDialogFactory.getSelectionFromDialog(SelectionDialogFactory.java:178)
    at eu.etaxonomy.taxeditor.ui.selection.EntitySelectionElement.widgetSelected(EntitySelectionElement.java:220)
    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.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)
    a

in the logfile.

#4 Updated by Patrick Plitzner 7 months ago

The dialog is created in SelectionDialogFactory. But null is passed as an argument which leads to this exception. @Katja: Did you forget to push something? This can never work with the current implementation.

        if(clazz.equals(User.class)){
            return (T) UserSelectionDialog.select(shell, //conversation,
                    (User) currentSelection, null);
        }

        if(clazz.equals(Group.class)){
            return (T) GroupSelectionDialog.select(shell, //conversation,
                    (Group) currentSelection, null);
        }

#5 Updated by Katja Luther 7 months ago

Another point, the cascading from group to member is not implemented. Adding or deleting a member is not saved when saving the group. If we provide member selection in editor we should add saveOrUpdate to the cascading.

#6 Updated by Andreas Müller 7 months ago

Katja Luther wrote:

Another point, the cascading from group to member is not implemented. Adding or deleting a member is not saved when saving the group. If we provide member selection in editor we should add saveOrUpdate to the cascading.

Cascading has been explicitly removed for groups and members recently, also to avoid some security issues when deleting a user and having it still as member at other place.

However, I don't understand the argument. Cascading does only save a member which is not yet persisted at all. Membership of an already persisted user is not touched by cascading. As cascading does not exist, we do not allow creating new "Users" via selection dialog anymore. So cascading is probably not needed.

If you think there is still an open issue please give more details what exactly you think does not work.

#7 Updated by Katja Luther 7 months ago

Andreas Müller wrote:

Katja Luther wrote:

Another point, the cascading from group to member is not implemented. Adding or deleting a member is not saved when saving the group. If we provide member selection in editor we should add saveOrUpdate to the cascading.

Cascading has been explicitly removed for groups and members recently, also to avoid some security issues when deleting a user and having it still as member at other place.

However, I don't understand the argument. Cascading does only save a member which is not yet persisted at all. Membership of an already persisted user is not touched by cascading. As cascading does not exist, we do not allow creating new "Users" via selection dialog anymore. So cascading is probably not needed.

If you think there is still an open issue please give more details what exactly you think does not work.

I also thought, that it should be saved, but it is not saved at all. When calling merge() on the group the changes in GrantedAuthorities and label are saved, but not the added or deleted members. I debugged right before calling the merge().

#8 Updated by Katja Luther 7 months ago

the selection dialog is fixed, but the saving still does not work.

#9 Updated by Andreas Müller 7 months ago

Can you write a test which shows that this problem is really due to merge?

By the way when calling merge saveOrUpdate cascading will not help anyway I guess. It should be merge. But as mentioned cascading is not wanted here.

#10 Updated by Andreas Müller 7 months ago

I tested with the latest version. Adding a group to a member seems to work correctly. Adding a user to a group is not stored. This to me looks like a wrong handling of the add methods and/or bidirectionality within TaxEditor.

Can you post the code how members are added?

#11 Updated by Katja Luther 7 months ago

Andreas Müller wrote:

I tested with the latest version. Adding a group to a member seems to work correctly. Adding a user to a group is not stored. This to me looks like a wrong handling of the add methods and/or bidirectionality within TaxEditor.

Can you post the code how members are added?

@Override
    public void addElement(User element) {
        getEntity().addMember(element);
    }

and when debugging on editor side, the member is added to the list of members

#12 Updated by Katja Luther 7 months ago

  • Status changed from Feedback to Resolved

#13 Updated by Katja Luther 7 months ago

  • Assignee changed from Katja Luther to Patrick Plitzner

The problem is solved by mergeing the users added or deleted to a group while saving the bulk editor model.
please review if everything is correct.

#14 Updated by Andreas Müller 7 months ago

  • Status changed from Resolved to Feedback
  • Assignee changed from Patrick Plitzner to Katja Luther

Deleting users from group does not work (after reload it is still there)

#15 Updated by Katja Luther 7 months ago

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

Andreas Müller wrote:

Deleting users from group does not work (after reload it is still there)

sorry missed to push some changes. Now this should work.

#16 Updated by Andreas Müller 7 months ago

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

seems to work now

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 40 MB)