Simple checkbox to exclude electronic resources from search results

[Kathy Lussier]

Our users have long wanted a simple option (checkbox) to exclude all electronic resources from a search result set.

Although we can limit by format when conducting catalog searches, this checkbox option would easily allow users to easily indicate they want any format except for electronic materials. This checkbox should be available on the advanced search screen as well as the search results screen where they can use it to further limit their results.

[Jake Litrell (jake-3)]

Initial bits at: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jlitrell/lp1519055_exclude_electronic_search

Testing at: http://mlnc3.mvlcstaff.org/eg/opac/advanced

Todo:
  - Sticky checkbox
  - Checkbox on search results bar
  - CVM creation for patch/upgrade
  - Make configurable?

[Jake Litrell (jake-3)]

Updated with remaining fixes.

[Jason Stephenson (jstephenson)]

It looks like an upgrade script/seed data for a new coded value map is missing.

The code above refers to a coded value map called electronic, but no seed data/upgrade creates that new coded value map.

Also, it looks like one could possibly replace electronic with some combination of search formats separated by commas.

[Dan Wells (dbw2)]

Took a quick look. Any reason we're adding a '<br class="clear-both" />' to the results header bar? That's a pretty big change to the layout, and doesn't seem particularly related to the bug at hand. Maybe it snuck (or, if you prefer, sneaked) in?

Overall this looks like it is on the right track. Thanks!

[Kathy Lussier (klussier)]

Hi Dan,

I had recommended that Jake move the checkboxes down to the next line, mainly because we're adding a fourth checkbox here, and the results bar was getting a bit cluttered with all of those options on the same line.

However, we can take it out if people think it's too big of a change to the layout.

[Kathy Lussier (klussier)]

Adding the needsreleasenotes tag as a reminder to myself to write the release notes.

[Dan Wells (dbw2)]

Kathy, thanks for the explanation.

I understand the concern, and there are no easy answers about how to approach this type of problem. Here are a couple general considerations I see for cases like this.

First, once you add a manual break point, things can start to get visually quirky at different screen sizes, giving you extra breaks and suboptimal use of the space (e.g. three lines where two would suffice). Second, these checkboxes are getting more and more specialized, making it ever less likely that "average library" is going to have them all displayed. The more we cater the display to a certain number of options, the more work it is to undo once you start removing them. In that sense, our current all-one-line-let-it-wrap approach is a generally better baseline to work from.

Again, I know we're never going to create a one-size-fits-all catalog unless we strip it down to the essentials, and that doing so would create a different set of problems. If we do feel all of these options are going to be enabled at most sites, it would (in my opinion) be visually more understandable to put them in a separate container, via a simple top border or a different background. I'll try to post a couple images if I get a few minutes.

Dan

[Dan Wells (dbw2)]

Okay, attaching a couple (very) quick Firebug mockups. *If* we decide we have enough options to justify a fixed second line, I'd rather from a visual comprehension standpoint that we do something like these. Either will make more sense once stuff starts to wrap, and of these, I'm partial to the first (using a complementary background color).

I'd be happy to create a patch for either option, and would also support making any layout change (break or new bar) a separate bug altogether.

[Kathy Lussier (klussier)]

Added commits for a release notes entry and to add reingest instructions to the end of the upgrade script, as was advised by bshum in IRC. Please check the reingest instructions to make sure I got that right.

Jake has also added the seed data and upgrade script and has removed the line break. We can address display issues in a separate bug.

Current working branch is at

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/kmlussier/lp1519055_exclude_electronic_search

[Jake Litrell (jake-3)]

Branch updated here, with Kathy's changes cherrypicked. I've added a basic javascript function for now which removes duplicate entries in the search box on the advanced form, which may be applied when other checkboxes (notably "Limit to Available Items") are checked/unchecked. It's not an ideal solution, but should work as enough of a stopgap until this code is refactored shortly.

Same links:
http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jlitrell/lp1519055_exclude_electronic_search

Testing at: http://mlnc3.mvlcstaff.org/eg/opac/advanced

[Jake Litrell (jake-3)]

Made the checkboxes configurable, and off by default. Updated the release notes.

[Ben Shum (bshum)]

Tested this branch on a clean master install and found the following error to occur in my logs when clicking on the "Exclude Electronic Resources" checkbox. No results show up in the search after what appears to be a timeout failure.

[2016-02-27 19:16:44] open-ils.storage [WARN:24194:Application.pm:595:14566182452430823] open-ils.storage.biblio.multiclass.staged.search_fts: DBD::Pg::st execute failed: ERROR: syntax error
LINE 32: AND mrv.vlist @@ '!()'
                                    ^

So something isn't happy here.

[Ben Shum (bshum)]

Oh and I think that we should turn on the config option by default while leaving a note in the release notes that sites have the choice to turn this off. For new features, it makes sense to me to ship with the options turned on if we intend for it to become part of the default configuration and better supported due to its visibility.

[Dan Wells (dbw2)]

Recognizing that we, as a community, have no real means or method to make these decisions, I'll just register my opinion that I (at this moment) think this option should remain off by default. It's a very narrow feature, and as Thomas mentioned in IRC, it could easily be seen as better handled by a local customization.

I don't mind seeing this included in a release, but I think the real value in this code is providing an example for someone wanting to do something similar. Including it, but leaving it off, serves this purpose without leading to an eventual glut of special purpose widgets.

I do think there is merit to the testing/support argument Ben raises. Ideally we might someday support two or three stock configurations (e.g. minimal, standard, and full), but until then, I think we muddle through with whatever we collectively deem "standard".

Just two more cents.

[Kathy Lussier (klussier)]

Just adding a note that I found that the seed data wasn't being loaded on a clean install. I made a change to the sql that loads the seed data, but there's still something wrong with the sql. I'm not getting all of the OUs to display in the library selector, and a search for 'concerto' is retrieving 0 results.

Anyway, the change I made to the sql is in the top commit at http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/kmlussier/lp1519055_exclude_electronic_search

Kathy

[Kathy Lussier (klussier)]

OK, I tracked down the other error in the sql to create the composite record attribute. The seed data looks good now. Slapping a pullrequest tag back on this one.

A search you can do against Concerto data to test this feature is: bibliographical

You'll retrieve 4 results, 3 of which are electronic resources. Click the exclude limiter, and you get 1 result.

I didn't change the default for whether this limiter should display. Typically, I would agree with Ben that we enable it so that people are aware that the new feature exists, but I also see how it can be used as an example. We also have libraries that have shown interest in a "include ONLY electronic resources" checkbox, and I know they were planning to use this code as the example to do so if they decided to move forward with it.

Current working branch is at:
http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/kmlussier/lp1519055_exclude_electronic_search

[Ben Shum (bshum)]

Some minor formatting tidbits for next time, Jake.

1) Try to keep the first line as a subject line, and keep a break between that and the next lines for larger paragraph description. For some interfaces, the extra break is important to avoid odd display interactions. So that's just my own personal pet peeve.

2) Try to add the LP# to every commit in the series, so that they remain linked together. This way, we know the work was all related to each other.

3) Remember to add your signed-off-by line to every commit you make, not just the ones that formally are part of new changes. This signifies the commit is to be shared/included.

Other than that, no big deal, I made the minor edits to the structure as best I could. Alternatively in the future, you can always squash fixups and changes into the same published commit to make it easier for reviewers to grab and apply your changeset. I know that developers differ on opinion and practice on this, but I find that when publishing work, it's a cleaner, more polished look to squash away unnecessary changesets.

Pushed to master for 2.10-beta.