Adds "Invalid Survey" dialog instead of disabling "Publish changes" button (#2158)

Co-authored-by: Gino Miceli <[email protected]>

Author: rfontanarosa

web/src/app/components/header/header.component.html

@@ -54,7 +54,7 @@
         <button
           mat-flat-button
           class="finish-edit-button"
-          [disabled]="isPublishingChanges || !isDraftSurveyDirtyAndValid()"
+          [disabled]="isPublishingChanges"
           (click)="onFinishEditSurveyClick()"
         >
           Publish changes

web/src/app/components/header/header.component.ts

@@ -102,19 +102,28 @@ export class HeaderComponent {
   }
 
   async onFinishEditSurveyClick() {
-    this.isPublishingChanges = true;
-    await this.draftSurveyService.updateSurvey();
-    this.isPublishingChanges = false;
-    this.navigationService.selectSurvey(this.surveyId);
+    if (this.isDraftSurveyValid()) {
+      this.isPublishingChanges = true;
+      await this.draftSurveyService.updateSurvey();
+      this.isPublishingChanges = false;
+      this.navigationService.selectSurvey(this.surveyId);
+      return;
+    }
+
+    this.dialog.open(JobDialogComponent, {
+      data: {dialogType: DialogType.InvalidSurvey},
+      panelClass: 'small-width-dialog',
+    });
   }
 
-  isDraftSurveyDirtyAndValid() {
-    return (
-      this.draftSurveyService.dirty &&
-      this.draftSurveyService.valid.reduce(
-        (accumulator, currentValue) => accumulator && currentValue,
-        true
-      )
+  isDraftSurveyValid(): boolean {
+    return this.draftSurveyService.valid.reduce(
+      (accumulator, currentValue) => accumulator && currentValue,
+      true
     );
   }
+
+  isDraftSurveyDirtyAndValid(): boolean {
+    return this.draftSurveyService.dirty && this.isDraftSurveyValid();
+  }
 }

web/src/app/pages/edit-survey/job-dialog/job-dialog.component.html

@@ -17,62 +17,32 @@
 <h1 mat-dialog-title>{{ title }}</h1>
 
 <mat-dialog-content [ngSwitch]="data.dialogType">
-  <mat-form-field
-    *ngSwitchCase="[DialogType.AddJob, DialogType.RenameJob].includes(data.dialogType)
-      ? data.dialogType
-      : ''
-  ">
+  <p *ngIf="content">{{ content }}</p>
+
+  <mat-form-field *ngIf="[DialogType.AddJob, DialogType.RenameJob].includes(data.dialogType)">
     <mat-label>Job name</mat-label>
 
     <input [id]="jobNameFieldId" matInput [(ngModel)]="data.jobName" />
   </mat-form-field>
-
-  <p *ngSwitchCase="DialogType.DeleteJob">
-    This job and all of its associated data will be deleted. This operation
-    can’t be undone. Are you sure?
-  </p>
-
-  <p *ngSwitchCase="DialogType.UndoJobs">
-    If you leave this page, changes you’ve made to this survey won’t be
-    published. Are you sure you want to continue?
-  </p>
-
-  <p *ngSwitchCase="DialogType.DeleteLois">
-    All predefined data collection sites and their associated data will
-    be immediately deleted. This action cannot be undone.
-  </p>
-
-  <p *ngSwitchCase="DialogType.DeleteOption">
-    Are you sure you wish to delete this option?
-    All associated data will be lost. This cannot be undone.
-  </p>
-
-  <p *ngSwitchCase="DialogType.DeleteSurvey">
-    Are you sure you wish to delete this survey?
-    All associated data will be lost. This cannot be undone.
-  </p>
-
-  <p *ngSwitchCase="DialogType.DisableFreeForm">
-    Data collector will no longer be able to add new sites for this job.
-    Data will only be collected for existing sites.
-  </p>
 </mat-dialog-content>
 
 <mat-dialog-actions>
   <button
     mat-button
     color="primary"
     mat-dialog-close
+    *ngIf="backButtonLabel"
   >
-    Cancel
+    {{ backButtonLabel }}
   </button>
 
   <button
     mat-button
     color="primary"
     [mat-dialog-close]="data"
     cdkFocusInitial
+    *ngIf="continueButtonLabel"
   >
-    {{ buttonLabel }}
+    {{ continueButtonLabel }}
   </button>
 </mat-dialog-actions>

web/src/app/pages/edit-survey/job-dialog/job-dialog.component.spec.ts

@@ -0,0 +1,120 @@
+/**
+ * Copyright 2025 The Ground Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the 'License');
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an 'AS IS' BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import {
+  ComponentFixture,
+  TestBed,
+  fakeAsync,
+  tick,
+} from '@angular/core/testing';
+import {
+  MAT_DIALOG_DATA,
+  MatDialogModule,
+  MatDialogRef,
+} from '@angular/material/dialog';
+import {By} from '@angular/platform-browser';
+
+import {
+  DialogData,
+  DialogType,
+  JobDialogComponent,
+  dialogConfigs,
+} from './job-dialog.component';
+
+describe('JobDialogComponent', () => {
+  let component: JobDialogComponent;
+  let fixture: ComponentFixture<JobDialogComponent>;
+  let dialogRefSpy: jasmine.SpyObj<MatDialogRef<JobDialogComponent>>;
+
+  const mockDialogData: DialogData = {
+    dialogType: DialogType.UndoJobs,
+  };
+
+  beforeEach(async () => {
+    dialogRefSpy = jasmine.createSpyObj('MatDialogRef', ['close']);
+
+    await TestBed.configureTestingModule({
+      declarations: [JobDialogComponent],
+      imports: [MatDialogModule],
+      providers: [
+        {provide: MatDialogRef, useValue: dialogRefSpy},
+        {provide: MAT_DIALOG_DATA, useValue: mockDialogData},
+      ],
+    }).compileComponents();
+  });
+
+  beforeEach(() => {
+    fixture = TestBed.createComponent(JobDialogComponent);
+    component = fixture.componentInstance;
+    fixture.detectChanges();
+  });
+
+  it('should create', () => {
+    expect(component).toBeTruthy();
+  });
+
+  it('should close dialog when back button is clicked', () => {
+    const backButton = fixture.debugElement.query(
+      By.css('.mat-mdc-dialog-actions button:first-child')
+    );
+    backButton.nativeElement.click();
+    expect(dialogRefSpy.close).toHaveBeenCalled();
+  });
+
+  it('should close dialog when continue button is clicked', () => {
+    const continueButton = fixture.debugElement.query(
+      By.css('.mat-mdc-dialog-actions button:last-child')
+    );
+    continueButton.nativeElement.click();
+    expect(dialogRefSpy.close).toHaveBeenCalled();
+  });
+
+  it('should display the correct title in the template', () => {
+    const titleElement = fixture.debugElement.query(
+      By.css('.mat-mdc-dialog-title')
+    );
+    expect(titleElement.nativeElement.textContent).toContain(
+      dialogConfigs[DialogType.UndoJobs].title
+    );
+  });
+
+  it('should display the correct content in the template', () => {
+    const contentElement = fixture.debugElement.query(
+      By.css('.mat-mdc-dialog-content')
+    );
+    expect(contentElement.nativeElement.textContent).toContain(
+      dialogConfigs[DialogType.UndoJobs].content
+    );
+  });
+
+  it('should display the back button with the correct label', () => {
+    const backButton = fixture.debugElement.query(
+      By.css('.mat-mdc-dialog-actions button:first-child')
+    );
+    expect(backButton.nativeElement.textContent).toContain(
+      dialogConfigs[DialogType.UndoJobs].backButtonLabel
+    );
+  });
+
+  it('should display the continue button with the correct label', () => {
+    const continueButton = fixture.debugElement.query(
+      By.css('.mat-mdc-dialog-actions button:last-child')
+    );
+    expect(continueButton.nativeElement.textContent).toContain(
+      dialogConfigs[DialogType.UndoJobs].continueButtonLabel
+    );
+  });
+});

web/src/app/pages/edit-survey/job-dialog/job-dialog.component.ts

@@ -26,11 +26,79 @@ export enum DialogType {
   DeleteOption,
   DeleteSurvey,
   DisableFreeForm,
+  InvalidSurvey,
 }
 
+export interface DialogConfig {
+  title: string;
+  content?: string;
+  backButtonLabel?: string;
+  continueButtonLabel?: string;
+}
+
+export const dialogConfigs: Record<DialogType, DialogConfig> = {
+  [DialogType.AddJob]: {
+    title: 'Add new job',
+    backButtonLabel: 'Cancel',
+    continueButtonLabel: 'Create',
+  },
+  [DialogType.RenameJob]: {
+    title: 'Rename job',
+    backButtonLabel: 'Cancel',
+    continueButtonLabel: 'Rename',
+  },
+  [DialogType.UndoJobs]: {
+    title: 'Unpublished changes',
+    content:
+      'If you leave this page, changes you’ve made to this survey won’t be published. Are you sure you want to continue?',
+    backButtonLabel: 'Go back',
+    continueButtonLabel: 'Continue',
+  },
+  [DialogType.DeleteJob]: {
+    title: 'Delete job',
+    content:
+      'This job and all of its associated data will be deleted. This operation can’t be undone. Are you sure?',
+    backButtonLabel: 'Cancel',
+    continueButtonLabel: 'Confirm',
+  },
+  [DialogType.DeleteLois]: {
+    title: 'Delete predefined sites',
+    content:
+      'All predefined data collection sites and their associated data will be immediately deleted. This action cannot be undone.',
+    backButtonLabel: 'Cancel',
+    continueButtonLabel: 'Confirm',
+  },
+  [DialogType.DeleteOption]: {
+    title: 'Delete option',
+    content:
+      'Are you sure you wish to delete this option? All associated data will be lost. This cannot be undone.',
+    backButtonLabel: 'Cancel',
+    continueButtonLabel: 'Confirm',
+  },
+  [DialogType.DeleteSurvey]: {
+    title: 'Delete survey',
+    content:
+      'Are you sure you wish to delete this survey? All associated data will be lost. This cannot be undone.',
+    backButtonLabel: 'Cancel',
+    continueButtonLabel: 'Confirm',
+  },
+  [DialogType.DisableFreeForm]: {
+    title: 'Disable free-form data collection?',
+    content:
+      'Data collector will no longer be able to add new sites for this job. Data will only be collected for existing sites.',
+    backButtonLabel: 'Cancel',
+    continueButtonLabel: 'Confirm',
+  },
+  [DialogType.InvalidSurvey]: {
+    title: 'Fix issues with survey',
+    content: 'To publish changes, fix any outstanding issues with your survey.',
+    backButtonLabel: 'Go back',
+  },
+};
+
 export interface DialogData {
   dialogType: DialogType;
-  jobName: string;
+  jobName?: string;
 }
 
 @Component({
@@ -48,46 +116,24 @@ export class JobDialogComponent {
     @Inject(MAT_DIALOG_DATA) public data: DialogData
   ) {}
 
-  public get title() {
-    switch (this.data.dialogType) {
-      case DialogType.AddJob:
-        return 'Add new job';
-      case DialogType.RenameJob:
-        return 'Rename job';
-      case DialogType.UndoJobs:
-        return 'Unpublished changes';
-      case DialogType.DeleteJob:
-        return 'Delete job';
-      case DialogType.DeleteLois:
-        return 'Delete predefined sites';
-      case DialogType.DeleteOption:
-        return 'Delete option';
-      case DialogType.DeleteSurvey:
-        return 'Delete survey';
-      case DialogType.DisableFreeForm:
-        return 'Disable free-form data collection?';
-      default:
-        return '';
-    }
+  get dialogConfig(): DialogConfig {
+    return dialogConfigs[this.data.dialogType];
+  }
+
+  get title(): string {
+    return this.dialogConfig.title;
+  }
+
+  get content(): string | undefined {
+    return this.dialogConfig.content;
+  }
+
+  get backButtonLabel(): string | undefined {
+    return this.dialogConfig.backButtonLabel;
   }
 
-  public get buttonLabel() {
-    switch (this.data.dialogType) {
-      case DialogType.AddJob:
-        return 'Create';
-      case DialogType.RenameJob:
-        return 'Rename';
-      case DialogType.UndoJobs:
-        return 'Continue';
-      case DialogType.DeleteJob:
-      case DialogType.DeleteLois:
-      case DialogType.DeleteOption:
-      case DialogType.DeleteSurvey:
-      case DialogType.DisableFreeForm:
-        return 'Confirm';
-      default:
-        return '';
-    }
+  get continueButtonLabel(): string | undefined {
+    return this.dialogConfig.continueButtonLabel;
   }
 
   get jobNameFieldId() {