Merge pull request #1080 from ORCID/refactorNgServices

Refactor ng services

ui/src/app/account/model/password-reset-init-result.model.ts

@@ -0,0 +1,17 @@
+export interface IPasswordResetInitResult {
+  success: boolean
+  emailNotFound: boolean
+  generalError: boolean
+}
+
+export class PasswordResetInitResult implements IPasswordResetInitResult {
+  success: boolean
+  emailNotFound: boolean
+  generalError: boolean
+
+  constructor(success: boolean, emailNotFound: boolean, generalError: boolean) {
+    this.emailNotFound = emailNotFound
+    this.generalError = generalError
+    this.success = success
+  }
+}

ui/src/app/account/password/password-reset-init.component.spec.ts

@@ -6,6 +6,7 @@ import { PasswordResetInitComponent } from './password-reset-init.component'
 import { EMAIL_NOT_FOUND_TYPE } from 'src/app/app.constants'
 import { HttpClientTestingModule } from '@angular/common/http/testing'
 import { By } from '@angular/platform-browser'
+import { PasswordResetInitResult } from '../model/password-reset-init-result.model'
 
 describe('Component Tests', () => {
   describe('PasswordResetInitComponent', () => {
@@ -28,7 +29,7 @@ describe('Component Tests', () => {
     it('notifies of success upon successful requestReset', inject(
       [PasswordResetInitService],
       (service: PasswordResetInitService) => {
-        spyOn(service, 'initPasswordReset').and.returnValue(of({}))
+        spyOn(service, 'initPasswordReset').and.returnValue(of(new PasswordResetInitResult(true, false, false)))
         comp.resetRequestForm.patchValue({
           email: '[email protected]',
         })
@@ -51,12 +52,7 @@ describe('Component Tests', () => {
     it('notifies of unknown email upon email address not registered/400', inject(
       [PasswordResetInitService],
       (service: PasswordResetInitService) => {
-        spyOn(service, 'initPasswordReset').and.returnValue(
-          throwError({
-            status: 400,
-            error: { type: EMAIL_NOT_FOUND_TYPE },
-          })
-        )
+        spyOn(service, 'initPasswordReset').and.returnValue(of(new PasswordResetInitResult(false, true, false)))
         comp.resetRequestForm.patchValue({
           email: '[email protected]',
         })
@@ -72,12 +68,7 @@ describe('Component Tests', () => {
     it('notifies of error upon error response', inject(
       [PasswordResetInitService],
       (service: PasswordResetInitService) => {
-        spyOn(service, 'initPasswordReset').and.returnValue(
-          throwError({
-            status: 503,
-            data: 'something else',
-          })
-        )
+        spyOn(service, 'initPasswordReset').and.returnValue(of(new PasswordResetInitResult(false, false, true)))
         comp.resetRequestForm.patchValue({
           email: '[email protected]',
         })

ui/src/app/account/password/password-reset-init.component.ts

@@ -3,6 +3,7 @@ import { FormBuilder, FormGroup, Validators } from '@angular/forms'
 
 import { PasswordResetInitService } from '../service/password-reset-init.service'
 import { EMAIL_NOT_FOUND_TYPE } from 'src/app/app.constants'
+import { PasswordResetInitResult } from '../model/password-reset-init-result.model'
 
 @Component({
   selector: 'app-password-reset-init',
@@ -31,19 +32,20 @@ export class PasswordResetInitComponent implements AfterViewInit {
     this.errorEmailNotExists = undefined
 
     if (this.resetRequestForm.get(['email'])) {
-      this.passwordResetInitService.initPasswordReset(this.resetRequestForm.get(['email'])!.value).subscribe({
-        next: () => {
-          this.success = 'OK'
-        },
-        error: (response) => {
-          this.success = undefined
-          if (response.status === 400 && response.error.type === EMAIL_NOT_FOUND_TYPE) {
-            this.errorEmailNotExists = 'ERROR'
+      this.passwordResetInitService
+        .initPasswordReset(this.resetRequestForm.get(['email'])!.value)
+        .subscribe((result: PasswordResetInitResult | null) => {
+          if (result && result.success) {
+            this.success = 'OK'
           } else {
-            this.error = 'ERROR'
+            this.success = undefined
+            if (result && result.emailNotFound) {
+              this.errorEmailNotExists = 'ERROR'
+            } else {
+              this.error = 'ERROR'
+            }
           }
-        },
-      })
+        })
     }
   }
 }

ui/src/app/account/service/account.service.ts

@@ -76,17 +76,40 @@ export class AccountService {
     return this.http.get<any>('/services/userservice/api/account/mfa')
   }
 
-  save(account: IAccount): Observable<HttpResponse<any>> {
+  save(account: IAccount): Observable<boolean> {
     const headers = { 'Accept-Language': account.langKey }
-    return this.http.post('/services/userservice/api/account', account, { observe: 'response', headers })
+    return this.http.post('/services/userservice/api/account', account, { observe: 'response', headers }).pipe(
+      map((res: HttpResponse<any>) => this.isSuccess(res)),
+      catchError((err) => {
+        return of(false)
+      })
+    )
   }
 
-  enableMfa(mfaSetup: any): Observable<HttpResponse<any>> {
-    return this.http.post('/services/userservice/api/account/mfa/on', mfaSetup, { observe: 'response' })
+  isSuccess(res: HttpResponse<any>): boolean {
+    if (res.status == 200) {
+      return true
+    }
+    return false
   }
 
-  disableMfa(): Observable<HttpResponse<any>> {
-    return this.http.post('/services/userservice/api/account/mfa/off', null, { observe: 'response' })
+  enableMfa(mfaSetup: any): Observable<string[] | null> {
+    return this.http.post('/services/userservice/api/account/mfa/on', mfaSetup, { observe: 'response' }).pipe(
+      map((res: HttpResponse<any>) => res.body),
+      catchError((err) => {
+        console.error('error enabling mfa')
+        return of(null)
+      })
+    )
+  }
+
+  disableMfa(): Observable<boolean> {
+    return this.http.post('/services/userservice/api/account/mfa/off', null, { observe: 'response' }).pipe(
+      map((res: HttpResponse<any>) => this.isSuccess(res)),
+      catchError((err) => {
+        return of(false)
+      })
+    )
   }
   // TODO: any - this seems to only be used for logging out (only ever receives null as arg)
   authenticate(identity: any) {

ui/src/app/account/service/password-reset-init.service.ts

@@ -1,12 +1,31 @@
 import { Injectable } from '@angular/core'
-import { HttpClient } from '@angular/common/http'
-import { Observable } from 'rxjs'
+import { HttpClient, HttpResponse } from '@angular/common/http'
+import { Observable, map, catchError, of } from 'rxjs'
+import { PasswordResetInitResult } from '../model/password-reset-init-result.model'
+import { EMAIL_NOT_FOUND_TYPE } from 'src/app/app.constants'
 
 @Injectable({ providedIn: 'root' })
 export class PasswordResetInitService {
   constructor(private http: HttpClient) {}
 
-  initPasswordReset(mail: string): Observable<any> {
-    return this.http.post('/services/userservice/api/account/reset-password/init', mail)
+  initPasswordReset(mail: string): Observable<PasswordResetInitResult | null> {
+    return this.http.post('/services/userservice/api/account/reset-password/init', mail, { observe: 'response' }).pipe(
+      map((res: HttpResponse<any>) => this.getResult(res)),
+      catchError((err) => {
+        return of(null)
+      })
+    )
+  }
+
+  getResult(res: HttpResponse<any>): PasswordResetInitResult {
+    if (res.status == 200) {
+      return new PasswordResetInitResult(true, false, false)
+    }
+
+    if (res.status === 400 && res.body.error.type === EMAIL_NOT_FOUND_TYPE) {
+      return new PasswordResetInitResult(false, false, true)
+    }
+
+    return new PasswordResetInitResult(false, true, false)
   }
 }

ui/src/app/account/settings/settings.component.spec.ts

@@ -154,7 +154,7 @@ describe('SettingsComponent', () => {
       })
     )
     accountServiceSpy.getMfaSetup.and.returnValue(of({ secret: 'test', otp: 'test', qrCode: ['test'] }))
-    accountServiceSpy.enableMfa.and.returnValue(of(new HttpResponse()))
+    accountServiceSpy.enableMfa.and.returnValue(of(['code1', 'code2']))
     fixture.detectChanges()
 
     component.mfaForm.patchValue({ mfaEnabled: true, verificationCode: 'test' })
@@ -182,7 +182,7 @@ describe('SettingsComponent', () => {
       })
     )
     accountServiceSpy.getMfaSetup.and.returnValue(of({ secret: 'test', otp: 'test', qrCode: ['test'] }))
-    accountServiceSpy.disableMfa.and.returnValue(of(new HttpResponse()))
+    accountServiceSpy.disableMfa.and.returnValue(of(true))
 
     component.mfaForm.patchValue({ mfaEnabled: false, verificationCode: 'test' })
     component.saveMfa()
@@ -192,7 +192,7 @@ describe('SettingsComponent', () => {
   })
 
   it('save form should call accountService.save and then account data requested when save is successful', () => {
-    accountServiceSpy.save.and.returnValue(of(new HttpResponse()))
+    accountServiceSpy.save.and.returnValue(of(true))
     accountServiceSpy.getAccountData.and.returnValue(
       of({
         activated: true,
@@ -218,7 +218,7 @@ describe('SettingsComponent', () => {
   })
 
   it('save form should call accountService.save and then account data requested when save is successful', () => {
-    accountServiceSpy.save.and.returnValue(throwError(() => new Error('error')))
+    accountServiceSpy.save.and.returnValue(of(false))
     accountServiceSpy.getAccountData.and.returnValue(
       of({
         activated: true,

ui/src/app/account/settings/settings.component.ts

@@ -83,8 +83,8 @@ export class SettingsComponent implements OnInit {
 
   save() {
     const settingsAccount = this.accountFromForm()
-    this.accountService.save(settingsAccount).subscribe({
-      next: () => {
+    this.accountService.save(settingsAccount).subscribe((success: boolean) => {
+      if (success) {
         this.error = undefined
         this.success = 'OK'
         this.accountService.getAccountData(true).subscribe((account) => {
@@ -96,11 +96,10 @@ export class SettingsComponent implements OnInit {
             this.updateMfaForm(account)
           }
         })
-      },
-      error: () => {
+      } else {
         this.success = undefined
         this.error = 'ERROR'
-      },
+      }
     })
   }
 
@@ -110,15 +109,14 @@ export class SettingsComponent implements OnInit {
       const otp = this.mfaForm.get('verificationCode')!.value
       console.log('about to set otp on ' + this.mfaSetup)
       this.mfaSetup.otp = otp
-      this.accountService.enableMfa(this.mfaSetup).subscribe({
-        next: (res) => {
-          this.mfaBackupCodes = res.body
+      this.accountService.enableMfa(this.mfaSetup).subscribe((codes: string[] | null) => {
+        if (codes) {
+          this.mfaBackupCodes = codes
           this.showMfaBackupCodes = true
           this.showMfaUpdated = true
-        },
-        error: (err) => {
+        } else {
           this.mfaSetupFailure = true
-        },
+        }
       })
     } else {
       this.accountService.disableMfa().subscribe({

ui/src/app/layout/navbar/navbar.component.ts

@@ -69,10 +69,6 @@ export class NavbarComponent implements OnInit {
               }
               return this.organizationName
             },
-            error: (res: HttpErrorResponse) => {
-              console.error('Error when getting org name: ' + res.error)
-              return null
-            },
           })
         }
       }

ui/src/app/shared/service/language.service.ts

@@ -3,7 +3,7 @@ import { Observable, of } from 'rxjs'
 
 @Injectable({ providedIn: 'root' })
 export class LanguageService {
-  private languages: any = {
+  private languages: { [langCode: string]: { name: string } } = {
     en: { name: 'English' },
     es: { name: 'Español' },
     fr: { name: 'Français' },
@@ -18,7 +18,7 @@ export class LanguageService {
     xx: { name: 'Test' },
   }
 
-  getAllLanguages(): any {
+  getAllLanguages(): { [langCode: string]: { name: string } } {
     return this.languages
   }