[BUG] There is no pop-up window when user is deleted from Project/Notebook level #374#706
[BUG] There is no pop-up window when user is deleted from Project/Notebook level #374#706IamBigBobby wants to merge 6 commits into
Conversation
# Conflicts: # indigo-frontend/src/core/components/common/team/team.component.ts
| type="checkbox" | ||
| class="mt-0.5" | ||
| [(ngModel)]="removeFromChildren" | ||
| [attr.aria-label]="'Also remove this member from all Experiments in this Notebook.'" |
There was a problem hiding this comment.
This dialog can be shown when deleting from Project or Notebook. Wordings must also change depending on it (e.g. "Also remove this member from all Experiments in this Notebook" vs "Also remove this member from all Notebooks and Experiments in this Project")
Also, please update the visible text in span to match aria-label (current wording "Also removes from this Projects, and ..." is a little broken)
There was a problem hiding this comment.
fixed
now it manages form getCascadeCheckboxLabel method in team.component.ts, depends on endpoint:
private getCascadeCheckboxLabel(endpoint: string): string | undefined {
if (endpoint.startsWith('projects/')) {
return 'Also remove this member from all Notebooks and Experiments in this Project.';
}
if (endpoint.startsWith('notebooks/')) {
return 'Also remove this member from all Experiments in this Notebook.';
}
return undefined;
}
| const updatePayload: ACLUpdate = { | ||
| username: member.username, | ||
| level: newLevel, | ||
| ...(newLevel === AclLevel.NONE ? { deleteNested } : {}), |
There was a problem hiding this comment.
Minor, but can be newLevel === AclLevel.None ? deleteNested : false (or maybe newLevel === AclLevel.None && deleteNested)
false is a safe value to pass even if AclLevel is other than None
| this._team.set(updated); | ||
| this.teamChanged.emit(updated); | ||
| } | ||
| if (!projectAcl) return; |
There was a problem hiding this comment.
It looks like this check validates against backend returning null, but it would also fail on empty list. We can't have empty list here (there should be at least author), but it's more clear if we remove this check. Backend always returns ACLEntry[]
|
|
||
| export interface RemoveMemberConfirmationDialogData { | ||
| showCascadeCheckbox: boolean; | ||
| title?: string; |
There was a problem hiding this comment.
Seems fields other that showCascadeCheckbox never used?
| readonly data = inject<RemoveMemberConfirmationDialogData>(MAT_DIALOG_DATA); | ||
| private dialogRef = inject(MatDialogRef<RemoveMemberConfirmationDialogComponent, RemoveMemberConfirmationResult>); | ||
|
|
||
| removeFromChildren = true; |
There was a problem hiding this comment.
We now have 3 places where eln-team component is used: Project, Notebook, Experiment. First two should have removeFromChildren = true, Experiment should have removeFromChildren = false
There was a problem hiding this comment.
refactored, now it depends on depends on availability cascadeCheckboxLabel field (look at team.compoent.ts)
private getCascadeCheckboxLabel(endpoint: string): string | undefined {
if (endpoint.startsWith('projects/')) {
return 'Also remove this member from all Notebooks and Experiments in this Project.';
}
if (endpoint.startsWith('notebooks/')) {
return 'Also remove this member from all Experiments in this Notebook.';
}
return undefined;
}
private buildRemoveMemberDialogData(endpoint: string): RemoveMemberConfirmationDialogData {
const cascadeCheckboxLabel = this.getCascadeCheckboxLabel(endpoint);
return {
showCascadeCheckbox: !!cascadeCheckboxLabel,
cascadeCheckboxLabel,
};
}
| readonly data = inject<RemoveMemberConfirmationDialogData>(MAT_DIALOG_DATA); | ||
| private dialogRef = inject(MatDialogRef<RemoveMemberConfirmationDialogComponent, RemoveMemberConfirmationResult>); | ||
|
|
||
| readonly cascadeCheckboxLabel = |
There was a problem hiding this comment.
Maybe better have non-nullable cascadeCheckboxLabel and demand dialog caller to always provide the text?
| this.teamChanged.emit(updatedTeam); | ||
| } | ||
|
|
||
| private getCascadeCheckboxLabel(endpoint: string): string | undefined { |
There was a problem hiding this comment.
Looks a little strange that we calculate endpoint based on entity type and then calculate the message based on that endpoint. Logically, it should depend on entity type also. Can we instead add a message to TeamComponentConfig?
| @@ -0,0 +1,16 @@ | |||
| <div class="p-6"> | |||
There was a problem hiding this comment.
Shall we reuse eln-form-dialog? It will give us title and buttons for free
2 participants
task: #374
screenshots:
Project page:
Notebook page:
Experiments page: