Merge pull request DSpace#2414 from alexandrevryghem/fix-display-orde…

…r-authentication-methods_contribute-7.6

Made it possible to reorder the login methods

src/app/core/auth/auth.interceptor.ts

@@ -152,20 +152,20 @@ export class AuthInterceptor implements HttpInterceptor {
 
         let authMethodModel: AuthMethod;
         if (splittedRealm.length === 1) {
-          authMethodModel = new AuthMethod(methodName);
+          authMethodModel = new AuthMethod(methodName, Number(j));
           authMethodModels.push(authMethodModel);
         } else if (splittedRealm.length > 1) {
           let location = splittedRealm[1];
           location = this.parseLocation(location);
-          authMethodModel = new AuthMethod(methodName, location);
+          authMethodModel = new AuthMethod(methodName, Number(j), location);
           authMethodModels.push(authMethodModel);
         }
       }
 
       // make sure the email + password login component gets rendered first
       authMethodModels = this.sortAuthMethods(authMethodModels);
     } else {
-      authMethodModels.push(new AuthMethod(AuthMethodType.Password));
+      authMethodModels.push(new AuthMethod(AuthMethodType.Password, 0));
     }
 
     return authMethodModels;

src/app/core/auth/auth.reducer.spec.ts

@@ -598,9 +598,9 @@ describe('authReducer', () => {
       authMethods: [],
       idle: false
     };
-    const authMethods = [
-      new AuthMethod(AuthMethodType.Password),
-      new AuthMethod(AuthMethodType.Shibboleth, 'location')
+    const authMethods: AuthMethod[] = [
+      new AuthMethod(AuthMethodType.Password, 0),
+      new AuthMethod(AuthMethodType.Shibboleth, 1, 'location'),
     ];
     const action = new RetrieveAuthMethodsSuccessAction(authMethods);
     const newState = authReducer(initialState, action);
@@ -632,7 +632,7 @@ describe('authReducer', () => {
       loaded: false,
       blocking: false,
       loading: false,
-      authMethods: [new AuthMethod(AuthMethodType.Password)],
+      authMethods: [new AuthMethod(AuthMethodType.Password, 0)],
       idle: false
     };
     expect(newState).toEqual(state);

src/app/core/auth/auth.reducer.ts

@@ -236,7 +236,7 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut
       return Object.assign({}, state, {
         loading: false,
         blocking: false,
-        authMethods: [new AuthMethod(AuthMethodType.Password)]
+        authMethods: [new AuthMethod(AuthMethodType.Password, 0)]
       });
 
     case AuthActionTypes.SET_REDIRECT_URL:

src/app/core/auth/models/auth.method.ts

@@ -2,11 +2,12 @@ import { AuthMethodType } from './auth.method-type';
 
 export class AuthMethod {
   authMethodType: AuthMethodType;
+  position: number;
   location?: string;
 
-  // isStandalonePage? = true;
+  constructor(authMethodName: string, position: number, location?: string) {
+    this.position = position;
 
-  constructor(authMethodName: string, location?: string) {
     switch (authMethodName) {
       case 'ip': {
         this.authMethodType = AuthMethodType.Ip;

src/app/shared/log-in/container/log-in-container.component.spec.ts

@@ -13,13 +13,17 @@ import { AuthMethod } from '../../../core/auth/models/auth.method';
 import { AuthServiceStub } from '../../testing/auth-service.stub';
 import { createTestComponent } from '../../testing/utils.test';
 import { HardRedirectService } from '../../../core/services/hard-redirect.service';
+import { AuthMethodType } from '../../../core/auth/models/auth.method-type';
+import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service';
+import { AuthorizationDataServiceStub } from '../../testing/authorization-service.stub';
+import { RouterTestingModule } from '@angular/router/testing';
 
 describe('LogInContainerComponent', () => {
 
   let component: LogInContainerComponent;
   let fixture: ComponentFixture<LogInContainerComponent>;
 
-  const authMethod = new AuthMethod('password');
+  const authMethod = new AuthMethod(AuthMethodType.Password, 0);
 
   let hardRedirectService: HardRedirectService;
 
@@ -35,13 +39,15 @@ describe('LogInContainerComponent', () => {
         ReactiveFormsModule,
         StoreModule.forRoot(authReducer),
         SharedModule,
-        TranslateModule.forRoot()
+        TranslateModule.forRoot(),
+        RouterTestingModule,
       ],
       declarations: [
         TestComponent
       ],
       providers: [
         { provide: AuthService, useClass: AuthServiceStub },
+        { provide: AuthorizationDataService, useClass: AuthorizationDataServiceStub },
         { provide: HardRedirectService, useValue: hardRedirectService },
         LogInContainerComponent
       ],
@@ -113,6 +119,6 @@ describe('LogInContainerComponent', () => {
 class TestComponent {
 
   isStandalonePage = true;
-  authMethod = new AuthMethod('password');
+  authMethod = new AuthMethod(AuthMethodType.Password, 0);
 
 }

src/app/shared/log-in/log-in.component.html

@@ -1,13 +1,11 @@
 <ds-themed-loading *ngIf="(loading | async) || (isAuthenticated | async)" class="m-5"></ds-themed-loading>
 <div *ngIf="!(loading | async) && !(isAuthenticated | async)" class="px-4 py-3 mx-auto login-container">
-  <ng-container *ngFor="let authMethod of (authMethods); let i = index">
-    <div *ngIf="i === 1" class="text-center mt-2">
-      <span class="align-middle">{{"login.form.or-divider" | translate}}</span>
+  <ng-container *ngFor="let authMethod of getOrderedAuthMethods(authMethods | async); let last = last">
+    <div [class.d-none]="contentRef.innerText?.trim().length === 0">
+      <div #contentRef>
+        <ds-log-in-container [authMethod]="authMethod" [isStandalonePage]="isStandalonePage"></ds-log-in-container>
+      </div>
+      <div *ngIf="!last" class="dropdown-divider my-2"></div>
     </div>
-    <ds-log-in-container [authMethod]="authMethod" [isStandalonePage]="isStandalonePage"></ds-log-in-container>
   </ng-container>
-
-  <div class="dropdown-divider"></div>
-  <a class="dropdown-item" *ngIf="canRegister$ | async" [routerLink]="[getRegisterRoute()]" [attr.data-test]="'register' | dsBrowserOnly">{{"login.form.new-user" | translate}}</a>
-  <a class="dropdown-item" [routerLink]="[getForgotRoute()]" [attr.data-test]="'forgot' | dsBrowserOnly">{{"login.form.forgot-password" | translate}}</a>
 </div>

src/app/shared/log-in/log-in.component.spec.ts

@@ -50,7 +50,7 @@ describe('LogInComponent', () => {
     });
 
     // refine the test module by declaring the test component
-    TestBed.configureTestingModule({
+    void TestBed.configureTestingModule({
       imports: [
         FormsModule,
         ReactiveFormsModule,

src/app/shared/log-in/log-in.component.ts

@@ -1,5 +1,5 @@
-import { Component, Input, OnInit } from '@angular/core';
-import { Observable } from 'rxjs';
+import { ChangeDetectionStrategy, Component, Input, OnInit } from '@angular/core';
+import { map, Observable } from 'rxjs';
 import { select, Store } from '@ngrx/store';
 import { AuthMethod } from '../../core/auth/models/auth.method';
 import {
@@ -8,11 +8,8 @@ import {
   isAuthenticated,
   isAuthenticationLoading
 } from '../../core/auth/selectors';
-import { getForgotPasswordRoute, getRegisterRoute } from '../../app-routing-paths';
 import { hasValue } from '../empty.util';
 import { AuthService } from '../../core/auth/auth.service';
-import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
-import { FeatureID } from '../../core/data/feature-authorization/feature-id';
 import { CoreState } from '../../core/core-state.model';
 import { AuthMethodType } from '../../core/auth/models/auth.method-type';
 
@@ -23,7 +20,8 @@ import { AuthMethodType } from '../../core/auth/models/auth.method-type';
 @Component({
   selector: 'ds-log-in',
   templateUrl: './log-in.component.html',
-  styleUrls: ['./log-in.component.scss']
+  styleUrls: ['./log-in.component.scss'],
+  changeDetection: ChangeDetectionStrategy.OnPush,
 })
 export class LogInComponent implements OnInit {
 
@@ -37,7 +35,7 @@ export class LogInComponent implements OnInit {
    * The list of authentication methods available
    * @type {AuthMethod[]}
    */
-  public authMethods: AuthMethod[];
+  public authMethods: Observable<AuthMethod[]>;
 
   /**
    * Whether user is authenticated.
@@ -51,24 +49,17 @@ export class LogInComponent implements OnInit {
    */
   public loading: Observable<boolean>;
 
-  /**
-   * Whether or not the current user (or anonymous) is authorized to register an account
-   */
-  canRegister$: Observable<boolean>;
-
   constructor(private store: Store<CoreState>,
               private authService: AuthService,
-              private authorizationService: AuthorizationDataService) {
+  ) {
   }
 
   ngOnInit(): void {
-
-    this.store.pipe(
+    this.authMethods = this.store.pipe(
       select(getAuthenticationMethods),
-    ).subscribe(methods => {
       // ignore the ip authentication method when it's returned by the backend
-      this.authMethods = methods.filter(a => a.authMethodType !== AuthMethodType.Ip);
-    });
+      map((methods: AuthMethod[]) => methods.filter((authMethod: AuthMethod) => authMethod.authMethodType !== AuthMethodType.Ip)),
+    );
 
     // set loading
     this.loading = this.store.pipe(select(isAuthenticationLoading));
@@ -82,15 +73,18 @@ export class LogInComponent implements OnInit {
         this.authService.clearRedirectUrl();
       }
     });
-
-    this.canRegister$ = this.authorizationService.isAuthorized(FeatureID.EPersonRegistration);
-  }
-
-  getRegisterRoute() {
-    return getRegisterRoute();
   }
 
-  getForgotRoute() {
-    return getForgotPasswordRoute();
+  /**
+   * Returns an ordered list of {@link AuthMethod}s based on their position.
+   *
+   * @param authMethods The {@link AuthMethod}s to sort
+   */
+  getOrderedAuthMethods(authMethods: AuthMethod[] | null): AuthMethod[] {
+    if (hasValue(authMethods)) {
+      return [...authMethods].sort((method1: AuthMethod, method2: AuthMethod) => method1.position - method2.position);
+    } else {
+      return [];
+    }
   }
 }

src/app/shared/log-in/methods/log-in-external-provider/log-in-external-provider.component.html

@@ -1,3 +1,3 @@
-<button class="btn btn-lg btn-primary btn-block mt-2 text-white" (click)="redirectToExternalProvider()">
+<button class="btn btn-lg btn-primary btn-block text-white" (click)="redirectToExternalProvider()">
   <i class="fas fa-sign-in-alt"></i> {{getButtonLabel() | translate}}
 </button>

src/app/shared/log-in/methods/log-in-external-provider/log-in-external-provider.component.spec.ts

@@ -3,11 +3,8 @@ import { ActivatedRoute, Router } from '@angular/router';
 import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
 
 import { provideMockStore } from '@ngrx/store/testing';
-import { Store, StoreModule } from '@ngrx/store';
+import { StoreModule } from '@ngrx/store';
 import { TranslateModule } from '@ngx-translate/core';
-
-import { EPerson } from '../../../../core/eperson/models/eperson.model';
-import { EPersonMock } from '../../../testing/eperson.mock';
 import { authReducer } from '../../../../core/auth/auth.reducer';
 import { AuthService } from '../../../../core/auth/auth.service';
 import { AuthServiceStub } from '../../../testing/auth-service.stub';
@@ -25,17 +22,14 @@ describe('LogInExternalProviderComponent', () => {
 
   let component: LogInExternalProviderComponent;
   let fixture: ComponentFixture<LogInExternalProviderComponent>;
-  let page: Page;
-  let user: EPerson;
   let componentAsAny: any;
   let setHrefSpy;
-  let orcidBaseUrl;
-  let location;
+  let orcidBaseUrl: string;
+  let location: string;
   let initialState: any;
   let hardRedirectService: HardRedirectService;
 
   beforeEach(() => {
-    user = EPersonMock;
     orcidBaseUrl = 'dspace-rest.test/orcid?redirectUrl=';
     location = orcidBaseUrl + 'http://dspace-angular.test/home';
 
@@ -59,7 +53,7 @@ describe('LogInExternalProviderComponent', () => {
 
   beforeEach(waitForAsync(() => {
     // refine the test module by declaring the test component
-    TestBed.configureTestingModule({
+    void TestBed.configureTestingModule({
       imports: [
         StoreModule.forRoot({ auth: authReducer }, storeModuleConfig),
         TranslateModule.forRoot()
@@ -69,7 +63,7 @@ describe('LogInExternalProviderComponent', () => {
       ],
       providers: [
         { provide: AuthService, useClass: AuthServiceStub },
-        { provide: 'authMethodProvider', useValue: new AuthMethod(AuthMethodType.Orcid, location) },
+        { provide: 'authMethodProvider', useValue: new AuthMethod(AuthMethodType.Orcid, 0, location) },
         { provide: 'isStandalonePage', useValue: true },
         { provide: NativeWindowService, useFactory: NativeWindowMockFactory },
         { provide: Router, useValue: new RouterStub() },
@@ -94,7 +88,6 @@ describe('LogInExternalProviderComponent', () => {
     componentAsAny = component;
 
     // create page
-    page = new Page(component, fixture);
     setHrefSpy = spyOnProperty(componentAsAny._window.nativeWindow.location, 'href', 'set').and.callThrough();
 
   });
@@ -130,25 +123,3 @@ describe('LogInExternalProviderComponent', () => {
   });
 
 });
-
-/**
- * I represent the DOM elements and attach spies.
- *
- * @class Page
- */
-class Page {
-
-  public emailInput: HTMLInputElement;
-  public navigateSpy: jasmine.Spy;
-  public passwordInput: HTMLInputElement;
-
-  constructor(private component: LogInExternalProviderComponent, private fixture: ComponentFixture<LogInExternalProviderComponent>) {
-    // use injector to get services
-    const injector = fixture.debugElement.injector;
-    const store = injector.get(Store);
-
-    // add spies
-    this.navigateSpy = spyOn(store, 'dispatch');
-  }
-
-}

src/app/shared/log-in/methods/password/log-in-password.component.html

@@ -28,3 +28,12 @@
   <button class="btn btn-lg btn-primary btn-block mt-3" type="submit" [attr.data-test]="'login-button' | dsBrowserOnly"
           [disabled]="!form.valid"><i class="fas fa-sign-in-alt"></i> {{"login.form.submit" | translate}}</button>
 </form>
+
+<div class="mt-2">
+  <a class="dropdown-item" *ngIf="canRegister$ | async" [routerLink]="[getRegisterRoute()]" [attr.data-test]="'register' | dsBrowserOnly">
+    {{ 'login.form.new-user' | translate }}
+  </a>
+  <a class="dropdown-item" [routerLink]="[getForgotRoute()]" [attr.data-test]="'forgot' | dsBrowserOnly">
+    {{ 'login.form.forgot-password' | translate }}
+  </a>
+</div>

src/app/shared/log-in/methods/password/log-in-password.component.scss

@@ -11,3 +11,7 @@
   border-top-right-radius: 0;
 }
 
+.dropdown-item {
+  white-space: normal;
+  padding: .25rem .75rem;
+}