Bug 36557

Summary: Improve logic and display of OPAC cart, tag, and lists controls
Product: Koha Reporter: Owen Leonard <oleonard>
Component: OPACAssignee: Owen Leonard <oleonard>
Status: Needs documenting --- QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: normal    
Priority: P5 - low CC: david, dcook, fridolin.somers, jonathan.druart, lucas, testopia
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Small patch Documentation contact:
Documentation submission: Text to go in the release notes:
This fixes errors in the way features are displayed in OPAC result lists so that controls for holds, tags, lists, and so on, are shown or hidden according to system preferences. It converts the Cart/Lists dropdown to separate buttons, making the display logic simpler and making the interface more consistent with updates to the staff interface search results.
Version(s) released in:
24.11.00,24.05.06
Circulation function:
Bug Depends on: 36207, 36166, 37150    
Bug Blocks: 38620    
Attachments: Bug 36166: Convert Cart and Lists conrols to buttons and rework logic
Bug 36166: Convert Cart and Lists controls to buttons and rework logic
Bug 36557: Convert Cart and Lists controls to buttons and rework logic
Bug 36557: Convert Cart and Lists controls to buttons and rework logic
Bug 36557: (follow-up) Restore missing "Remove from list" control
Bug 36557: Convert Cart and Lists controls to buttons and rework logic
Bug 36557: (follow-up) Restore missing "Remove from list" control
Bug 36557: Convert Cart and Lists controls to buttons and rework logic
Bug 36557: (follow-up) Restore missing "Remove from list" control
Bug 36557: Convert Cart and Lists controls to buttons and rework logic
Bug 36557: (follow-up) Restore missing "Remove from list" control

