LP#1956986: fix and refactor item alert handing in Angular editor
authorGalen Charlton <gmc@equinoxOLI.org>
Mon, 11 Jul 2022 16:31:36 +0000 (12:31 -0400)
committerBill Erickson <berickxx@gmail.com>
Wed, 27 Jul 2022 20:35:56 +0000 (16:35 -0400)
This patch changes the handling of item alerts in the Angular
Holdings View and Item Attributes editor to be more like how
item notes and item tags are handled.

In particular:

- The separate 'Add Item Alert' and 'Edit Item Alerts' context menu
  items in Holdings View are consolidated to a 'Add/Manage Item Alerts'
  action.
- Pending new alerts are displayed in separate area of the modal
- The 'Apply Changes' and 'Add New' buttons now work the same way as
  item notes and item tags

To test
-------
[1] Apply patch.
[2] Create a new item with one or more item alerts. Verify that they are
    properly saved.
[3] Edit an existing item and add, modify, and clear alerts. Verify that
    the changes are saved.
[4] From Holdings View, choose 'Add/Manage Item Alerts' with a single
    item selected. Verify that changes are saved.
[5] From Holdings View, choose 'Add/Manage Item Alerts' with multiple
    items selected. Verify that you are only given the option to add
    new alerts. Verify that new alerts are saved.

Signed-off-by: Galen Charlton <gmc@equinoxOLI.org>
Signed-off-by: Bill Erickson <berickxx@gmail.com>
Signed-off-by: Elaine Hardy <ehardy@georgialibraries.org>
Open-ILS/src/eg2/src/app/staff/cat/volcopy/copy-attrs.component.ts
Open-ILS/src/eg2/src/app/staff/cat/volcopy/volcopy.component.ts
Open-ILS/src/eg2/src/app/staff/catalog/record/holdings.component.html
Open-ILS/src/eg2/src/app/staff/catalog/record/holdings.component.ts
Open-ILS/src/eg2/src/app/staff/share/holdings/copy-alerts-dialog.component.html
Open-ILS/src/eg2/src/app/staff/share/holdings/copy-alerts-dialog.component.ts

