UoE/Remove debug console output and avoidable failed requests

UoE/Remove debug console output and avoidable failed requests

Author: milanmajchrak

angular.json

@@ -40,7 +40,12 @@
             "aot": true,
             "assets": [
               "src/assets",
-              "src/robots.txt"
+              "src/robots.txt",
+              {
+                "glob": "favicon.ico",
+                "input": "src/themes/datashare/assets/images",
+                "output": "/"
+              }
             ],
             "styles": [
               "src/styles/startup.scss",

config/config.example.yml

@@ -465,6 +465,10 @@ info:
   enableEndUserAgreement: true
   enablePrivacyStatement: true
   enableCOARNotifySupport: true
+  # Only enable Google Analytics on installations that configure a `google.analytics.key`
+  # property on the backend. When false (default) the UI does not probe for that property,
+  # avoiding a 404 request on every page.
+  enableGoogleAnalytics: false
 
 # Whether to enable Markdown (https://commonmark.org/) and MathJax (https://www.mathjax.org/)
 # display in supported metadata fields. By default, only dc.description.abstract is supported.

src/app/datashare/datashare-submission-form-section-container.service.ts

@@ -19,7 +19,6 @@ export class DatashareSubmissionFormSectionContainerService {
    * @param id The ID of the panel to open, or null to close all panels
    */
   setOpenPanelId(id: string | null): void {
-    console.log('Setting open panel ID to:', id);
     this.openPanelId.set(id);
   }
 }

src/app/datashare/datashare-submission.service.ts

@@ -23,14 +23,12 @@ export class DatashareSubmissionService {
   constructor(private notificationsService: NotificationsService,
     private translate: TranslateService,
   ) {
-    console.log('DatashareSubmissionService created');
   }
 
   /**
    * Update the deposit button visibility state
    */
   updatehasUploadFilesErrors(show: boolean): void {
-    console.log('Updating hasUploadFilesErrors to:', show);
     this._hasUploadFilesErrorsSignal.set(show);
   }
 

src/app/item-page/simple/field-components/file-section/file-section.component.ts

@@ -154,10 +154,6 @@ export class FileSectionComponent implements OnInit {
         this.notificationsService.error(this.translateService.get('file-section.error.header'), `${bitstreamsRD.statusCode} ${bitstreamsRD.errorMessage}`);
       } else if (hasValue(bitstreamsRD.payload)) {
         const current: Bitstream[] = this.bitstreams$.getValue();
-        // For debugging.
-        bitstreamsRD.payload.page.forEach(bitstream => {
-          console.log('Bitstream:', bitstream);
-        });
         this.bitstreams$.next([...current, ...bitstreamsRD.payload.page]);
         this.isLoading = false;
         this.isLastPage = this.currentPage === bitstreamsRD.payload.totalPages;
@@ -169,10 +165,6 @@ export class FileSectionComponent implements OnInit {
             this.notificationsService.error(this.translateService.get('file-section.error.header'), `${licenseRD.statusCode} ${licenseRD.errorMessage}`);
           } else if (hasValue(licenseRD.payload)) {
             const updated: Bitstream[] = this.bitstreams$.getValue();
-            // For debugging.
-            licenseRD.payload.page.forEach(bitstream => {
-              console.log('Bitstream:', bitstream);
-            });
             this.bitstreams$.next([...updated, ...licenseRD.payload.page]);
           }
           this.isLoading = false;

src/app/item-page/simple/qa-event-notification/qa-event-notification.component.spec.ts

@@ -13,6 +13,7 @@ import { SplitPipe } from 'src/app/shared/utils/split.pipe';
 import { APP_DATA_SERVICES_MAP } from '../../../../config/app-config.interface';
 import { RemoteDataBuildService } from '../../../core/cache/builders/remote-data-build.service';
 import { ObjectCacheService } from '../../../core/cache/object-cache.service';
+import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service';
 import { RequestService } from '../../../core/data/request.service';
 import { QualityAssuranceSourceObject } from '../../../core/notifications/qa/models/quality-assurance-source.model';
 import { QualityAssuranceSourceDataService } from '../../../core/notifications/qa/source/quality-assurance-source-data.service';
@@ -28,6 +29,7 @@ describe('QaEventNotificationComponent', () => {
   let component: QaEventNotificationComponent;
   let fixture: ComponentFixture<QaEventNotificationComponent>;
   let qualityAssuranceSourceDataServiceStub: any;
+  let authorizationServiceStub: any;
 
   const obj = Object.assign(new QualityAssuranceSourceObject(), {
     id: 'sourceName:target',
@@ -41,14 +43,18 @@ describe('QaEventNotificationComponent', () => {
   beforeEach(async () => {
 
     qualityAssuranceSourceDataServiceStub = {
-      getSourcesByTarget: () => objPL,
+      getSourcesByTarget: jasmine.createSpy('getSourcesByTarget').and.returnValue(objPL),
+    };
+    authorizationServiceStub = {
+      isAuthorized: jasmine.createSpy('isAuthorized').and.returnValue(of(true)),
     };
     await TestBed.configureTestingModule({
       imports: [CommonModule, TranslateModule.forRoot(), QaEventNotificationComponent, SplitPipe],
       providers: [
         { provide: APP_DATA_SERVICES_MAP, useValue: {} },
         { provide: ActivatedRoute, useValue: new ActivatedRouteStub() },
         { provide: QualityAssuranceSourceDataService, useValue: qualityAssuranceSourceDataServiceStub },
+        { provide: AuthorizationDataService, useValue: authorizationServiceStub },
         { provide: RequestService, useValue: {} },
         { provide: NotificationsService, useValue: {} },
         { provide: HALEndpointService, useValue: new HALEndpointServiceStub('test') },
@@ -57,6 +63,12 @@ describe('QaEventNotificationComponent', () => {
         provideMockStore({}),
       ],
     })
+      // The component declares QualityAssuranceSourceDataService in its own `providers`,
+      // which would otherwise shadow the stub above with a real instance. Override it so
+      // the component uses the stub we can assert against.
+      .overrideComponent(QaEventNotificationComponent, {
+        set: { providers: [{ provide: QualityAssuranceSourceDataService, useValue: qualityAssuranceSourceDataServiceStub }] },
+      })
       .compileComponents();
     fixture = TestBed.createComponent(QaEventNotificationComponent);
     component = fixture.componentInstance;
@@ -78,4 +90,22 @@ describe('QaEventNotificationComponent', () => {
     const route = component.getQualityAssuranceRoute();
     expect(route).toBe('/notifications/quality-assurance');
   });
+
+  it('should request QA sources when the user is authorized to see QA', () => {
+    authorizationServiceStub.isAuthorized.and.returnValue(of(true));
+    qualityAssuranceSourceDataServiceStub.getSourcesByTarget.calls.reset();
+    let result: QualityAssuranceSourceObject[];
+    component.getQualityAssuranceSources$().subscribe((sources) => result = sources);
+    expect(qualityAssuranceSourceDataServiceStub.getSourcesByTarget).toHaveBeenCalled();
+    expect(result).toEqual([obj]);
+  });
+
+  it('should NOT request QA sources when the user is not authorized', () => {
+    authorizationServiceStub.isAuthorized.and.returnValue(of(false));
+    qualityAssuranceSourceDataServiceStub.getSourcesByTarget.calls.reset();
+    let result: QualityAssuranceSourceObject[];
+    component.getQualityAssuranceSources$().subscribe((sources) => result = sources);
+    expect(qualityAssuranceSourceDataServiceStub.getSourcesByTarget).not.toHaveBeenCalled();
+    expect(result).toEqual([]);
+  });
 });

src/app/item-page/simple/qa-event-notification/qa-event-notification.component.ts

@@ -12,14 +12,20 @@ import {
 } from '@angular/core';
 import { RouterLink } from '@angular/router';
 import { TranslateModule } from '@ngx-translate/core';
-import { Observable } from 'rxjs';
+import {
+  Observable,
+  of as observableOf,
+} from 'rxjs';
 import {
   catchError,
   map,
+  switchMap,
 } from 'rxjs/operators';
 
 import { getNotificatioQualityAssuranceRoute } from '../../../admin/admin-routing-paths';
 import { RequestParam } from '../../../core/cache/models/request-param.model';
+import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service';
+import { FeatureID } from '../../../core/data/feature-authorization/feature-id';
 import { FindListOptions } from '../../../core/data/find-list-options.model';
 import { PaginatedList } from '../../../core/data/paginated-list.model';
 import { RemoteData } from '../../../core/data/remote-data';
@@ -61,6 +67,7 @@ export class QaEventNotificationComponent implements OnChanges {
 
   constructor(
     private qualityAssuranceSourceDataService: QualityAssuranceSourceDataService,
+    private authorizationService: AuthorizationDataService,
   ) {}
 
   /**
@@ -77,20 +84,31 @@ export class QaEventNotificationComponent implements OnChanges {
    * Note: sourceId is composed as: id: "sourceName:<target>"
    */
   getQualityAssuranceSources$(): Observable<QualityAssuranceSourceObject[]> {
-    const findListTopicOptions: FindListOptions = {
-      searchParams: [new RequestParam('target', this.item.uuid)],
-    };
-    return this.qualityAssuranceSourceDataService.getSourcesByTarget(findListTopicOptions, false)
-      .pipe(
-        getFirstCompletedRemoteData(),
-        map((data: RemoteData<PaginatedList<QualityAssuranceSourceObject>>) => {
-          if (data.hasSucceeded) {
-            return data.payload.page;
-          }
-          return [];
-        }),
-        catchError(() => []),
-      );
+    // Quality Assurance sources are only available to authorized users. Checking the
+    // authorization first avoids issuing the `qualityassurancesources/search/byTarget`
+    // request for anonymous/unauthorized users, which would otherwise return a 401 that
+    // shows up as a failed request in the browser. Authorized users keep the same behavior.
+    return this.authorizationService.isAuthorized(FeatureID.CanSeeQA).pipe(
+      switchMap((canSeeQA: boolean) => {
+        if (!canSeeQA) {
+          return observableOf([] as QualityAssuranceSourceObject[]);
+        }
+        const findListTopicOptions: FindListOptions = {
+          searchParams: [new RequestParam('target', this.item.uuid)],
+        };
+        return this.qualityAssuranceSourceDataService.getSourcesByTarget(findListTopicOptions, false)
+          .pipe(
+            getFirstCompletedRemoteData(),
+            map((data: RemoteData<PaginatedList<QualityAssuranceSourceObject>>) => {
+              if (data.hasSucceeded) {
+                return data.payload.page;
+              }
+              return [];
+            }),
+          );
+      }),
+      catchError(() => observableOf([] as QualityAssuranceSourceObject[])),
+    );
   }
 
   /**

src/app/shared/access-control-form-container/access-control-form-container.component.ts

@@ -98,11 +98,6 @@ export class AccessControlFormContainerComponent<T extends DSpaceObject> impleme
    * Will be used from a parent component to read the value of the form
    */
   getFormValue() {
-    console.log({
-      bitstream: this.bitstreamAccessCmp.getValue(),
-      item: this.itemAccessCmp.getValue(),
-      state: this.state,
-    });
     return {
       bitstream: this.bitstreamAccessCmp.getValue(),
       item: this.itemAccessCmp.getValue(),
@@ -137,9 +132,7 @@ export class AccessControlFormContainerComponent<T extends DSpaceObject> impleme
     this.bulkAccessControlService.executeScript(
       [ this.itemRD.payload.uuid ],
       file,
-    ).pipe(take(1)).subscribe((res) => {
-      console.log('success', res);
-    });
+    ).pipe(take(1)).subscribe();
   }
 
   /**

src/app/shared/access-control-form-container/bulk-access-control.service.ts

@@ -59,8 +59,6 @@ export class BulkAccessControlService {
    * @param file
    */
   executeScript(uuids: string[], file: File): Observable<boolean> {
-    console.log('execute', { uuids, file });
-
     const params: ProcessParameter[] = [
       { name: '-f', value: file.name },
     ];

src/app/shared/cookies/browser-klaro.service.spec.ts

@@ -6,6 +6,7 @@ import cloneDeep from 'lodash/cloneDeep';
 import { of as observableOf } from 'rxjs';
 import { TestScheduler } from 'rxjs/testing';
 
+import { environment } from '../../../environments/environment';
 import { AuthService } from '../../core/auth/auth.service';
 import { RestResponse } from '../../core/cache/response.models';
 import { ConfigurationDataService } from '../../core/data/configuration-data.service';
@@ -310,6 +311,9 @@ describe('BrowserKlaroService', () => {
     let GOOGLE_ANALYTICS_KEY;
     let REGISTRATION_VERIFICATION_ENABLED_KEY;
     beforeEach(() => {
+      // The Google Analytics key is only probed when GA is enabled for the installation;
+      // these tests exercise that probe-and-filter path.
+      environment.info.enableGoogleAnalytics = true;
       GOOGLE_ANALYTICS_KEY = clone((service as any).GOOGLE_ANALYTICS_KEY);
       REGISTRATION_VERIFICATION_ENABLED_KEY = clone((service as any).REGISTRATION_VERIFICATION_ENABLED_KEY);
       spyOn((service as any), 'getUser$').and.returnValue(observableOf(user));
@@ -320,6 +324,10 @@ describe('BrowserKlaroService', () => {
       configurationDataService.findByPropertyName = findByPropertyName;
     });
 
+    afterEach(() => {
+      environment.info.enableGoogleAnalytics = false;
+    });
+
     it('should not filter googleAnalytics when servicesToHide are empty', () => {
       const filteredConfig = (service as any).filterConfigServices([]);
       expect(filteredConfig).toContain(jasmine.objectContaining({ name: googleAnalytics }));
@@ -401,5 +409,19 @@ describe('BrowserKlaroService', () => {
       service.initialize();
       expect(service.klaroConfig.services).not.toContain(jasmine.objectContaining({ name: googleAnalytics }));
     });
+    it('should hide googleAnalytics without requesting the key when GA is disabled', () => {
+      environment.info.enableGoogleAnalytics = false;
+      const findByPropertyNameSpy = jasmine.createSpy('findByPropertyName').and.returnValue(
+        createSuccessfulRemoteDataObject$({
+          ...new ConfigurationProperty(),
+          name: trackingIdTestValue,
+          values: ['false'],
+        }),
+      );
+      configurationDataService.findByPropertyName = findByPropertyNameSpy;
+      service.initialize();
+      expect(findByPropertyNameSpy).not.toHaveBeenCalledWith(GOOGLE_ANALYTICS_KEY);
+      expect(service.klaroConfig.services).not.toContain(jasmine.objectContaining({ name: googleAnalytics }));
+    });
   });
 });