Skip to content

Commit

Permalink
NAS-132503: Non-local (e.g.directory services) users should not be li…
Browse files Browse the repository at this point in the history
…sted as options for group member selector (#11035)
  • Loading branch information
undsoft authored Nov 15, 2024
1 parent 663e7bc commit 0c71811
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
type="button"
color="primary"
ixTest="move-selected-right"
matTooltipPosition="right"
[attr.aria-label]="'Move selected items to the right side list' | translate"
[matTooltip]="'Move selected items to the right side list' | translate"
[disabled]="available.pick.length === 0"
Expand All @@ -51,6 +52,7 @@
type="button"
color="primary"
ixTest="move-all-right"
matTooltipPosition="right"
[disabled]="isAllSelected(available)"
[attr.aria-label]="'Move all items to the right side list' | translate"
[matTooltip]="'Move all items to the right side list' | translate"
Expand All @@ -64,6 +66,7 @@
type="button"
color="primary"
ixTest="move-selected-left"
matTooltipPosition="right"
[attr.aria-label]="'Move selected items to the left side list' | translate"
[matTooltip]="'Move selected items to the left side list' | translate"
[disabled]="confirmed.pick.length === 0"
Expand All @@ -77,6 +80,7 @@
type="button"
color="primary"
ixTest="move-all-left"
matTooltipPosition="right"
[attr.aria-label]="'Move all items to the left side list' | translate"
[matTooltip]="'Move all items to the left side list' | translate"
[disabled]="isAllSelected(confirmed)"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
<mat-card class="form-card">
@if (isFormLoading) {
@if (isLoading()) {
<mat-progress-bar mode="indeterminate"></mat-progress-bar>
}

@if (group?.group) {
@if (group()?.group) {
<mat-card-title>
{{ 'Manage members of {name} group' | translate: { name: group.group } }}
{{ 'Manage members of {name} group' | translate: { name: group().group } }}
@if (requiredRoles?.length && !(hasRequiredRoles | async)) {
<ix-readonly-badge></ix-readonly-badge>
}
</mat-card-title>
}

<mat-card-content>
@if (users.length) {
@if (users().length) {
<ix-dual-listbox
class="padding-16"
display="username"
Expand All @@ -22,8 +22,8 @@
[listItemIcon]="iconMarker('mdi-account')"
[sourceName]="'All Users' | translate"
[targetName]="'Group Members' | translate"
[source]="users"
[destination]="selectedMembers"
[source]="users()"
[(destination)]="selectedMembers"
></ix-dual-listbox>
}
</mat-card-content>
Expand All @@ -34,7 +34,7 @@
mat-button
color="primary"
ixTest="save"
[disabled]="isFormLoading"
[disabled]="isLoading()"
(click)="onSubmit()"
>
{{ 'Save' | translate }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ describe('GroupMembersComponent', () => {
api = spectator.inject(ApiService);
});

it('loads local users to show in available users', () => {
expect(spectator.inject(ApiService).call).toHaveBeenCalledWith('user.query', [[['local', '=', true]]]);
});

it('shows current group values when form is being edited', async () => {
const userList = await loader.getHarness(MatListHarness.with({ selector: '[aria-label="All Users"]' }));
const memberList = await loader.getHarness(MatListHarness.with({ selector: '[aria-label="Group Members"]' }));
Expand All @@ -63,7 +67,6 @@ describe('GroupMembersComponent', () => {
expect(await userList.getItems()).toHaveLength(1);
expect(await memberList.getItems()).toHaveLength(1);

expect(api.call).toHaveBeenCalledWith('user.query');
expect(api.call).toHaveBeenCalledWith('group.query', [[['id', '=', 1]]]);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AsyncPipe } from '@angular/common';
import {
ChangeDetectionStrategy, ChangeDetectorRef, Component, OnInit,
ChangeDetectionStrategy, Component, OnInit, signal,
} from '@angular/core';
import { MatButton } from '@angular/material/button';
import {
Expand All @@ -11,7 +11,7 @@ import { MatProgressBar } from '@angular/material/progress-bar';
import { ActivatedRoute, Router } from '@angular/router';
import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
import { TranslateModule } from '@ngx-translate/core';
import { Observable } from 'rxjs';
import { forkJoin, Observable } from 'rxjs';
import { switchMap } from 'rxjs/operators';
import { RequiresRolesDirective } from 'app/directives/requires-roles/requires-roles.directive';
import { Role } from 'app/enums/role.enum';
Expand Down Expand Up @@ -56,12 +56,11 @@ import { ErrorHandlerService } from 'app/services/error-handler.service';
export class GroupMembersComponent implements OnInit {
protected readonly requiredRoles = [Role.AccountWrite];
protected readonly iconMarker = iconMarker;
protected selectedMembers: User[] = [];
protected readonly users = signal<User[]>([]);

selectedMembers: User[] = [];
users: User[] = [];

isFormLoading = false;
group: Group;
protected readonly isLoading = signal(false);
protected readonly group = signal<Group | null>(null);

get hasRequiredRoles(): Observable<boolean> {
return this.authService.hasRole(this.requiredRoles);
Expand All @@ -73,27 +72,22 @@ export class GroupMembersComponent implements OnInit {
private router: Router,
private dialog: DialogService,
private errorHandler: ErrorHandlerService,
private cdr: ChangeDetectorRef,
private authService: AuthService,
) {}

ngOnInit(): void {
this.isFormLoading = true;
this.isLoading.set(true);
this.activatedRoute.params.pipe(
switchMap((params) => {
return this.api.call('group.query', [[['id', '=', parseInt(params.pk as string)]]]);
}),
switchMap((groups) => {
this.group = groups[0];
this.cdr.markForCheck();
return this.api.call('user.query');
}),
switchMap((params) => forkJoin([
this.api.call('group.query', [[['id', '=', parseInt(params.pk as string)]]]),
this.api.call('user.query', [[['local', '=', true]]]),
])),
untilDestroyed(this),
).subscribe((users) => {
this.users = users;
this.selectedMembers = users.filter((user) => this.group.users.includes(user.id));
this.isFormLoading = false;
this.cdr.markForCheck();
).subscribe(([groups, users]) => {
this.group.set(groups[0]);
this.users.set(users);
this.selectedMembers = users.filter((user) => this.group().users.includes(user.id));
this.isLoading.set(false);
});
}

Expand All @@ -102,20 +96,18 @@ export class GroupMembersComponent implements OnInit {
}

onSubmit(): void {
this.isFormLoading = true;
this.cdr.markForCheck();
this.isLoading.set(true);

const userIds = this.selectedMembers.map((user) => user.id);
this.api.call('group.update', [this.group.id, { users: userIds }]).pipe(
this.api.call('group.update', [this.group().id, { users: userIds }]).pipe(
untilDestroyed(this),
).subscribe({
next: () => {
this.isFormLoading = false;
this.isLoading.set(false);
this.router.navigate(['/', 'credentials', 'groups']);
},
error: (error) => {
this.isFormLoading = false;
this.cdr.markForCheck();
this.isLoading.set(false);
this.dialog.error(this.errorHandler.parseError(error));
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export class CloudBackupFormComponent implements OnInit {
editingTask: CloudBackup;

bucketOptions$ = of<SelectOption[]>([]);
transferSettings$ = this.ws.call('cloud_backup.transfer_setting_choices').pipe(
transferSettings$ = this.api.call('cloud_backup.transfer_setting_choices').pipe(
map((availableSettings) => {
const allOptions = mapToOptions(cloudsyncTransferSettingLabels, this.translate);
return allOptions.filter((option) => availableSettings.includes(option.value as CloudsyncTransferSetting));
Expand Down
1 change: 0 additions & 1 deletion src/assets/i18n/zh-hans.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@
"No proxies added.": "",
"Non expiring": "",
"Not Installed": "",
"Now/Restart": "",
"OS": "",
"Override Admin Email": "",
"Performance Optimization": "",
Expand Down
2 changes: 1 addition & 1 deletion src/assets/icons/sprite-config.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"iconUrl": "assets/icons/sprite.svg?v=889c0f9a7a"
"iconUrl": "assets/icons/sprite.svg?v=c8ca8bd44c"
}
2 changes: 1 addition & 1 deletion src/assets/icons/sprite.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 0c71811

Please sign in to comment.