LP#1851884: eg-fm-record-editor: avoid fetching all rows from linked table

This patch ensures that Angular FM record editor dialogs do
not attempt to fetch all rows from target table when constructing
a combobox for a linked field.

In particular, it makes the following changes:

* If a custom template for a field is supplied, use that by
  default rather than _also_ creating a data source for
  a combobox.
* Rather than creating its own data source for a linked field,
  the FM record editor now uses the IDL mode of eg-combobox. By doing
  this, we use eg-combobox's default data source, which limits
  record retrievals to 100 rows max. Also, empty-click is now
  enabled by default.
* When attempting to identify a selector for an IDL class, if
  the class doesn't define a selector and doesn't have a field named
  'name', but its primary key is a text field, use the primary
  key as the selector.

To test
-------
[1] Create a few thousand empty bib record buckets in your test database.
[2] Edit a carousel under Local Administration; note that it can
    take some time for the dialog to load.
[3] Apply the patch and repeat step 2. This time, the carousel edit modal
    should open instantly.
[4] Test various Angular record editor modals to confirm that they behave
    as expected. For example:

    * The allocate to fund dialog in Fund Administration, in particular
      to verify that only active funds show up.
    * The assign user to course modal, to verify that searching is done
      by course number, not name.
    * Filter Dialog Set editor under /eg2/en-US/staff/admin/local/config/filter_dialog_filter_set
      to verify that the drop down for the creator doesn't fetch
      all patrons. (Note that this is an artificial example).

Signed-off-by: Galen Charlton <gmc@equinoxOLI.org>
Signed-off-by: Tiffany Little <tlittle@georgialibraries.org>

diff --git a/Open-ILS/src/eg2/src/app/core/idl.service.ts b/Open-ILS/src/eg2/src/app/core/idl.service.ts
index 662c735..3741e93 100644 (file)
--- a/Open-ILS/src/eg2/src/app/core/idl.service.ts
+++ b/Open-ILS/src/eg2/src/app/core/idl.service.ts
@@ -161,7 +161,9 @@ export class IdlService {
     }
 
     // Return the selector field for the class.  If no selector is
-    // defined, use 'name' if it exists as a field on the class.
+    // defined, use 'name' if it exists as a field on the class. As
+    // a last ditch fallback, if there's no selector but the primary
+    // key is a text field, use that.
     getClassSelector(idlClass: string): string {
 
         if (idlClass) {
@@ -173,6 +175,13 @@ export class IdlService {
 
                 // No selector defined in the IDL, try 'name'.
                 if ('name' in classDef.field_map) { return 'name'; }
+
+                // last ditch - if the primary key is a text field,
+                // treat it as the selector
+                if (classDef.field_map[classDef.pkey].datatype === 'text') {
+                    return classDef.pkey;
+                }
+
             }
         }
 

diff --git a/Open-ILS/src/eg2/src/app/share/fm-editor/fm-editor.component.html b/Open-ILS/src/eg2/src/app/share/fm-editor/fm-editor.component.html
index 859dc0b..deda3e5 100644 (file)
--- a/Open-ILS/src/eg2/src/app/share/fm-editor/fm-editor.component.html
+++ b/Open-ILS/src/eg2/src/app/share/fm-editor/fm-editor.component.html
@@ -161,6 +161,19 @@
               </ng-container>
             </ng-container>
 
+            <ng-container *ngSwitchCase="'link'">
+              <eg-combobox
+                id="{{idPrefix}}-{{field.name}}" name="{{field.name}}"
+                placeholder="{{field.label}}..." i18n-placeholder 
+                [required]="field.isRequired()"
+                [idlClass]="field.class" [asyncSupportsEmptyTermClick]="true"
+                [idlBaseQuery]="field.idlBaseQuery"
+                [idlField]="field.selector"
+                [selectedId]="record[field.name]()"
+                (onChange)="record[field.name]($event ? $event.id : null)">
+              </eg-combobox>
+            </ng-container>
+
             <ng-container *ngSwitchCase="'list'">
               <eg-combobox
                 id="{{idPrefix}}-{{field.name}}" name="{{field.name}}"

diff --git a/Open-ILS/src/eg2/src/app/share/fm-editor/fm-editor.component.ts b/Open-ILS/src/eg2/src/app/share/fm-editor/fm-editor.component.ts
index 94b3549..8c1f5ba 100644 (file)
--- a/Open-ILS/src/eg2/src/app/share/fm-editor/fm-editor.component.ts
+++ b/Open-ILS/src/eg2/src/app/share/fm-editor/fm-editor.component.ts
@@ -518,7 +518,10 @@ export class FmRecordEditorComponent
             };
         }
 