Description Owen Leonard 2024-04-09 14:40:40 UTC
I would like to update the OPAC so that the Cart, Tag, and List controls on multi-record pages (search results, lists) have consistent controls and use consistent logic for displaying controls.
Comment 1 Owen Leonard 2024-04-09 14:45:11 UTC Comment hidden (obsolete)
Comment 2 David Nind 2024-05-04 20:49:51 UTC
Requires bug 32607, which currently no longer applies.
Comment 3 David Nind 2024-05-07 06:47:37 UTC
(In reply to David Nind from comment #2)
> Requires bug 32607, which currently no longer applies.

That should have been bug 36207! That bug was fixed.

However, now I'm getting a sha1/fake ancestor error for bug 36166.
Comment 4 David Nind 2024-05-07 07:19:32 UTC
(In reply to David Nind from comment #3)
 
> However, now I'm getting a sha1/fake ancestor error for bug 36166.

If I apply bug 36166 by itself, everything works as it should - no sha1/fake ancestor error.

However, I think I worked out what the problem is. The sha1/fake ancestor is for this bug - the patch itself for this bug has the wrong bug number!

- Bug title: Bug 36557 - Improve logic and display of OPAC cart, tag, and lists controls 
- Bug attached: Bug 36166: Convert Cart and Lists conrols to buttons and rework logic ()

Here is the sha1/fake ancestor error:

git bz apply 36557

Bug 36557 Depends on bug 36166 (Signed Off)
Follow? [(y)es, (n)o] y

Bug 36166 Depends on bug 36207 (Signed Off)
Follow? [(y)es, (n)o] y

Bug 36207 - Convert browser alerts to modals: OPAC tags

166212 - Bug 36207: Use confirmation modal when removing tags from titles in the OPAC

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 36207: Use confirmation modal when removing tags from titles in the OPAC

Bug 36166 - Disable select to add to list if opacuserlogin is disabled

162794 - Bug 36166: Disable select to add to list if opacuserlogin is disabled

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 36166: Disable select to add to list if opacuserlogin is disabled

Bug 36557 - Improve logic and display of OPAC cart, tag, and lists controls

164563 - Bug 36166: Convert Cart and Lists conrols to buttons and rework logic

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 36166: Convert Cart and Lists conrols to buttons and rework logic
error: sha1 information is lacking or useless (koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss).
error: could not build fake ancestor
Patch failed at 0001 Bug 36166: Convert Cart and Lists conrols to buttons and rework logic
Comment 5 Owen Leonard 2024-05-07 15:54:28 UTC
Created attachment 166297 [details] [review]
Bug 36166: Convert Cart and Lists controls to buttons and rework logic

    This patch tries to address errors in the way features are displayed in
    result lists so that controls for holds, tags, lists, etc. are shown or
    hidden according to system preferences.

    The patch converts the Cart/Lists dropdown to separate buttons, making
    the display logic simpler and making the interface more consistent with
    updates to the staff interface search results.

    To test, apply the patch and rebuild the OPAC CSS.

    - In the OPAC, test these pages:
      - Catalog search results
      - List contents
    - Test with various combinations of these system preferences:
      - opacbookbag
      - DisplayMultiPlaceHold
      - virtualshelves
      - OPACHoldRequests
      - TagsEnabled
      - TagsInputOnList
      - UseRecalls
      - ArticleRequests

    With each different combination of settings the right controls should
    appear in the toolbar at the top (if present), and with each search
    result.
Comment 6 David Nind 2024-05-08 09:07:46 UTC
Getting a really weird error now when trying to apply the patch 8-(...

git bz apply 36557

Bug 36557 Depends on bug 36166 (Signed Off)
Follow? [(y)es, (n)o] y

Bug 36166 Depends on bug 36207 (Signed Off)
Follow? [(y)es, (n)o] y

Bug 36207 - Convert browser alerts to modals: OPAC tags

166212 - Bug 36207: Use confirmation modal when removing tags from titles in the OPAC

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 36207: Use confirmation modal when removing tags from titles in the OPAC

Bug 36166 - Disable select to add to list if opacuserlogin is disabled

162794 - Bug 36166: Disable select to add to list if opacuserlogin is disabled

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 36166: Disable select to add to list if opacuserlogin is disabled

Bug 36557 - Improve logic and display of OPAC cart, tag, and lists controls

166297 - Bug 36166: Convert Cart and Lists controls to buttons and rework logic

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 36166: Convert Cart and Lists controls to buttons and rework logic
Moving to starting point
Adding bug reference   825098d Bug 36166: Convert Cart and Lists controls to buttons and rework logic
Cleaning up back to original state on error
Traceback (most recent call last):
  File "/usr/bin/git-bz", line 2834, in <module>
    applied = do_apply(bug_ref)
              ^^^^^^^^^^^^^^^^^
  File "/usr/bin/git-bz", line 1977, in do_apply
    add_url(bug, commits)
  File "/usr/bin/git-bz", line 1719, in add_url
    add_url_to_head_commit(commit, bug)
  File "/usr/bin/git-bz", line 1682, in add_url_to_head_commit
    git.commit(file="-", amend=True, _input=input)
  File "/usr/bin/git-bz", line 197, in f
    o = git_run(command, *args, **kwargs)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/bin/git-bz", line 167, in git_run
    output, error = process.communicate(input)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 1207, in communicate
    stdout, stderr = self._communicate(input, endtime, timeout)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 2039, in _communicate
    input_view = memoryview(self._input)
                 ^^^^^^^^^^^^^^^^^^^^^^^
TypeError: memoryview: a bytes-like object is required, not 'str'
Comment 7 Katrin Fischer 2024-05-08 09:19:40 UTC
It looks like a git bz issue, but not spotting what's actually going on. Adding Joubu.
Comment 8 Jonathan Druart 2024-05-08 12:40:27 UTC
Bug number on the patch is not 36557, and the git-bz python3 version is buggy...
Comment 9 Owen Leonard 2024-05-08 15:28:19 UTC
Created attachment 166422 [details] [review]
Bug 36557: Convert Cart and Lists controls to buttons and rework logic

This patch tries to address errors in the way features are displayed in
result lists so that controls for holds, tags, lists, etc. are shown or
hidden according to system preferences.

The patch converts the Cart/Lists dropdown to separate buttons, making
the display logic simpler and making the interface more consistent with
updates to the staff interface search results.

To test, apply the patch and rebuild the OPAC CSS.

- In the OPAC, test these pages:
  - Catalog search results
  - List contents
- Test with various combinations of these system preferences:
  - opacbookbag
  - DisplayMultiPlaceHold
  - virtualshelves
  - OPACHoldRequests
  - TagsEnabled
  - TagsInputOnList
  - UseRecalls
  - ArticleRequests

With each different combination of settings the right controls should
appear in the toolbar at the top (if present), and with each search
result.
Comment 10 David Nind 2024-05-08 23:36:58 UTC
Thanks for fixing! The patch now applies.

The only thing that wasn't working as expected for me was article requests.

With ArticleRequests enabled (as well as ArticleRequestsLinkControl set to 'Use algorithm to show or hide'):
1. Before the patch, 'Request article' was displayed in the actions for each result in the search results and on the record details page. It was also displayed on the list results.
2. After the patch, 'Request article' is not displayed in the search results, record details page, or lists.

Not sure whether this was intended, or if it has something to do with ArticleRequestsLinkControl. With that set to always show, 'Request article' appears for all search results and on lists, but not on the record details page.
Comment 11 Owen Leonard 2024-05-15 10:49:09 UTC
(In reply to David Nind from comment #10)

> 2. After the patch, 'Request article' is not displayed in the search
> results, record details page, or lists.

I'm not seeing this. In my testing the 'Request article' link shows up as expected on search results and lists with both settings of ArticleRequestsLinkControl.

My patch doesn't touch the record detail page, so if there's an issue with that it's separate.
Comment 12 David Nind 2024-05-15 23:29:25 UTC
(In reply to Owen Leonard from comment #11)
> (In reply to David Nind from comment #10)
> 
> > 2. After the patch, 'Request article' is not displayed in the search
> > results, record details page, or lists.
> 
> I'm not seeing this. In my testing the 'Request article' link shows up as
> expected on search results and lists with both settings of
> ArticleRequestsLinkControl.
> 
> My patch doesn't touch the record detail page, so if there's an issue with
> that it's separate.

Have re-tested and is now showing. My apologies Owen, there must have been something wrong with my setup....
Comment 13 David Nind 2024-05-15 23:29:59 UTC Comment hidden (obsolete)
Comment 14 Owen Leonard 2024-06-21 15:18:04 UTC Comment hidden (obsolete)
Comment 15 David Nind 2024-06-21 23:21:39 UTC
Created attachment 167997 [details] [review]
Bug 36557: Convert Cart and Lists controls to buttons and rework logic

This patch tries to address errors in the way features are displayed in
result lists so that controls for holds, tags, lists, etc. are shown or
hidden according to system preferences.

The patch converts the Cart/Lists dropdown to separate buttons, making
the display logic simpler and making the interface more consistent with
updates to the staff interface search results.

To test, apply the patch and rebuild the OPAC CSS.

- In the OPAC, test these pages:
  - Catalog search results
  - List contents
- Test with various combinations of these system preferences:
  - opacbookbag
  - DisplayMultiPlaceHold
  - virtualshelves
  - OPACHoldRequests
  - TagsEnabled
  - TagsInputOnList
  - UseRecalls
  - ArticleRequests

With each different combination of settings the right controls should
appear in the toolbar at the top (if present), and with each search
result.

Signed-off-by: David Nind <david@davidnind.com>
Comment 16 David Nind 2024-06-21 23:21:41 UTC
Created attachment 167998 [details] [review]
Bug 36557: (follow-up) Restore missing "Remove from list" control

This patch restores the "Remove from list" control to the batch
operations toolbar when one is looking at the contents of a list. It was
unintentionally left out of the previous patch.

The patch also corrects some button markup to ensure consisten style of
controls in the batch operations toolbar.

Signed-off-by: David Nind <david@davidnind.com>
Comment 17 Martin Renvoize (ashimema) 2024-06-28 13:56:35 UTC
Sorry Owen.. I'm getting the dreaded sha1 apply error here
Comment 18 Owen Leonard 2024-06-28 14:37:11 UTC
Created attachment 168248 [details] [review]
Bug 36557: Convert Cart and Lists controls to buttons and rework logic

This patch tries to address errors in the way features are displayed in
result lists so that controls for holds, tags, lists, etc. are shown or
hidden according to system preferences.

The patch converts the Cart/Lists dropdown to separate buttons, making
the display logic simpler and making the interface more consistent with
updates to the staff interface search results.

To test, apply the patch and rebuild the OPAC CSS.

- In the OPAC, test these pages:
  - Catalog search results
  - List contents
- Test with various combinations of these system preferences:
  - opacbookbag
  - DisplayMultiPlaceHold
  - virtualshelves
  - OPACHoldRequests
  - TagsEnabled
  - TagsInputOnList
  - UseRecalls
  - ArticleRequests

With each different combination of settings the right controls should
appear in the toolbar at the top (if present), and with each search
result.

Signed-off-by: David Nind <david@davidnind.com>
Comment 19 Owen Leonard 2024-06-28 14:37:14 UTC
Created attachment 168249 [details] [review]
Bug 36557: (follow-up) Restore missing "Remove from list" control

This patch restores the "Remove from list" control to the batch
operations toolbar when one is looking at the contents of a list. It was
unintentionally left out of the previous patch.

The patch also corrects some button markup to ensure consisten style of
controls in the batch operations toolbar.

Signed-off-by: David Nind <david@davidnind.com>
Comment 20 Martin Renvoize (ashimema) 2024-06-28 14:47:43 UTC
Created attachment 168251 [details] [review]
Bug 36557: Convert Cart and Lists controls to buttons and rework logic

This patch tries to address errors in the way features are displayed in
result lists so that controls for holds, tags, lists, etc. are shown or
hidden according to system preferences.

The patch converts the Cart/Lists dropdown to separate buttons, making
the display logic simpler and making the interface more consistent with
updates to the staff interface search results.

To test, apply the patch and rebuild the OPAC CSS.

- In the OPAC, test these pages:
  - Catalog search results
  - List contents
- Test with various combinations of these system preferences:
  - opacbookbag
  - DisplayMultiPlaceHold
  - virtualshelves
  - OPACHoldRequests
  - TagsEnabled
  - TagsInputOnList
  - UseRecalls
  - ArticleRequests

With each different combination of settings the right controls should
appear in the toolbar at the top (if present), and with each search
result.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 21 Martin Renvoize (ashimema) 2024-06-28 14:47:46 UTC
Created attachment 168252 [details] [review]
Bug 36557: (follow-up) Restore missing "Remove from list" control

This patch restores the "Remove from list" control to the batch
operations toolbar when one is looking at the contents of a list. It was
unintentionally left out of the previous patch.

The patch also corrects some button markup to ensure consisten style of
controls in the batch operations toolbar.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 22 Martin Renvoize (ashimema) 2024-06-28 14:48:22 UTC
Logic all checks out, looks nicer.

Thanks for the thorough testing David.

QA scripts all happy.

Passing QA
Comment 23 Katrin Fischer 2024-07-01 16:58:27 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 24 Lucas Gass (lukeg) 2024-11-18 17:16:48 UTC
Backported to 24.05.x for upcoming 24.05.06
Comment 25 Fridolin Somers 2024-12-02 10:37:41 UTC
Dependancies not in 23.11.x