Arvados

Changes on branch 18123-group-edit-page-2 / f4654fd69770c860e464535c9d2ed51f3338a6cc

I'm having issues initializing the group rename dialog redux form with the existing group name. In groups-panel-actions.ts line 88 I initialize the existing group name and the group UUID into the form so that renameGroup can update the group. The form in /src/views-components/dialog-forms/rename-group-dialog.tsx doesn't seem to receive the initial form value though I'm doing things similarly to the edit permission level dialog.

Relevant snippets:
From groups-panel-actions

export interface RenameGroupFormData {
    [RENAME_GROUP_UUID_FIELD_NAME]: string;
    [RENAME_GROUP_NAME_FIELD_NAME]: string;
}

export const openRenameGroupDialog = (uuid: string) =>
    (dispatch: Dispatch, getState: () => RootState) => {
        const group = getResource<GroupResource>(uuid)(getState().resources);

        if (group) {
            const formData: RenameGroupFormData = {[RENAME_GROUP_UUID_FIELD_NAME]: group.uuid, [RENAME_GROUP_NAME_FIELD_NAME]: group.name};
            console.log("Initialize form: ", formData);
            dispatch(reset(RENAME_GROUP_FORM));
            dispatch<any>(initialize(RENAME_GROUP_FORM, formData));
            dispatch(dialogActions.OPEN_DIALOG({ id: RENAME_GROUP_DIALOG, data: group }));
        }
    };

export const renameGroup = (data: RenameGroupFormData) =>
    async (dispatch: Dispatch, getState: () => RootState, { groupsService }: ServiceRepository) => {
        console.log("RenameGroupFormData", data);
        await groupsService.update(data[RENAME_GROUP_UUID_FIELD_NAME], { name: data[RENAME_GROUP_NAME_FIELD_NAME] });
        dispatch(dialogActions.CLOSE_DIALOG({ id: RENAME_GROUP_DIALOG }));
        dispatch(snackbarActions.OPEN_SNACKBAR({ message: 'Renamed.', hideDuration: 2000, kind: SnackbarKind.SUCCESS }));
    };

From rename-group-dialog:

export const RenameGroupDialog = compose(
    withDialog(RENAME_GROUP_DIALOG),
    reduxForm<RenameGroupFormData>({
        form: RENAME_GROUP_DIALOG,
        // touchOnChange: true,
        onSubmit: (data: RenameGroupFormData, dispatch: Dispatch) => {
            console.log(data);
            // dispatch<any>(renameGroup(data));
        }
    })
)((props: RenameGroupDialogProps) =>
    <FormDialog
        dialogTitle='Rename'
        formFields={RenameGroupFormFields}
        submitLabel='Ok'
        {...props}
    />);

interface RenameGroupDataProps {
    data: GroupResource;
}

type RenameGroupDialogProps = RenameGroupDataProps & WithDialogProps<{}> & InjectedFormProps<RenameGroupFormData>;

const RenameGroupFormFields = (props: RenameGroupDialogProps) => {
    // console.log(props);
    return <>
        <DialogContentText>
            {`Please enter a new name for ${props.data.name}`}
        </DialogContentText>
        <Field
            name={RENAME_GROUP_NAME_FIELD_NAME}
            component={TextField as any}
            autoFocus={true}
            validate={RENAME_FILE_VALIDATION}
        />
        {/* <WarningCollection text="Renaming a file will change the collection's content address." /> */}
    </>;
}

Thanks that helps me narrow down the issue - I didn't realize the breadcrumbs are right clickable.

I tried swapping out my dialog with the project edit dialog, and after finding out that for some reason the execute functions in group-action-set pass in an object with an empty name string:

{
    name: "Rename",
    icon: RenameIcon,
    execute: (dispatch, resource) => {
        // resource.name is empty string in group-action-set.ts
    }
}

{
    icon: RenameIcon,
    name: "Edit project",
    execute: (dispatch, resource) => {
        // resource.name is populated in project-action-set.ts
    }
}

As a workaround I dispatch my openRenameGroupDialog with just the uuid then get the whole object from the store, then dispatch openProjectUpdateDialog) it seems to pass the data correctly when I do this as well as renaming working (besides lack of reloading the table and an error about the project dialog not being open) so I think there's something wrong with my dialog / redux form.

diff --git a/src/store/groups-panel/groups-panel-actions.ts b/src/store/groups-panel/groups-panel-actions.ts
index 9c9f15cf..5714547b 100644
--- a/src/store/groups-panel/groups-panel-actions.ts
+++ b/src/store/groups-panel/groups-panel-actions.ts
@@ -16,6 +16,7 @@ import { snackbarActions, SnackbarKind } from 'store/snackbar/snackbar-actions';
 import { PermissionLevel } from 'models/permission';
 import { PermissionService } from 'services/permission-service/permission-service';
 import { FilterBuilder } from 'services/api/filter-builder';
+import { openProjectUpdateDialog } from 'store/projects/project-update-actions';

 export const GROUPS_PANEL_ID = "groupsPanel";

