eg-fm-record-editor: take care wiring up comboboxes

[Galen Charlton]

As alluded to in bug 1851879, eg-fm-record-editor can attempt to scan an entire linked table when creating a modal.

Specifically, if the IDL class has one or more linked tables as columns, wireUpCombobox() is invoked for each of them. If preloadLinkedValues is active, which is the default, the entire contents of the linked table are fetched. This happens even if customTemplate is in effect for the linked column and a combobox isn't desired in the first place.

In the case of bug 1851879, this can result in loading all bib buckets in the system when opening a carousel modal, which is a problem in systems with large numbers of buckets.

Possible fixes:

- Do not invoke wireUpCombobox for link columns if a custom template is in force for that column
- Do not have preloadLinkedValues be on by default

[Galen Charlton (gmc)]

As one correction, preloadLinkedValues is not on by default, so I modify my suggestion: that it either not be a component-level option (and just be a fieldOption) or that it not be turned on in the default admin-page component.

[Galen Charlton (gmc)]

Also, this bit in fm-editor.component.ts is suspicious:

        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) {
            return this.pcrud.retrieveAll(field.class, {}, {atomic : true})
            .toPromise().then(list => {
                field.linkedValues =
                    this.flattenLinkedValues(field, list);
            });
        }

In particular, it means that at the moment one cannot even use the preloadLinkedValues field option to prevent fetching all of cbreb in the carousels interface.

[Galen Charlton (gmc)]

And passing the isReadOnly field option suppresses the attempt to fetch the entire linked table, but also doesn't result in the linked value being fetched.

[Terran McCanna (tmccanna)]

FYI, when this was on our 3.4.1 test server I could open the carousel create/edit form if I let it sit for a minute or two, but now that it's on our production server I can't open it at all. The only way we can create or edit carousels in production right now is directly in the database.

[Steve Callender (stevecallender)]

Just to throw my 2 cents in, I verified this on a couple other 3.5 systems that have over 100000 existing bib buckets and you cannot edit the carousels no matter what you do, so this is a pretty high priority item.

[Terran McCanna (tmccanna)]

I agree with Steve. Bumping priority to High.

[Galen Charlton (gmc)]

A patch is available here:

user/gmcharlt/lp1851884-avoid-fetching-the-world / https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/gmcharlt/lp1851884-avoid-fetching-the-world

[Tiffany Little (tslittle)]

Galen mentioned in bug 1958186 that this could also affect the slow/no loading of Acq EDI messages. With the patch loaded on a copy of PINES production data, the EDI messages popped right up. I'm not going to sign off on this since I did not test with carousels, but adding the data point that I did test with the patch on a related issue and it did seem to fix that issue.

[Terran McCanna (tmccanna)]

I tested this on a PINES test server with our large data set and it solved the carousel loading problems - woo hoo!

[Tiffany Little (tslittle)]

Since Terran tested with carousels, I'm fine adding my signoff.

I have tested this code and consent to signing off on it with my name, Tiffany Little, and my email address, <email address hidden>.

[Galen Charlton (gmc)]

Pushed to master, rel_3_9, rel_3_8, and rel_3_7 for the 3.7.4 quasi-security release. Noting that the rel_3_7 version is a backport made by Jason Boyer.

Thanks, Terran and Tiffany!

[Jason Stephenson (jstephenson)]

I reverted the patch from rel_3_7. While git applies it to the code just fine without conflicts, the changes rely on code added in rel_3_8 that is not present in rel_3_7. This dependency is too extensive to be reconciled without bringing in more code from the future.

The following error occurs when building Angular on rel_3_7 with the patch applied:

    ERROR in src/app/share/fm-editor/fm-editor.component.ts:517:30 - error TS2339: Property 'linkedSearchConditions' does not exist on type 'FmFieldOptions'.

    517 if (fieldOptions.linkedSearchConditions) {
                                     ~~~~~~~~~~~~~~~~~~~~~~
    src/app/share/fm-editor/fm-editor.component.ts:518:51 - error TS2339: Property 'linkedSearchConditions' does not exist on type 'FmFieldOptions'.

    518 field.idlBaseQuery = fieldOptions.linkedSearchConditions;

Attempting to resolve it by removing some of the offending code leads to further errors.

Since rel_3_7 is EOL, reverting the patch seems to be the better option.