[#808] Institution Form Fixes

* Fix emails validator and add unit tests * Fix admin members roles adding / removal * Fix tests, remove admin email from profile
cyfronet-fid · Oct 28, 2020 · ccbad94 · ccbad94
1 parent 92e6e2e
commit ccbad94
Show file tree

Hide file tree

Showing 11 changed files with 50 additions and 49 deletions.
diff --git a/s4e-web/cypress/page-objects/settings/settings-add-institution-form.po.ts b/s4e-web/cypress/page-objects/settings/settings-add-institution-form.po.ts
@@ -55,10 +55,6 @@ export class AddInstitutionForm extends Core {
       .pageObject
       .getLogoImage()
       .should('be.visible');
-    AddInstitutionForm
-      .pageObject
-      .getEmailInput()
-      .type(institution.institutionAdminEmail);
 
     return AddInstitutionForm;
   }

diff --git a/s4e-web/src/app/views/settings/email-list-validator.spec.ts b/s4e-web/src/app/views/settings/email-list-validator.spec.ts
@@ -0,0 +1,22 @@
+import {emailListValidator} from './email-list-validator.utils';
+import {FormControl} from '@angular/forms';
+
+describe('emailListValidator', () => {
+  function validateValue(value: string) {
+    const control = new FormControl(value);
+    return emailListValidator(control)
+  }
+
+  it('should work', () => {
+    expect(validateValue('notValid')).toEqual({emails: true});
+    expect(validateValue('admin; admin2 ? admin@mail.pl')).toEqual({emails: true});
+    expect(validateValue('admin, admin@mail.pl')).toEqual({emails: true});
+    expect(validateValue('admin@mail.pl')).toEqual(null);
+    expect(validateValue('admin@mail.pl, admin2@mail.pl')).toEqual(null);
+    expect(validateValue('admin@mail.pl,admin2@mail.pl')).toEqual(null);
+    expect(validateValue('admin@mail.pl,  admin2@mail.pl')).toEqual(null);
+    expect(validateValue('   admin@mail.pl,  admin2@mail.pl   ')).toEqual(null);
+    expect(validateValue('')).toEqual(null);
+    expect(validateValue(null)).toEqual(null);
+  });
+});
diff --git a/s4e-web/src/app/views/settings/email-list-validator.utils.ts b/s4e-web/src/app/views/settings/email-list-validator.utils.ts
@@ -1,13 +1,14 @@
 import { AbstractControl } from '@angular/forms';
 
-const COMMA_SEPARATED_EMAILS_EXPRESSION = /((,? ?)((\.?)[a-zA-Z0-9_]+)+@((\.?)[a-zA-Z0-9_]+)+)+/i;
+const COMMA_SEPARATED_EMAILS_EXPRESSION = /^ *((,? *)((\.?)[a-zA-Z0-9_]+)+@((\.?)[a-zA-Z0-9_]+)+)+ *$/i;
 
 interface IValidatorOutput {
   [key: string]: boolean;
 }
 
 export function emailListValidator(control: AbstractControl): IValidatorOutput | null {
-  return _hasMatch(control.value, COMMA_SEPARATED_EMAILS_EXPRESSION) ? null : { emails: true };
+  return !control.value ? null :
+    (_hasMatch(control.value, COMMA_SEPARATED_EMAILS_EXPRESSION) ? null : { emails: true });
 }
 
 function _hasMatch(url: string, regex: RegExp) {

diff --git a/s4e-web/src/app/views/settings/intitution-profile/institution-profile.component.html b/s4e-web/src/app/views/settings/intitution-profile/institution-profile.component.html
@@ -2,28 +2,25 @@
   <h1 i18n>Profil instytucji</h1>
 </header>
 <section class="content__details content__details--75 profile">
-  <ng-container *ngIf="!!(activeInstitution$ | async)">
+  <ng-container *ngIf="(activeInstitution$ | async) as institution">
     <div
       class="profile__content"
       data-e2e="institution-details"
     >
-      <h2 id="institution-title">{{ (activeInstitution$ | async).name }}</h2>
+      <h2 id="institution-title">{{ institution.name }}</h2>
       <ul>
-        <li id="institution-address">{{ (activeInstitution$ | async).address }}</li>
+        <li id="institution-address">{{ institution.address }}</li>
         <li
           id="institution-postal-code"
           data-e2e="postal-code-with-city"
         >
-          {{ (activeInstitution$ | async).postalCode }} {{ (activeInstitution$ | async).city }}
+          {{ institution.postalCode }} {{ institution.city }}
         </li>
-        <li *ngIf="!!(activeInstitution$ | async).institutionAdminEmail" id="institution-email">
-          email: {{ (activeInstitution$ | async).institutionAdminEmail }}
+        <li *ngIf="!!institution.phone" id="institution-phone">
+          Telefon: {{ institution.phone }}
         </li>
-        <li *ngIf="!!(activeInstitution$ | async).phone" id="institution-phone">
-          Telefon: {{ (activeInstitution$ | async).phone }}
-        </li>
-        <li *ngIf="!!(activeInstitution$ | async).secondaryPhone" id="institution-second-phone">
-          Telefon: {{ (activeInstitution$ | async).secondaryPhone }}
+        <li *ngIf="!!institution.secondaryPhone" id="institution-second-phone">
+          Telefon: {{ institution.secondaryPhone }}
         </li>
       </ul>
     </div>

diff --git a/s4e-web/src/app/views/settings/intitution-profile/institution-profile.component.spec.ts b/s4e-web/src/app/views/settings/intitution-profile/institution-profile.component.spec.ts
@@ -78,9 +78,6 @@ describe('InstitutionProfileComponent', () => {
     const postalCode = de.query(By.css('#institution-postal-code'));
     expect(postalCode.nativeElement.innerHTML).toContain(institution.postalCode);
 
-    const institutionAdminEmail = de.query(By.css('#institution-email'));
-    expect(institutionAdminEmail.nativeElement.innerHTML).toContain('email: ' + institution.institutionAdminEmail);
-
     const phone = de.query(By.css('#institution-phone'));
     expect(phone.nativeElement.innerHTML.split(':')[1].trim()).toContain(institution.phone);
 
@@ -122,9 +119,6 @@ describe('InstitutionProfileComponent', () => {
     const postalCode = de.query(By.css('#institution-postal-code'));
     expect(postalCode.nativeElement.innerHTML).toContain(institution.postalCode);
 
-    const institutionAdminEmail = de.query(By.css('#institution-email'));
-    expect(institutionAdminEmail.nativeElement.innerHTML).toContain('email: ' + institution.institutionAdminEmail);
-
     const phone = de.query(By.css('#institution-phone'));
     expect(phone.nativeElement.innerHTML.split(':')[1].trim()).toContain(institution.phone);
 

diff --git a/s4e-web/src/app/views/settings/intitution-profile/institution-profile.component.ts b/s4e-web/src/app/views/settings/intitution-profile/institution-profile.component.ts
@@ -17,7 +17,7 @@ import { Observable, combineLatest } from 'rxjs';
 })
 export class InstitutionProfileComponent implements OnDestroy {
   public isLoading$: Observable<boolean> = this._institutionQuery.selectLoading();
-  public activeInstitution$ = this._institutionsSearchResultsQuery
+  public activeInstitution$: Observable<Institution> = this._institutionsSearchResultsQuery
     .selectActive$(this._activatedRoute)
     .pipe(untilDestroyed(this));
   public childrenInstitutions$: Observable<Institution[]> = combineLatest(

diff --git a/...pp/views/settings/manage-institutions/institution-form/institution-form.component.spec.ts b/...pp/views/settings/manage-institutions/institution-form/institution-form.component.spec.ts
@@ -134,9 +134,9 @@ describe('InstitutionFormComponent', () => {
   });
 
   it('should validate institution admin email', () => {
-    component.form.controls.institutionAdminEmail.setValue('');
-    expect(component.form.controls.institutionAdminEmail.valid).toBeTruthy();
-    component.form.controls.institutionAdminEmail.setValue(InstitutionFactory.build().institutionAdminEmail);
-    expect(component.form.controls.institutionAdminEmail.valid).toBeTruthy();
+    component.form.controls.adminsEmails.setValue('');
+    expect(component.form.controls.adminsEmails.valid).toBeTruthy();
+    component.form.controls.adminsEmails.setValue(InstitutionFactory.build().adminsEmails);
+    expect(component.form.controls.adminsEmails.valid).toBeTruthy();
   });
 });
diff --git a/...src/app/views/settings/manage-institutions/institution-form/institution-form.component.ts b/...src/app/views/settings/manage-institutions/institution-form/institution-form.component.ts
@@ -71,6 +71,8 @@ export class InstitutionFormComponent extends GenericFormComponent<InstitutionQu
 
   loadLogo($event) {
     const emblem = File.getFirst($event);
+    console.log('emblem', emblem);
+
     if (!emblem) {
       return;
     }

diff --git a/s4e-web/src/app/views/settings/people/invitation-form/invitation-form.component.spec.ts b/s4e-web/src/app/views/settings/people/invitation-form/invitation-form.component.spec.ts
@@ -76,7 +76,7 @@ describe('InvitationFormComponent', () => {
 
     spyOn(invitationService, 'send').and.callThrough();
     component.send();
-    expect(invitationService.send).toHaveBeenCalledWith(slug, email);
+    expect(invitationService.send).toHaveBeenCalledWith(slug, email, false);
   });
   it('should call invitation resend', () => {
     const name = 'test';
@@ -92,6 +92,6 @@ describe('InvitationFormComponent', () => {
 
     spyOn(invitationService, 'resend').and.callThrough();
     component.send();
-    expect(invitationService.resend).toHaveBeenCalledWith({newEmail: email, oldEmail: email}, component.institution);
+    expect(invitationService.resend).toHaveBeenCalledWith({forAdmin: false, newEmail: email, oldEmail: email}, component.institution);
   });
 });
diff --git a/s4e-web/src/app/views/settings/people/state/person/person.service.ts b/s4e-web/src/app/views/settings/people/state/person/person.service.ts
@@ -1,7 +1,7 @@
 import { NotificationService } from 'notifications';
 import {HttpClient} from '@angular/common/http';
 import {PersonStore} from './person.store';
-import {DEFAULT_GROUP_SLUG, Person } from './person.model';
+import {DEFAULT_GROUP_SLUG, Person, UserRole} from './person.model';
 import { InstitutionQuery } from '../../../state/institution/institution.query';
 import environment from 'src/environments/environment';
 import { handleHttpRequest$ } from 'src/app/common/store.util';
@@ -32,11 +32,11 @@ export class PersonService {
     this._http.post(url, {})
       .pipe(handleHttpRequest$(this._store))
       .subscribe(() => {
-          person.roles.push({
+          const newRole: UserRole = {
             institutionSlug,
             role: 'INST_ADMIN'
-          });
-          this._store.replace(person.email, person);
+          };
+          this._store.update(person.email, {roles: [...person.roles, newRole]});
           this._notificationService.addGeneral({
           content: `${person.email} otrzymał rolę administratora`,
           type: 'success'
@@ -50,8 +50,7 @@ export class PersonService {
     this._http.delete(url)
       .pipe(handleHttpRequest$(this._store))
       .subscribe(() => {
-        this._store.remove(person.email);
-        this._store.add(this._getPersonWithout(person, 'INST_ADMIN'));
+        this._store.update(person.email, {roles: person.roles.filter(role => role.role !== 'INST_ADMIN' && role.institutionSlug !== institutionSlug)});
         this._notificationService.addGeneral({
           content: `Rola administratora dla ${person.email} została usunięta`,
           type: 'success'
@@ -66,14 +65,4 @@ export class PersonService {
       .pipe(handleHttpRequest$(this._store))
       .subscribe(() => this._store.remove(person.email));
   }
-
-  private _getPersonWithout(person: Person, role: 'INST_MEMBER' | 'INST_ADMIN'): Person {
-    const roleIndex = person.roles.map(personRole => personRole.role).indexOf(role);
-    const clonedPerson = JSON.parse(JSON.stringify(person));
-    delete(clonedPerson.roles[roleIndex]);
-
-    console.log(clonedPerson);
-
-    return person;
-  }
 }
diff --git a/s4e-web/src/app/views/settings/state/institution/institution.factory.spec.ts b/s4e-web/src/app/views/settings/state/institution/institution.factory.spec.ts
@@ -1,7 +1,7 @@
-import { Institution } from './institution.model';
+import {Institution, InstitutionForm} from './institution.model';
 import * as Factory from 'factory.ts';
 
-export const InstitutionFactory = Factory.makeFactory<Institution>({
+export const InstitutionFactory = Factory.makeFactory<InstitutionForm>({
   id: Factory.each(i => `institution:${i}`),
 
   parentName : null,
@@ -15,5 +15,5 @@ export const InstitutionFactory = Factory.makeFactory<Institution>({
   phone: '+48 000 000 000',
   secondaryPhone: '+48 000 000 000',
   emblem: 'logo_um.png',
-  institutionAdminEmail: 'zkMember@mail.pl'
+  adminsEmails: 'zkMember@mail.pl'
 });