-        if (fieldOptions.customValues) {
+        if (fieldOptions.customTemplate) {
+            field.template = fieldOptions.customTemplate.template;
+            field.context = fieldOptions.customTemplate.context;
+        } else if (fieldOptions.customValues) {
 
             field.linkedValues = fieldOptions.customValues;
 
@@ -550,7 +553,11 @@ export class FmRecordEditorComponent
 
         } else if (field.datatype === 'link') {
 
-            promise = this.wireUpCombobox(field);
+            if (fieldOptions.linkedSearchConditions) {
+                field.idlBaseQuery = fieldOptions.linkedSearchConditions;
+            }
+            field.selector = fieldOptions.linkedSearchField ||
+                             this.idl.getClassSelector(field.class);
 
         } else if (field.datatype === 'timestamp') {
             field.datetime = this.datetimeFieldsList.includes(field.name);
@@ -559,11 +566,6 @@ export class FmRecordEditorComponent
                 this.orgDefaultAllowedList.includes(field.name);
         }
 
-        if (fieldOptions.customTemplate) {
-            field.template = fieldOptions.customTemplate.template;
-            field.context = fieldOptions.customTemplate.context;
-        }
-
         if (fieldOptions.helpText) {
             field.helpText = fieldOptions.helpText;
             field.helpText.current().then(help => field.helpTextValue = help);
@@ -579,103 +581,6 @@ export class FmRecordEditorComponent
         return promise || Promise.resolve();
     }
 
-    wireUpCombobox(field: any): Promise<any> {
-
-        const fieldOptions = this.fieldOptions[field.name] || {};
-
-        // globally preloading unless a field-specific value is set.
-        if (this.preloadLinkedValues) {
-            if (!('preloadLinkedValues' in fieldOptions)) {
-                fieldOptions.preloadLinkedValues = true;
-            }
-        }
-
-        const selector = fieldOptions.linkedSearchField ||
-            this.idl.getClassSelector(field.class);
-
-        if (!selector && !fieldOptions.preloadLinkedValues) {
-            // User probably expects an async data source, but we can't
-            // provide one without a selector.  Warn the user.
-            console.warn(`Class ${field.class} has no selector.
-                Pre-fetching all rows for combobox`);
-        }
-
-        if (fieldOptions.preloadLinkedValues || !selector) {
-            const search = {};
-            const orderBy = {order_by: {}};
-            if (selector) {
-                orderBy.order_by[field.class] = selector;
-            }
-            const idField = this.idl.classes[field.class].pkey || 'id';
-            search[idField] = {'!=' : null};
-            if (fieldOptions.linkedSearchConditions) {
-                const conditions = {};
-                Object.keys(fieldOptions.linkedSearchConditions).forEach(key => {
-                    conditions[key] = fieldOptions.linkedSearchConditions[key];
-                });
-                // ensure that the current value, if present, is included
-                // in case it doesn't otherwise meet the conditions
-                const linkedValue = this.record[field.name]();
-                if (linkedValue !== null && linkedValue !== undefined) {
-                    search['-or'] = [];
-                    const retrieveRec = {};
-                    retrieveRec[idField] = linkedValue;
-                    search['-or'].push(retrieveRec);
-                    search['-or'].push(conditions);
-                } else {
-                    // just tack on the conditions
-                    Object.keys(conditions).forEach(key => {
-                        search[key] = conditions[key];
-                    });
-                }
-            }
-            return this.pcrud.search(field.class, search, orderBy, {atomic : true})
-            .toPromise().then(list => {
-                field.linkedValues =
-                    this.flattenLinkedValues(field, list);
-            });
-        }
-
-        // If we have a selector, wire up for async data retrieval
-        field.linkedValuesSource =
-            (term: string): Observable<ComboboxEntry> => {
-
-            const search = {};
-            const orderBy = {order_by: {}};
-            const idField = this.idl.classes[field.class].pkey || 'id';
-
-            search[selector] = {'ilike': `%${term}%`};
-            if (fieldOptions.linkedSearchConditions) {
-                Object.keys(fieldOptions.linkedSearchConditions).forEach(key => {
-                    search[key] = fieldOptions.linkedSearchConditions[key];
-                });
-            }
-            orderBy.order_by[field.class] = selector;
-
-            return this.pcrud.search(field.class, search, orderBy)
-            .pipe(map(idlThing =>
-                // Map each object into a ComboboxEntry upon arrival
-                this.flattenLinkedValues(field, [idlThing])[0]
-            ));
-        };
-
-        // Using an async data source, but a value is already set
-        // on the field.  Fetch the linked object and add it to the
-        // combobox entry list so it will be avilable for display
-        // at dialog load time.
-        const linkVal = this.record[field.name]();
-        if (linkVal !== null && linkVal !== undefined) {
-            return this.pcrud.retrieve(field.class, linkVal).toPromise()
-            .then(idlThing => {
-                field.linkedValues =
-                    this.flattenLinkedValues(field, Array(idlThing));
-            });
-        }
-
-        // No linked value applied, nothing to pre-fetch.
-        return Promise.resolve();
-    }
-
     // Returns a context object to be inserted into a custom
     // field template.
     customTemplateFieldContext(fieldDef: any): CustomFieldContext {
@@ -786,7 +691,11 @@ export class FmRecordEditorComponent
             return field.datatype;
         }
 
-        if (field.datatype === 'link' || field.linkedValues) {
+        if (field.datatype === 'link') {
+            return 'link';
+        }
+
+        if (field.linkedValues) {
             return 'list';
         }