index d4f41d4..ecd0e86 100644 (file)
@@ -422,10 +422,18 @@ export class CopyAttrsComponent implements OnInit, AfterViewInit {
         this.copyAlertsDialog.inPlaceCreateMode = true;
         this.copyAlertsDialog.copyIds = this.context.copyList().map(c => c.id());
 
-        this.copyAlertsDialog.open({size: 'lg'}).subscribe(
-            newAlert => {
-                if (newAlert) {
-                    this.context.copyList().forEach(copy => {
+        this.copyAlertsDialog.open({size: 'lg'}).subscribe(changes => {
+            if (!changes) { return; }
+
+            if ((!changes.newAlerts || changes.newAlerts.length === 0) &&
+                (!changes.changedAlerts || changes.changedAlerts.length === 0)
+               ) {
+                return;
+            }
+
+            if (changes.newAlerts) {
+                this.context.copyList().forEach(copy => {
+                    changes.newAlerts.forEach(newAlert => {
                         const a = this.idl.clone(newAlert);
                         a.isnew(true);
                         a.copy(copy.id());
@@ -433,9 +441,25 @@ export class CopyAttrsComponent implements OnInit, AfterViewInit {
                         copy.copy_alerts().push(a);
                         copy.ischanged(true);
                     });
-                }
+                });
             }
-        );
+            if (changes.changedAlerts && this.context.copyList().length === 1) {
+                const copy = this.context.copyList()[0];
+                changes.changedAlerts.forEach(alert => {
+                    const existing = copy.copy_alerts().filter(a => a.id() === alert.id())[0];
+                    if (existing) {
+                        existing.ischanged(true);
+                        existing.alert_type(alert.alert_type());
+                        existing.temp(alert.temp());
+                        existing.ack_time(alert.ack_time());
+                        if (alert.ack_time() === 'now') {
+                            existing.ack_staff(this.auth.user().id());
+                        }
+                        copy.ischanged(true);
+                    }
+                });
+            }
+        });
     }
 
     openCopyTags() {
index c650d6d..8268967 100644 (file)
@@ -22,7 +22,8 @@ const COPY_FLESH = {
     flesh_fields: {
         acp: [
             'call_number', 'location', 'parts', 'tags',
-            'creator', 'editor', 'stat_cat_entries', 'notes'
+            'creator', 'editor', 'stat_cat_entries', 'notes',
+            'copy_alerts'
         ],
         acptcm: ['tag'],
         acpt: ['tag_type']
index 14a173e..966a570 100644 (file)
     </eg-grid-toolbar-action>
 
     <eg-grid-toolbar-action
-      i18n-group group="Add" i18n-label label="Add Item Alerts"
-      (onClick)="openItemAlerts($event, 'create')">
+      i18n-group group="Add" i18n-label label="Add/Manage Item Alerts"
+      (onClick)="openItemAlerts($event)">
     </eg-grid-toolbar-action>
 
     <eg-grid-toolbar-action
     </eg-grid-toolbar-action>
     
     <eg-grid-toolbar-action
-      i18n-group group="Edit" i18n-label label="Edit Item Alerts"
-      (onClick)="openItemAlerts($event, 'manage')">
-    </eg-grid-toolbar-action>
-
-    <eg-grid-toolbar-action
       i18n-group group="Edit" i18n-label label="Replace Barcodes"
       (onClick)="openReplaceBarcodeDialog($event)">
     </eg-grid-toolbar-action>
index 922cc1e..0c31d59 100644 (file)
@@ -935,15 +935,15 @@ export class HoldingsMaintenanceComponent implements OnInit {
         }
     }
 
-    openItemAlerts(rows: HoldingsEntry[], mode: string) {
+    openItemAlerts(rows: HoldingsEntry[]) {
         const copyIds = this.selectedCopyIds(rows);
         if (copyIds.length === 0) { return; }
 
         this.copyAlertsDialog.copyIds = copyIds;
-        this.copyAlertsDialog.mode = mode;
         this.copyAlertsDialog.open({size: 'lg'}).subscribe(
-            modified => {
-                if (modified) {
+            changes => {
+                if (!changes) { return; }
+                if (changes.newAlerts.length > 0 || changes.changedAlerts.length > 0) {
                     this.hardRefresh();
                 }
             }
index db782ad..96cba1a 100644 (file)
@@ -21,8 +21,9 @@
     <div class="row mt-2 p-2 rounded border border-success">
       <div class="col-lg-4">
         <eg-combobox [entries]="alertTypes" 
-          i18n-placeholder placeholder="New Alert Type..."
-          [required]="true"
+          i18n-placeholder placeholder="Select Alert Type..."
+          [selectedId]="newAlert.alert_type()"
+          [mandatory]="true"
           (onChange)="newAlert.alert_type($event ? $event.id : null)">
         </eg-combobox>
       </div>
         </div>  
       </div>
     </div>
+    <h4 class="mt-2" i18n *ngIf="newAlerts.length > 0">Pending New Alerts</h4>
+    <div class="row mt-2" *ngFor="let alert of newAlerts">
+      <div class="col-lg-4">{{getAlertTypeLabel(alert)}}</div>
+      <div class="col-lg-5">{{alert.note()}}</div>
+      <div class="col-lg-3">
+        <button class="btn btn-outline-danger" (click)="removeAlert(alert)" i18n>
+          Remove
+        </button>
+      </div>
+    </div>
+
     <ng-container *ngIf="mode == 'manage'">
       <!-- in manage mode list all of the alerts linked to the copy -->
       <div class="row mt-2" 
   <div class="modal-footer">
     <button type="button" class="btn btn-secondary" 
       (click)="close()" i18n>Close</button>
-    <ng-container *ngIf="mode == 'manage'">
-      <button class="btn btn-success mr-2" 
-        (click)="applyChanges()" i18n>Apply Changes</button>
-    </ng-container>
+    <button class="btn btn-success mr-2" 
+      (click)="applyChanges()" i18n>Apply Changes</button>
   </div>
 </ng-template>
index 85960fc..8ae114c 100644 (file)
@@ -17,6 +17,11 @@ import {ComboboxEntry} from '@eg/share/combobox/combobox.component';
  * Dialog for managing copy alerts.
  */
 
+export interface CopyAlertsChanges {
+    newAlerts: IdlObject[];
+    changedAlerts: IdlObject[];
+}
+
 @Component({
   selector: 'eg-copy-alerts-dialog',
   templateUrl: 'copy-alerts-dialog.component.html'
@@ -42,6 +47,8 @@ export class CopyAlertsDialogComponent
 
     alertTypes: ComboboxEntry[];
     newAlert: IdlObject;
+    newAlerts: IdlObject[];
+    autoId = -1;
     changesMade: boolean;
 
     @ViewChild('successMsg', { static: true }) private successMsg: StringComponent;
@@ -67,10 +74,11 @@ export class CopyAlertsDialogComponent
      * Dialog promise resolves with true/false indicating whether
      * the mark-damanged action occured or was dismissed.
      */
-    open(args: NgbModalOptions): Observable<boolean> {
+    open(args: NgbModalOptions): Observable<CopyAlertsChanges> {
         this.copy = null;
         this.copies = [];
         this.newAlert = this.idl.create('aca');
+        this.newAlerts = [];
         this.newAlert.create_staff(this.auth.user().id());
 
         if (this.copyIds.length === 0 && !this.inPlaceCreateMode) {
@@ -142,48 +150,61 @@ export class CopyAlertsDialogComponent
         });
     }
 
+    getAlertTypeLabel(alert: IdlObject): string {
+        const alertType = this.alertTypes.filter(t => t.id === alert.alert_type());
+        return alertType[0].label;
+    }
+
+    removeAlert(alert: IdlObject) {
+        // the only type of alerts we can remove are pending ones that
+        // we have created during the lifetime of this modal; alerts
+        // that already exist can only be cleared
+        this.newAlerts = this.newAlerts.filter(t => t.id() !== alert.id());
+    }
+
     // Add the in-progress new note to all copies.
     addNew() {
         if (!this.newAlert.alert_type()) { return; }
 
+        this.newAlert.id(this.autoId--);
+        this.newAlert.isnew(true);
+        this.newAlerts.push(this.newAlert);
+
+        this.newAlert = this.idl.create('aca');
+
+    }
+
+    applyChanges() {
+
+        const changedAlerts = this.copy ?
+            this.copy.copy_alerts().filter(a => a.ischanged()) :
+            [];
         if (this.inPlaceCreateMode) {
-            this.close(this.newAlert);
+            this.close({ newAlerts: this.newAlerts, changedAlerts: changedAlerts });
             return;
         }
 
-        const alerts: IdlObject[] = [];
-        this.copies.forEach(c => {
-            const a = this.idl.clone(this.newAlert);
-            a.copy(c.id());
-            alerts.push(a);
+        const alerts = [];
+        this.newAlerts.forEach(alert => {
+            this.copies.forEach(c => {
+                const a = this.idl.clone(alert);
+                a.isnew(true);
+                a.id(null);
+                a.copy(c.id());
+                alerts.push(a);
+            });
         });
-
-        this.pcrud.create(alerts).toPromise().then(
-            newAlert => {
+        if (this.mode === 'manage') {
+            changedAlerts.forEach(alert => {
+                alerts.push(alert);
+            });
+        }
+        this.pcrud.autoApply(alerts).toPromise().then(
+            ok => {
                 this.successMsg.current().then(msg => this.toast.success(msg));
-                this.changesMade = true;
-                if (this.mode === 'create') {
-                    // In create mode, we assume the user wants to create
-                    // a single alert and be done with it.
-                    this.close(this.changesMade);
-                } else {
-                    // Otherwise, add the alert to the copy
-                    this.copy.copy_alerts().push(newAlert);
-                }
+                this.close({ newAlerts: this.newAlerts, changedAlerts: changedAlerts });
             },
-            err => {
-                this.errorMsg.current().then(msg => this.toast.danger(msg));
-            }
-        );
-    }
-
-    applyChanges() {
-        const alerts = this.copy.copy_alerts().filter(a => a.ischanged());
-        if (alerts.length === 0) { return; }
-        this.pcrud.update(alerts).toPromise().then(
-            ok => this.successMsg.current().then(msg => this.toast.success(msg)),
             err => this.errorMsg.current().then(msg => this.toast.danger(msg))
         );
     }
 }
-