Carousel Enhancements

[Andrea Neiman]

This work is funded by PaILS.

This project takes the existing Carousels feature and modifies it to be a fully-integrated native feature of Evergreen.

The work will include:
 * Creating new Administrative interfaces to manage Carousels and Carousel Types
 * More advanced tools for controlling Carousel items and visibility, including improvements to Carousel refreshes and a new bucket type specifically to support Carousels
 * New tables in the database to support Carousels, Carousel Types, and Carousel visibility

Full specifications can be seen here: http://yeti.esilibrary.com/dev/public/techspecs/carousels.pdf

Once the feature has undergone partner testing, Equinox will post a public branch for community review and testing.

[Terran McCanna (tmccanna)]

Excellent! Looking forward to this!

[Blake GH (bmagic)]

Yay! Very exciting!

[Galen Charlton (gmc)]

A patch series is available at collab/gmcharlt/lp1832897_carousels. Please note that this branch includes general improvements to several Angular components as well as a new multi-select widget.

[Bill Erickson (berick)]

Thanks, Galen. I was able to configure a manual carousel and see it in the catalog.

New branch pushed:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1832897-carousel-integration

Includes sign-off's to the original commits and 2 minor repair patches.

Note I also set about tweaking how the comboboxes display the library shortname, to make it render via <eg-string /> for better i18n, but bumped into some component import order issues (resolved in bug #1840050) so I let it be for now and may revisit that later.

[Bill Erickson (berick)]

And the branch I pushed was rebased to master, so it includes some small merge repairs.

[Jane Sandberg (sandbergja)]

This is fun to play with! Two comments:

* I was unable to create a carousel from a bucket. Whenever I tried to do so, it failed silently, I got redirected to the carousel admin screen, and I got the following error in osrfsys.log:

open-ils.cstore ERROR inserting container::carousel object using query [INSERT INTO container.carousel (id,type,owner,name,bucket,creator,editor,create_time,edit_time,age_filter,owning_lib_filter,copy_location_filter,last_refresh_time,active,max_items) VALUES (DEFAULT,1,4,'My bucket',DEFAULT,1,1,DEFAULT,DEFAULT,DEFAULT,DEFAULT,DEFAULT,DEFAULT,DEFAULT,DEFAULT);]: 3505682 3505682: ERROR: null value in column "max_items" violates not-null constraint

* On a related note, the fm-editor allows users to attempt to create a carousel without setting the maximum number of items. This always fails without a descriptive error. I pushed a follow-up commit to Bill's branch to mark max_items (and a few others) as required in the IDL, which also makes them required in the UI. That can be found at user/sandbergja/lp1832897-carousel-integration

* The new multi-select is cool! It does have an accessibility issue, though. The comboboxes have no labels associated with them. I don't know what the best practice for labeling repeatable inputs is, but they should have *some* labels for sure. Note that a few other components have an Input called domId that allows the fmeditor component to associate form controls and their labels.

[Jane Sandberg (sandbergja)]

One last note: I think there is an inconsistency in how shelving locations are handled in the "Newly Catalogued Items" carousel type. The carousel type does have "Filter By Item Location" set to true, but the $new_items_query doesn't have a spot for LOC_LIST.

And if you add some Item Locations to a Newly Catalogued Item carousel and run refresh_carousels.srfsh, you get this error:

Received Exception:
Name: osrfMethodException
Status: *** Call to [open-ils.storage.carousel.refresh_all] failed for session [1567268440.381267.156726844014243], thread trace [1]:
DBD::Pg::st execute failed: called with 15 bind variables when 14 are needed [for Statement "
    WITH c_attr AS (SELECT c_attrs::query_int AS vis_test FROM asset.patron_default_visibility_mask() x)
    SELECT acn.record AS bib
    FROM asset.call_number acn
    JOIN asset.copy acp ON (acp.call_number = acn.id)
    JOIN asset.copy_location acpl ON (acp.location = acpl.id)
    JOIN config.copy_status ccs ON (acp.status = ccs.id)
    , c_attr
    WHERE acn.owning_lib IN (?,?,?,?,?,?)
    AND acp.circ_lib IN (?,?,?,?,?,?)
    AND acp.holdable
    AND acp.circulate
    AND ccs.holdable
    AND acpl.holdable
    AND acpl.circulate
    AND acp.active_date > NOW() - ?::INTERVAL
    AND (EXISTS (SELECT 1 FROM asset.copy_vis_attr_cache WHERE record = acn.record AND vis_attr_vector @@ c_attr.vis_test))
    AND (NOT EXISTS (SELECT 1 FROM metabib.record_attr_vector_list WHERE source = acn.record AND vlist @@ metabib.compile_composite_attr(' {"1":[{"_val":"s","_attr":"bib_level"}]}')::query_int))
    GROUP BY acn.record
    ORDER BY MIN(AGE(acp.active_date))
    LIMIT ?
" with ParamValues: 1=undef, 2=undef, 3=undef, 4=undef, 5=undef, 6=undef, 7=undef, 8=undef, 9=undef, 10=undef, 11=undef, 12=undef, 13=undef, 14=undef] at /usr/share/perl5/DBIx/ContextualFetch.pm line 52.

Status: 500

[Jane Sandberg (sandbergja)]

One last question: is there a way to limit automatically generated carousels to records that have cover images available? I ran a quick "recently circed" carousel on the concerto data set, and the carousel looks a little empty with so few cover images. :-(

[Mike Rylander (mrylander)]

Thanks, Jane!

On your last point, I can't think of a good way to do it since it's just using the normal added-content link generation.

I've added a commit to address the shelving location inconsistency (and the error that lead to), a separate commit to make sure max_items is set on create-from-bucket, and signed off your and Bill's recent commits. Branch is at working/user/miker/lp1832897-carousel-integration-fix-signoff

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/miker/lp1832897-carousel-integration-fix-signoff

[Jane Sandberg (sandbergja)]

Thanks, Galen, Bill, and Mike (and PaILS!). I've pushed this to master, along with one small follow-up commit.

So many libraries (including ours) will be happy to have this.