Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

General: Improve user administration #9533

Merged
merged 2 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
<div class="row justify-content-center">
<div class="col-md-8">
<form name="editForm" role="form" novalidate (ngSubmit)="save()" [formGroup]="editForm">
<h2 id="myUserLabel" jhiTranslate="artemisApp.userManagement.home.createOrEditLabel"></h2>
@if (user.id === undefined) {
<h2 id="createUser" jhiTranslate="artemisApp.userManagement.home.createLabel"></h2>
} @else {
<h2 id="editUser" jhiTranslate="artemisApp.userManagement.home.editLabel"></h2>
}
<div>
<div class="form-group">
<label for="login" class="form-control-label" jhiTranslate="artemisApp.userManagement.login"></label>
Expand Down Expand Up @@ -83,7 +87,7 @@ <h2 id="myUserLabel" jhiTranslate="artemisApp.userManagement.home.createOrEditLa
</div>
<div class="form-check">
<label for="internal" class="form-check-label" ngbTooltip="{{ 'artemisApp.userManagement.passwordTooltip' | artemisTranslate }}">
<input class="form-check-input" type="checkbox" id="internal" name="internal" formControlName="internalInput" [(ngModel)]="user.internal" />
<input class="form-check-input" type="checkbox" id="internal" name="internal" formControlName="internalInput" />
<span jhiTranslate="artemisApp.userManagement.internal"></span>
</label>
<jhi-help-icon text="artemisApp.userManagement.passwordTooltip" />
Expand Down Expand Up @@ -255,15 +259,7 @@ <h5>
</div>
<div class="form-check">
<label for="activated" class="form-check-label">
<input
class="form-check-input"
[disabled]="user.id === null"
type="checkbox"
id="activated"
name="activated"
formControlName="activatedInput"
[(ngModel)]="user.activated"
/>
<input class="form-check-input" type="checkbox" id="activated" name="activated" [(ngModel)]="user.activated" />
krusche marked this conversation as resolved.
Show resolved Hide resolved
<span jhiTranslate="artemisApp.userManagement.activated"></span>
</label>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { MatAutocompleteSelectedEvent } from '@angular/material/autocomplete';
import { AlertService, AlertType } from 'app/core/util/alert.service';
import { ProfileService } from 'app/shared/layouts/profiles/profile.service';
import { AdminUserService } from 'app/core/user/admin-user.service';
import { CourseManagementService } from 'app/course/manage/course-management.service';
import { Observable } from 'rxjs';
import { map, startWith } from 'rxjs/operators';
import { CourseAdminService } from 'app/course/manage/course-admin.service';
Expand Down Expand Up @@ -59,7 +58,6 @@ export class UserManagementUpdateComponent implements OnInit {
constructor(
private languageHelper: JhiLanguageHelper,
private userService: AdminUserService,
private courseManagementService: CourseManagementService,
private courseAdminService: CourseAdminService,
private route: ActivatedRoute,
private organizationService: OrganizationManagementService,
Expand Down Expand Up @@ -232,11 +230,17 @@ export class UserManagementUpdateComponent implements OnInit {
passwordInput: ['', [Validators.minLength(PASSWORD_MIN_LENGTH), Validators.maxLength(PASSWORD_MAX_LENGTH)]],
emailInput: ['', [Validators.required, Validators.minLength(this.EMAIL_MIN_LENGTH), Validators.maxLength(this.EMAIL_MAX_LENGTH)]],
registrationNumberInput: ['', [Validators.maxLength(this.REGISTRATION_NUMBER_MAX_LENGTH)]],
activatedInput: ['', []],
activatedInput: [{ value: this.user.activated }],
langKeyInput: ['', []],
authorityInput: ['', []],
internalInput: [{ value: this.user.internal, disabled: true }],
internalInput: [{ value: this.user.internal, disabled: true }], // initially disabled, will be enabled if user.id is undefined
});
// Conditionally enable or disable 'internalInput' based on user.id
if (this.user.id !== undefined) {
this.editForm.get('internalInput')?.disable(); // Artemis does not support to edit the internal flag for existing users
} else {
this.editForm.get('internalInput')?.enable(); // New users can either be internal or external
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ <h2>
name="searchTerm"
id="field_searchTerm"
formControlName="searchControl"
[(ngModel)]="searchTerm"
(focusout)="loadAll()"
(blur)="loadAll()"
(keydown)="onKeydown($event)"
/>
<button class="btn btn-primary ms-3" (click)="loadAll()">
<span jhiTranslate="artemisApp.userManagement.search"></span>
</button>
@if (searchControl.invalid && (searchControl.dirty || searchControl.touched)) {
@if (searchInvalid) {
<div class="alert alert-danger ms-3 mb-0">
<div jhiTranslate="artemisApp.userManagement.searchError"></div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import { User } from 'app/core/user/user.model';
import { AccountService } from 'app/core/auth/account.service';
import { AlertService } from 'app/core/util/alert.service';
import { SortingOrder } from 'app/shared/table/pageable-table';
import { debounceTime, switchMap, tap } from 'rxjs/operators';
import { AbstractControl, FormControl, FormGroup } from '@angular/forms';
import { switchMap, tap } from 'rxjs/operators';
import { FormControl, FormGroup } from '@angular/forms';
import { EventManager } from 'app/core/util/event-manager.service';
import { ASC, DESC, ITEMS_PER_PAGE, SORT } from 'app/shared/constants/pagination.constants';
import { faEye, faFilter, faPlus, faSort, faTimes, faWrench } from '@fortawesome/free-solid-svg-icons';
Expand All @@ -17,7 +17,6 @@ import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
import { ButtonSize, ButtonType } from 'app/shared/components/button.component';
import { ProfileService } from 'app/shared/layouts/profiles/profile.service';
import { AdminUserService } from 'app/core/user/admin-user.service';
import { UserService } from 'app/core/user/user.service';

export class UserFilter {
authorityFilter: Set<AuthorityFilter> = new Set();
Expand Down Expand Up @@ -103,6 +102,7 @@ export class UserManagementComponent implements OnInit, OnDestroy {
predicate!: string;
ascending!: boolean;
searchTermString = '';
searchInvalid = false;
isLdapProfileActive: boolean;

// filters
Expand All @@ -129,7 +129,6 @@ export class UserManagementComponent implements OnInit, OnDestroy {

constructor(
private adminUserService: AdminUserService,
private userService: UserService,
private alertService: AlertService,
private accountService: AccountService,
private activatedRoute: ActivatedRoute,
Expand All @@ -148,7 +147,6 @@ export class UserManagementComponent implements OnInit, OnDestroy {
this.search
.pipe(
tap(() => (this.loadingSearchResult = true)),
debounceTime(1000),
switchMap(() =>
this.adminUserService.query(
{
Expand All @@ -175,7 +173,7 @@ export class UserManagementComponent implements OnInit, OnDestroy {
});

this.userSearchForm = new FormGroup({
searchControl: new FormControl('', { validators: [this.validateUserSearch], updateOn: 'blur' }),
searchControl: new FormControl('', { updateOn: 'change' }),
});
this.accountService.identity().then((user) => {
this.currentAccount = user!;
Expand Down Expand Up @@ -443,17 +441,21 @@ export class UserManagementComponent implements OnInit, OnDestroy {
* Retrieve the list of users from the user service for a single page in the user management based on the page, size and sort configuration
*/
loadAll() {
this.searchTerm = this.searchControl.value;
if (this.searchTerm.length >= 3 || this.searchTerm.length === 0) {
this.searchInvalid = false;
this.search.next();
} else {
this.searchInvalid = true;
}
}

/**
* Returns the unique identifier for items in the collection
* @param index of a user in the collection
* @param _index of a user in the collection
* @param item current user
*/
trackIdentity(index: number, item: User) {
trackIdentity(_index: number, item: User) {
return item.id ?? -1;
}

Expand Down Expand Up @@ -520,14 +522,14 @@ export class UserManagementComponent implements OnInit, OnDestroy {
return this.searchTermString;
}

validateUserSearch(control: AbstractControl) {
if (control.value.length >= 1 && control.value.length <= 2) {
return { searchControl: true };
}
return null;
}

get searchControl() {
return this.userSearchForm.get('searchControl')!;
}

onKeydown(event: KeyboardEvent) {
if (event.key === 'Enter') {
event.preventDefault(); // Prevent the default form submission behavior
this.loadAll(); // Trigger the search logic
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const userManagementRoute: Route[] = [
path: 'edit',
component: UserManagementUpdateComponent,
data: {
pageTitle: 'artemisApp.userManagement.home.createOrEditLabel',
pageTitle: 'artemisApp.userManagement.home.editLabel',
},
},
],
Expand Down
4 changes: 2 additions & 2 deletions src/main/webapp/i18n/de/user-management.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
"home": {
"title": "Nutzer:in",
"createLabel": "Nutzer:in erstellen",
"createOrEditLabel": "Nutzer:in erstellen oder bearbeiten"
"editLabel": "Nutzer:in bearbeiten"
},
"created": "Nutzer:in wurde mit ID {{ param }} erstellt",
"updated": "Nutzer:in mit ID {{ param }} wurde geändert",
Expand Down Expand Up @@ -93,7 +93,7 @@
"profiles": "Profile",
"langKey": "Sprache",
"internal": "Intern",
"passwordTooltip": "Du kannst nur Passwörter für interne Nutzer:innen ändern.",
"passwordTooltip": "Du kannst diese Einstellung nur für neue Nutzer:innen ändern. Bitte beachte, dass du nur Passwörter für interne Nutzer:innen ändern kannst.",
"createdBy": "Erstellt von",
"createdDate": "Erstellt am",
"lastModifiedBy": "Bearbeitet von",
Expand Down
4 changes: 2 additions & 2 deletions src/main/webapp/i18n/en/user-management.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
"home": {
"title": "Users",
"createLabel": "Create a new user",
"createOrEditLabel": "Create or edit a user"
"editLabel": "Edit user"
},
"created": "Created new user with identifier {{ param }}",
"updated": "Updated User with identifier {{ param }}",
Expand Down Expand Up @@ -92,7 +92,7 @@
"deactivated": "Deactivated",
"profiles": "Profiles",
"langKey": "Language",
"passwordTooltip": "You can only change passwords for internal users.",
"passwordTooltip": "This setting can only be changed for new users. Notice, that you can only change passwords for internal users.",
"internal": "Internal",
"createdBy": "Created by",
"createdDate": "Created date",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { ActivatedRoute, Router } from '@angular/router';
import { HttpHeaders, HttpParams, HttpResponse, provideHttpClient } from '@angular/common/http';
import { User } from 'app/core/user/user.model';
import { Subscription, of } from 'rxjs';
import { AbstractControl, ReactiveFormsModule } from '@angular/forms';
import { ReactiveFormsModule } from '@angular/forms';
import { ItemCountComponent } from 'app/shared/pagination/item-count.component';
import { NgbModule } from '@ng-bootstrap/ng-bootstrap';
import { ArtemisDatePipe } from 'app/shared/pipes/artemis-date.pipe';
Expand Down Expand Up @@ -168,7 +168,7 @@ describe('UserManagementComponent', () => {

// THEN
expect(userService.update).toHaveBeenCalledWith({ ...user, activated: true });
expect(userService.query).toHaveBeenCalledOnce();
expect(userService.query).toHaveBeenCalledTimes(2);
expect(comp.users && comp.users[0]).toEqual(expect.objectContaining({ id: 123 }));
expect(profileSpy).toHaveBeenCalledOnce();
}),
Expand Down Expand Up @@ -243,21 +243,25 @@ describe('UserManagementComponent', () => {
jest.restoreAllMocks();
});

it('should validate user search correctly', () => {
expect(comp.validateUserSearch({ value: [] } as AbstractControl)).toBeNull();
expect(comp.validateUserSearch({ value: [0] } as AbstractControl)).toEqual({ searchControl: true });
expect(comp.validateUserSearch({ value: [0, 0] } as AbstractControl)).toEqual({ searchControl: true });
expect(comp.validateUserSearch({ value: [0, 0, 0] } as AbstractControl)).toBeNull();
});

it('should call initFilters', () => {
const headers = new HttpHeaders().append('link', 'link;link');
const user = new User(123);
jest.spyOn(userService, 'query').mockReturnValue(
of(
new HttpResponse({
body: [user],
headers,
}),
),
);
const spy = jest.spyOn(comp, 'initFilters');
const initSpy = jest.spyOn(profileService, 'getProfileInfo').mockReturnValue(of(new ProfileInfo()));

comp.ngOnInit();

expect(spy).toHaveBeenCalledOnce();
expect(initSpy).toHaveBeenCalledOnce();
expect(userService.query).toHaveBeenCalledTimes(0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(userService.query).toHaveBeenCalledTimes(0);
expect(userService.query).not.toHaveBeenCalled();

});

it.each`
Expand Down
Loading