Project

General

Profile

Actions

bug #8111

closed

User selection dialog for groups does not filter out existing users

Added by Andreas Müller about 5 years ago. Updated almost 5 years ago.

Status:
Closed
Priority:
Priority14
Assignee:
Category:
taxeditor
Target version:
Start date:
Due date:
% Done:

50%

Estimated time:
Severity:
normal
Found in Version:

Description

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

Actions #1

Updated by Katja Luther about 5 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 50
Actions #2

Updated by Katja Luther about 5 years ago

  • Assignee changed from Katja Luther to Andreas Müller

please review

Actions #3

Updated by Andreas Müller almost 5 years 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.

Actions #4

Updated by Patrick Plitzner almost 5 years 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);
        }
Actions #5

Updated by Katja Luther almost 5 years 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.

Actions #6

Updated by Andreas Müller almost 5 years 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.

Actions #7

Updated by Katja Luther almost 5 years 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().

Actions #8

Updated by Katja Luther almost 5 years ago

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

Actions #9

Updated by Andreas Müller almost 5 years 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.

Actions #10

Updated by Andreas Müller almost 5 years 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?

Actions #11

Updated by Katja Luther almost 5 years 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

Actions #12

Updated by Katja Luther almost 5 years ago

  • Status changed from Feedback to Resolved
Actions #13

Updated by Katja Luther almost 5 years 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.

Actions #14

Updated by Andreas Müller almost 5 years 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)

Actions #15

Updated by Katja Luther almost 5 years 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.

Actions #16

Updated by Andreas Müller almost 5 years ago

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

seems to work now

Actions

Also available in: Atom PDF