Improve repo_read_only header on the UI (#18729)

1. Fixes #18694 2. Now non-system-admin users can also see the repo_read_only header Signed-off-by: AllForNothing <[email protected]>
goharbor · May 24, 2023 · c399e5e · c399e5e
1 parent 43c6586
commit c399e5e
Show file tree

Hide file tree

Showing 10 changed files with 57 additions and 64 deletions.
diff --git a/src/portal/src/app/base/account-settings/account-settings-modal.component.html b/src/portal/src/app/base/account-settings/account-settings-modal.component.html
@@ -224,7 +224,9 @@ <h3 class="modal-title">{{ 'PROFILE.TITLE' | translate }}</h3>
             type="button"
             id="submit-btn"
             class="btn btn-primary"
-            [disabled]="!isValid || showProgress || !isUserDataChange()"
+            [disabled]="
+                !isValid || showProgress || !isUserDataChange() || checkProgress
+            "
             (click)="submit()">
             {{ 'BUTTON.OK' | translate }}
         </button>

diff --git a/src/portal/src/app/base/account-settings/account-settings-modal.component.spec.ts b/src/portal/src/app/base/account-settings/account-settings-modal.component.spec.ts
@@ -18,6 +18,7 @@ import { InlineAlertComponent } from '../../shared/components/inline-alert/inlin
 import { ConfirmationDialogService } from '../global-confirmation-dialog/confirmation-dialog.service';
 import { ConfirmationMessage } from '../global-confirmation-dialog/confirmation-message';
 import { UserService } from '../../../../ng-swagger-gen/services/user.service';
+import { AppConfigService } from '../../services/app-config.service';
 
 describe('AccountSettingsModalComponent', () => {
     let component: AccountSettingsModalComponent;
@@ -74,6 +75,12 @@ describe('AccountSettingsModalComponent', () => {
         },
     };
 
+    const MockedAppConfigService = {
+        getConfig() {
+            return { self_registration: true };
+        },
+    };
+
     beforeEach(async () => {
         await TestBed.configureTestingModule({
             declarations: [
@@ -110,6 +117,7 @@ describe('AccountSettingsModalComponent', () => {
                     provide: ConfirmationDialogService,
                     useValue: fakeConfirmationDialogService,
                 },
+                { provide: AppConfigService, useValue: MockedAppConfigService },
             ],
             schemas: [CUSTOM_ELEMENTS_SCHEMA],
         }).compileComponents();

diff --git a/src/portal/src/app/base/account-settings/account-settings-modal.component.ts b/src/portal/src/app/base/account-settings/account-settings-modal.component.ts
@@ -30,6 +30,7 @@ import { ConfirmationDialogComponent } from '../../shared/components/confirmatio
 import { InlineAlertComponent } from '../../shared/components/inline-alert/inline-alert.component';
 import { ConfirmationMessage } from '../global-confirmation-dialog/confirmation-message';
 import { UserService } from 'ng-swagger-gen/services/user.service';
+import { AppConfigService } from '../../services/app-config.service';
 
 @Component({
     selector: 'account-settings-modal',
@@ -72,7 +73,8 @@ export class AccountSettingsModalComponent implements OnInit, AfterViewChecked {
         private msgHandler: MessageHandlerService,
         private router: Router,
         private searchTrigger: SearchTriggerService,
-        private userService: UserService
+        private userService: UserService,
+        private appConfigService: AppConfigService
     ) {}
 
     private validationStateMap: any = {
@@ -136,29 +138,34 @@ export class AccountSettingsModalComponent implements OnInit, AfterViewChecked {
                             return;
                         }
 
-                        // Mail changed
-                        this.checkOnGoing = true;
-                        this.session
-                            .checkUserExisting('email', this.account.email)
-                            .subscribe(
-                                (res: boolean) => {
-                                    this.checkOnGoing = false;
-                                    this.validationStateMap[key] = !res;
-                                    if (res) {
-                                        this.emailTooltip =
-                                            'TOOLTIP.EMAIL_EXISTING';
+                        // Mail changed, if self-registration disabled, only system admin can check mail-existing status
+                        if (
+                            this.session.getCurrentUser()?.has_admin_role ||
+                            this.appConfigService.getConfig()?.self_registration
+                        ) {
+                            this.checkOnGoing = true;
+                            this.session
+                                .checkUserExisting('email', this.account.email)
+                                .subscribe(
+                                    (res: boolean) => {
+                                        this.checkOnGoing = false;
+                                        this.validationStateMap[key] = !res;
+                                        if (res) {
+                                            this.emailTooltip =
+                                                'TOOLTIP.EMAIL_EXISTING';
+                                        }
+                                        this.mailAlreadyChecked[
+                                            this.account.email
+                                        ] = {
+                                            result: res,
+                                        }; // Tag it checked
+                                    },
+                                    error => {
+                                        this.checkOnGoing = false;
+                                        this.validationStateMap[key] = false; // Not valid @ backend
                                     }
-                                    this.mailAlreadyChecked[
-                                        this.account.email
-                                    ] = {
-                                        result: res,
-                                    }; // Tag it checked
-                                },
-                                error => {
-                                    this.checkOnGoing = false;
-                                    this.validationStateMap[key] = false; // Not valid @ backend
-                                }
-                            );
+                                );
+                        }
                     }
                 }
             }

diff --git a/src/portal/src/app/base/harbor-shell/app-level-alerts/app-level-alerts.component.html b/src/portal/src/app/base/harbor-shell/app-level-alerts/app-level-alerts.component.html
@@ -1,11 +1,11 @@
 <clr-alerts>
     <clr-alert
         *ngIf="showReadOnly && isLogin()"
-        [clrAlertType]="message?.type"
+        [clrAlertType]="'warning'"
         [clrAlertAppLevel]="true"
         [clrAlertClosable]="false">
         <clr-alert-item>
-            <span class="alert-text">{{ message?.message | translate }}</span>
+            <span class="alert-text">{{ 'REPO_READ_ONLY' | translate }}</span>
         </clr-alert-item>
     </clr-alert>
     <clr-alert

diff --git a/src/portal/src/app/base/harbor-shell/app-level-alerts/app-level-alerts.component.spec.ts b/src/portal/src/app/base/harbor-shell/app-level-alerts/app-level-alerts.component.spec.ts
@@ -7,6 +7,7 @@ import { delay } from 'rxjs/operators';
 import { Scanner } from '../../left-side-nav/interrogation-services/scanner/scanner';
 import { ScannerService } from 'ng-swagger-gen/services/scanner.service';
 import { SessionService } from 'src/app/shared/services/session.service';
+import { AppConfigService } from '../../../services/app-config.service';
 
 describe('AppLevelAlertsComponent', () => {
     let component: AppLevelAlertsComponent;
@@ -44,6 +45,11 @@ describe('AppLevelAlertsComponent', () => {
             return { has_admin_role: true };
         },
     };
+    const MockedAppConfigService = {
+        getConfig() {
+            return { read_only: false };
+        },
+    };
 
     beforeEach(async () => {
         await TestBed.configureTestingModule({
@@ -55,6 +61,7 @@ describe('AppLevelAlertsComponent', () => {
                     useValue: fakeScannerService,
                 },
                 { provide: SessionService, useValue: fakeSessionService },
+                { provide: AppConfigService, useValue: MockedAppConfigService },
             ],
         }).compileComponents();
 

diff --git a/src/portal/src/app/base/harbor-shell/app-level-alerts/app-level-alerts.component.ts b/src/portal/src/app/base/harbor-shell/app-level-alerts/app-level-alerts.component.ts
@@ -14,7 +14,7 @@ import { ActivatedRoute, Router } from '@angular/router';
 import { MessageService } from '../../../shared/components/global-message/message.service';
 import { Message } from '../../../shared/components/global-message/message';
 import { JobServiceDashboardHealthCheckService } from '../../left-side-nav/job-service-dashboard/job-service-dashboard-health-check.service';
-import { AppLevelMessage } from '../../../shared/services/message-handler.service';
+import { AppConfigService } from '../../../services/app-config.service';
 const HAS_SHOWED_SCANNER_INFO: string = 'hasShowScannerInfo';
 const YES: string = 'yes';
 @Component({
@@ -29,14 +29,14 @@ export class AppLevelAlertsComponent implements OnInit, OnDestroy {
     appLevelMsgSub: Subscription;
     clearSub: Subscription;
     showLogin: boolean = false;
-    showReadOnly: boolean = false;
     constructor(
         private session: SessionService,
         private scannerService: ScannerService,
         private router: Router,
         private messageService: MessageService,
         private route: ActivatedRoute,
-        private jobServiceDashboardHealthCheckService: JobServiceDashboardHealthCheckService
+        private jobServiceDashboardHealthCheckService: JobServiceDashboardHealthCheckService,
+        private appConfigService: AppConfigService
     ) {}
     ngOnInit() {
         if (
@@ -53,10 +53,6 @@ export class AppLevelAlertsComponent implements OnInit, OnDestroy {
             this.appLevelMsgSub =
                 this.messageService.appLevelAnnounced$.subscribe(message => {
                     this.message = message;
-                    this.showReadOnly =
-                        message.statusCode === httpStatusCode.AppLevelWarning &&
-                        message.message === AppLevelMessage.REPO_READ_ONLY;
-
                     if (message.statusCode === httpStatusCode.Unauthorized) {
                         this.showLogin = true;
                         // User session timed out, then redirect to sign-in page
@@ -86,7 +82,6 @@ export class AppLevelAlertsComponent implements OnInit, OnDestroy {
         if (!this.clearSub) {
             this.clearSub = this.messageService.clearChan$.subscribe(clear => {
                 this.showLogin = false;
-                this.showReadOnly = false;
             });
         }
     }
@@ -96,7 +91,9 @@ export class AppLevelAlertsComponent implements OnInit, OnDestroy {
             this.appLevelMsgSub = null;
         }
     }
-
+    get showReadOnly(): boolean {
+        return this.appConfigService.getConfig()?.read_only;
+    }
     shouldShowScannerInfo(): boolean {
         return (
             this.session.getCurrentUser()?.has_admin_role &&
@@ -186,6 +183,6 @@ export class AppLevelAlertsComponent implements OnInit, OnDestroy {
     }
 
     isLogin(): boolean {
-        return this.session.getCurrentUser()?.has_admin_role;
+        return !!this.session.getCurrentUser();
     }
 }
diff --git a/src/portal/src/app/base/left-side-nav/config/system/system-settings.component.ts b/src/portal/src/app/base/left-side-nav/config/system/system-settings.component.ts
@@ -139,12 +139,6 @@ export class SystemSettingsComponent implements OnInit {
                         // Unfortunately API does not do that
                         // So we need to call update function again
                         this.conf.updateConfig();
-                        // Handle read only
-                        if (changes['read_only']) {
-                            this.errorHandler.handleReadOnly();
-                        } else {
-                            this.errorHandler.clear();
-                        }
                         // Reload bootstrap option
                         this.appConfigService.load().subscribe(
                             () => {},

diff --git a/src/portal/src/app/shared/components/navigator/navigator.component.ts b/src/portal/src/app/shared/components/navigator/navigator.component.ts
@@ -84,9 +84,6 @@ export class NavigatorComponent implements OnInit {
             this.translateClarityComponents();
         }
         this.selectedDatetimeRendering = getDatetimeRendering();
-        if (this.appConfigService.getConfig().read_only) {
-            this.msgHandler.handleReadOnly();
-        }
     }
     //Internationalization for Clarity components, refer to https://clarity.design/documentation/internationalization
     translateClarityComponents() {

diff --git a/src/portal/src/app/shared/router-guard/auth-user-activate.service.ts b/src/portal/src/app/shared/router-guard/auth-user-activate.service.ts
@@ -44,13 +44,6 @@ export class AuthCheckGuard {
     ): Observable<boolean> | boolean {
         // When routing change, clear
         this.msgHandler.clear();
-        if (
-            this.appConfigService.getConfig().read_only &&
-            this.appConfigService.getConfig().read_only.toString() === 'true'
-        ) {
-            this.msgHandler.handleReadOnly();
-        }
-
         this.searchTrigger.closeSearch(true);
         return new Observable(observer => {
             // if the url has the queryParam `publicAndNotLogged=yes`, then skip auth check

diff --git a/src/portal/src/app/shared/services/message-handler.service.ts b/src/portal/src/app/shared/services/message-handler.service.ts
@@ -72,14 +72,6 @@ export class MessageHandlerService implements ErrorHandler {
         }
     }
 
-    public handleReadOnly(): void {
-        this.msgService.announceAppLevelMessage(
-            httpStatusCode.AppLevelWarning,
-            AppLevelMessage.REPO_READ_ONLY,
-            AlertType.WARNING
-        );
-    }
-
     public showError(message: string, params: any): void {
         if (!params) {
             params = {};
@@ -131,7 +123,3 @@ export class MessageHandlerService implements ErrorHandler {
         this.showInfo(log);
     }
 }
-
-export enum AppLevelMessage {
-    REPO_READ_ONLY = 'REPO_READ_ONLY',
-}