@@ -82,12 +83,15 @@ export const openRenameGroupDialog = (uuid: string) =>
         const group = getResource<GroupResource>(uuid)(getState().resources);

         if (group) {
-            const formData: RenameGroupFormData = {[RENAME_GROUP_UUID_FIELD_NAME]: group.uuid, [RENAME_GROUP_NAME_FIELD_NAME]: group.name};
-            console.log("Initialize form: ", formData);
-            dispatch(reset(RENAME_GROUP_FORM));
-            dispatch<any>(initialize(RENAME_GROUP_FORM, formData));
-            dispatch(dialogActions.OPEN_DIALOG({ id: RENAME_GROUP_DIALOG, data: group }));
+            dispatch<any>(openProjectUpdateDialog(group));
         }
+        // if (group) {
+        //     const formData: RenameGroupFormData = {[RENAME_GROUP_UUID_FIELD_NAME]: group.uuid, [RENAME_GROUP_NAME_FIELD_NAME]: group.name};
+        //     console.log("Initialize form: ", formData);
+        //     dispatch(reset(RENAME_GROUP_FORM));
+        //     dispatch<any>(initialize(RENAME_GROUP_FORM, formData));
+        //     dispatch(dialogActions.OPEN_DIALOG({ id: RENAME_GROUP_DIALOG, data: group }));
+        // }
     };

I'll look into it more

Changes at arvados-workbench2|5ea93d2e3c077f9cad78f8176a903ced7ceea62e branch 18123-group-edit-page-rebase1
Tests: developer-tests-workbench2: #524

• Without the permissions API, the control-disabling logic for the members tab uses writableBy, which is not exactly correct since it should be manageableBy / can manage
• Outgoing permissions can't easily checked for permissions also without the permissions API, so they are just available and return an error
• There is no add permission dialog, so adding permissions must be done using the sharing dialog of an resource
• Non-admin users cannot see the group's links even with read permissions on the objects

18123-group-edit-page-rebase1 @ arvados-workbench2|5ea93d2e3c077f9cad78f8176a903ced7ceea62e

Groups¶

• "Groups" in the left nav tree needs a different icon than the generic folder
• The "New group" dialog box has an "enter email address" field but not a place to write a description. The "New group" dialog should use the same code as the "Edit" dialog for renaming groups.
• The "Edit" dialog says "Edit project" instead of "Edit group". (It should use the same "Edit project" dialog code, just add a parameter to set the title).

Members tab¶

• "Members" has too many columns, the remove button is scrolled off the screen. We should either prune some columns and/or add a menu that lets you toggle individual columns.
  • The "Email" column seems to be redundant with the "Name" column which is also displaying the email address
  • I don't know if having the user UUID in a column is necessary (and it takes up a lot of horizontal space)
• Instead of "Member hidden", suggest calling it "Visible to other members" and flip the checkbox state (so visible is checked and hidden is unchecked).
• The link on the "name" column goes to the user's home project. It would be better if this went to a page that displays the user's profile. However, that page doesn't exist yet, so we will follow up in a different ticket.
• The "User active" checkbox should should either be made read-only in all cases, or the column removed entirely. We don't want to activate/deactivate users through this particular UI.
• The "Add user" dialog is functionally almost the same as the "sharing" dialog (but the sharing dialog has more features). Could you use the sharing dialog instead? (you might have to add a couple of options to tweak the dialog contents but that should still be simpler than duplicating the functionality).
• The "Name" column says "Project: ..." when the member is actually a role group (in that case it should say "Group: ...").

Permissions tab¶

• It is listing "read" permission to users who are members of the group. This is a bit confusing, and is already captured by the "member hidden / visible to other members" checkbox. Read links to users who are also group members should should be hidden.

Changes at arvados-workbench2|7bf8014027507777a91a0266cf37beaa9841a2d1 18123-group-edit-page-rebase1
Tests: developer-tests-workbench2: #544

• Changed groups icon to fontawesome users icon
• New group now uses the edit project dialog * Added users entry field when creating group
• Title of create/edit group/project reflects project or group
• Removed email and uuid from member list table
• Inverted logic and renamed hidden/visible control
• User active now read only
• Add user uses sharing dialog
• Fixed incorrect labeling of group members
• Skipped hiding read permissions for visible members due to pagination coming from the api

Stephen Smith wrote:

Changes at arvados-workbench2|7bf8014027507777a91a0266cf37beaa9841a2d1 18123-group-edit-page-rebase1
Tests: developer-tests-workbench2: #544

one thing to address:

• There's a field below "Project Name" that says "Enter email addresses" which makes me think I am setting an email address for the whole group or something, not that it is a way to add members. Can you change it to say something like "Search for users to add to the group"

Additional thoughts:

• I noticed that when you add users, by default it doesn't add back links (allowing the group to see individual members). I think that is probably fine, we have been going in the direction of making all users and role groups visible by default, so you don't need group membership to be able to see other users. I appreciate making the checkbox represent "visible" instead of "hidden", it makes that detail more evident.

• I skimmed through the code, looks good, nothing jumped out at me

Changes at arvados-workbench2|04ddc76f953ae6c61264a9c1fa5a8cade9e1773e
Tests developer-tests-workbench2: #550

• Changed new group user input field label