Feature: separate save / save & close buttons (#3575)

* Add setting to decide whether the edit dialog should automatically close on save

* Add the actual button to the ui

* Revert "Add the actual button to the ui"

This reverts commit e1f5a8bde0e8a2f6bafa2896d6a7e57e70d0c670.

* Revert "Add setting to decide whether the edit dialog should automatically close on save"

This reverts commit feef3c909b055b4bc4d2884324a4678348e1e536.

* Add button for save without exit

* Correct save button ordering, ensure perms, update translation strings

* fix e2e tests

* Add unit testing for save / save & close button

---------

Update messages.xlf

Update document-detail.component.spec.ts

Co-Authored-By: shamoon <4887959+shamoon@users.noreply.github.com>

src-ui/e2e/document-detail/document-detail.spec.ts

@@ -12,9 +12,13 @@ test('should activate / deactivate save button when changes are saved', async ({
   await expect(page.getByTitle('Storage path', { exact: true })).toHaveText(
     /\w+/
   )
-  await expect(page.getByRole('button', { name: 'Save' })).toBeDisabled()
+  await expect(
+    page.getByRole('button', { name: 'Save', exact: true })
+  ).toBeDisabled()
   await page.getByTitle('Storage path').getByTitle('Clear all').click()
-  await expect(page.getByRole('button', { name: 'Save' })).toBeEnabled()
+  await expect(
+    page.getByRole('button', { name: 'Save', exact: true })
+  ).toBeEnabled()
 })
 
 test('should warn on unsaved changes', async ({ page }) => {
@@ -23,13 +27,17 @@ test('should warn on unsaved changes', async ({ page }) => {
   await expect(page.getByTitle('Correspondent', { exact: true })).toHaveText(
     /\w+/
   )
-  await expect(page.getByRole('button', { name: 'Save' })).toBeDisabled()
+  await expect(
+    page.getByRole('button', { name: 'Save', exact: true })
+  ).toBeDisabled()
   await page
     .getByTitle('Storage path', { exact: true })
     .getByTitle('Clear all')
     .click()
-  await expect(page.getByRole('button', { name: 'Save' })).toBeEnabled()
+  await expect(
-  await page.getByRole('button', { name: 'Close' }).click()
+    page.getByRole('button', { name: 'Save', exact: true })
+  ).toBeEnabled()
+  await page.getByRole('button', { name: 'Close', exact: true }).click()
   await expect(page.getByRole('dialog')).toHaveText(/unsaved changes/)
   await page.getByRole('button', { name: 'Cancel' }).click()
   await page.getByRole('link', { name: 'Close all' }).click()

src-ui/src/app/components/document-detail/document-detail.component.html

@@ -191,9 +191,12 @@
             <div [ngbNavOutlet]="nav" class="mt-2"></div>
 
             <ng-container>
-                <button type="button" class="btn btn-outline-secondary" (click)="discard()" i18n [disabled]="!userCanEdit || networkActive || (isDirty$ | async) !== true">Discard</button>&nbsp;
+                <button type="button" class="btn btn-outline-secondary me-2" (click)="discard()" i18n [disabled]="!userCanEdit || networkActive || (isDirty$ | async) !== true">Discard</button>
-                <button type="button" class="btn btn-outline-primary" (click)="saveEditNext()" *ngIf="hasNext()" i18n [disabled]="!userCanEdit || networkActive || (isDirty$ | async) !== true">Save & next</button>&nbsp;
+                <ng-container *appIfPermissions="{ action: PermissionAction.Change, type: PermissionType.Document }">
-                <button type="submit" class="btn btn-primary" *appIfPermissions="{ action: PermissionAction.Change, type: PermissionType.Document }" i18n [disabled]="!userCanEdit || networkActive || (isDirty$ | async) !== true">Save</button>&nbsp;
+                    <button *ngIf="hasNext()" type="button" class="btn btn-outline-primary me-2" (click)="saveEditNext()" i18n [disabled]="!userCanEdit || networkActive || (isDirty$ | async) !== true">Save &amp; next</button>
+                    <button *ngIf="!hasNext()" type="button" class="btn btn-outline-primary me-2" (click)="save(true)" i18n [disabled]="!userCanEdit || networkActive || (isDirty$ | async) !== true">Save &amp; close</button>
+                    <button type="submit" class="btn btn-primary" i18n [disabled]="!userCanEdit || networkActive || (isDirty$ | async) !== true">Save</button>
+                </ng-container>
             </ng-container>
         </form>
     </div>

src-ui/src/app/components/document-detail/document-detail.component.spec.ts

@@ -372,12 +372,25 @@ describe('DocumentDetailComponent', () => {
     const updateSpy = jest.spyOn(documentService, 'update')
     const toastSpy = jest.spyOn(toastService, 'showInfo')
     updateSpy.mockImplementation((o) => of(doc))
-    component.save()
+    component.save(true)
     expect(updateSpy).toHaveBeenCalled()
     expect(closeSpy).toHaveBeenCalled()
     expect(toastSpy).toHaveBeenCalledWith('Document saved successfully.')
   })
 
+  it('should support save without close and show success toast', () => {
+    initNormally()
+    component.title = 'Foo Bar'
+    const closeSpy = jest.spyOn(component, 'close')
+    const updateSpy = jest.spyOn(documentService, 'update')
+    const toastSpy = jest.spyOn(toastService, 'showInfo')
+    updateSpy.mockImplementation((o) => of(doc))
+    component.save()
+    expect(updateSpy).toHaveBeenCalled()
+    expect(closeSpy).not.toHaveBeenCalled()
+    expect(toastSpy).toHaveBeenCalledWith('Document saved successfully.')
+  })
+
   it('should show toast error on save if error occurs', () => {
     currentUserHasObjectPermissions = true
     initNormally()
@@ -406,7 +419,7 @@ describe('DocumentDetailComponent', () => {
     updateSpy.mockImplementation(() =>
       throwError(() => new Error('failed to save'))
     )
-    component.save()
+    component.save(true)
     expect(updateSpy).toHaveBeenCalled()
     expect(closeSpy).toHaveBeenCalled()
     expect(toastSpy).toHaveBeenCalledWith('Document saved successfully.')
@@ -430,7 +443,7 @@ describe('DocumentDetailComponent', () => {
     expect
   })
 
-  it('should show toast error on saveAll if error occurs', () => {
+  it('should show toast error on save & next if error occurs', () => {
     currentUserHasObjectPermissions = true
     initNormally()
     component.title = 'Foo Bar'
@@ -448,6 +461,39 @@ describe('DocumentDetailComponent', () => {
     )
   })
 
+  it('should show save button and save & close or save & next', () => {
+    const nextSpy = jest.spyOn(component, 'hasNext')
+    nextSpy.mockReturnValueOnce(false)
+    fixture.detectChanges()
+    expect(
+      fixture.debugElement
+        .queryAll(By.css('button'))
+        .find((b) => b.nativeElement.textContent === 'Save')
+    ).not.toBeUndefined()
+    expect(
+      fixture.debugElement
+        .queryAll(By.css('button'))
+        .find((b) => b.nativeElement.textContent === 'Save & close')
+    ).not.toBeUndefined()
+    expect(
+      fixture.debugElement
+        .queryAll(By.css('button'))
+        .find((b) => b.nativeElement.textContent === 'Save & next')
+    ).toBeUndefined()
+    nextSpy.mockReturnValue(true)
+    fixture.detectChanges()
+    expect(
+      fixture.debugElement
+        .queryAll(By.css('button'))
+        .find((b) => b.nativeElement.textContent === 'Save & close')
+    ).toBeUndefined()
+    expect(
+      fixture.debugElement
+        .queryAll(By.css('button'))
+        .find((b) => b.nativeElement.textContent === 'Save & next')
+    ).not.toBeUndefined()
+  })
+
   it('should allow close and navigate to documents by default', () => {
     initNormally()
     const navigateSpy = jest.spyOn(router, 'navigate')

src-ui/src/app/components/document-detail/document-detail.component.ts

@@ -518,7 +518,7 @@ export class DocumentDetailComponent
       })
   }
 
-  save() {
+  save(close: boolean = false) {
     this.networkActive = true
     this.documentsService
       .update(this.document)
@@ -527,7 +527,7 @@ export class DocumentDetailComponent
         next: () => {
           this.store.next(this.documentForm.value)
           this.toastService.showInfo($localize`Document saved successfully.`)
-          this.close()
+          close && this.close()
           this.networkActive = false
           this.error = null
         },
@@ -535,7 +535,7 @@ export class DocumentDetailComponent
           this.networkActive = false
           if (!this.userCanEdit) {
             this.toastService.showInfo($localize`Document saved successfully.`)
-            this.close()
+            close && this.close()
           } else {
             this.error = error.error
             this.toastService.showError(