Bug 29144 - Move branches.opac_info to AdditionalContents allowing multi language
Summary: Move branches.opac_info to AdditionalContents allowing multi language
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Marcel de Rooy
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 31313
Blocks: 35006 29190 31775 31842
  Show dependency treegraph
 
Reported: 2021-09-30 12:25 UTC by Marcel de Rooy
Modified: 2023-10-10 13:31 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/665
Text to go in the release notes:
This reports moves the contents of column branches.opac_info to a HTML block under Additional contents, identified by the new location OpacLibraryInfo. This allows translation of this block too.
Version(s) released in:
22.11.00


Attachments
Bug 29144: First sort the HTML options (1.68 KB, patch)
2022-08-01 14:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Add OpacBranchInfo as HTML option, show it on opac (3.40 KB, patch)
2022-08-01 14:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: (follow-up) Terminology: rename to OpacLibraryInfo (3.57 KB, patch)
2022-08-03 11:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Add $contents->find_best_match, $library->opac_info (7.44 KB, patch)
2022-08-03 11:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: First sort the HTML options (1.68 KB, patch)
2022-08-04 11:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Add OpacBranchInfo as HTML option, show it on opac (3.40 KB, patch)
2022-08-04 11:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: (follow-up) Terminology: rename to OpacLibraryInfo (3.57 KB, patch)
2022-08-04 11:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Add $contents->find_best_match, $library->opac_info (7.50 KB, patch)
2022-08-04 11:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Use ->opac_info in opac-library template (1.86 KB, patch)
2022-08-04 11:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Remove opac_info from GetItemsInfo (2.25 KB, patch)
2022-08-04 11:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Show html opac_info on opac-detail page (9.94 KB, patch)
2022-08-04 11:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Adjust libraries form (3.85 KB, patch)
2022-08-04 11:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Remove opac_info from a few tests (2.99 KB, patch)
2022-08-04 11:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Copy and remove branches.opac_info (dbrev) (3.86 KB, patch)
2022-08-04 11:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: (DBIx schema) Drop branches.opac_info (1.60 KB, patch)
2022-08-04 11:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: First sort the HTML options (1.69 KB, patch)
2022-08-16 13:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29144: Add OpacBranchInfo as HTML option, show it on opac (3.41 KB, patch)
2022-08-16 13:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29144: (follow-up) Terminology: rename to OpacLibraryInfo (3.58 KB, patch)
2022-08-16 13:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29144: Add $contents->find_best_match, $library->opac_info (7.51 KB, patch)
2022-08-16 13:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29144: Use ->opac_info in opac-library template (1.88 KB, patch)
2022-08-16 13:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29144: Remove opac_info from GetItemsInfo (2.27 KB, patch)
2022-08-16 13:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29144: Show html opac_info on opac-detail page (9.95 KB, patch)
2022-08-16 13:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29144: Adjust libraries form (3.86 KB, patch)
2022-08-16 13:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29144: Remove opac_info from a few tests (3.00 KB, patch)
2022-08-16 13:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29144: Copy and remove branches.opac_info (dbrev) (3.87 KB, patch)
2022-08-16 13:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29144: (DBIx schema) Drop branches.opac_info (1.61 KB, patch)
2022-08-16 13:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29144: First sort the HTML options (1.73 KB, patch)
2022-08-16 14:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Add OpacBranchInfo as HTML option, show it on opac (3.45 KB, patch)
2022-08-16 14:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: (follow-up) Terminology: rename to OpacLibraryInfo (3.62 KB, patch)
2022-08-16 14:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Add $contents->find_best_match, $library->opac_info (7.55 KB, patch)
2022-08-16 14:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Use ->opac_info in opac-library template (1.91 KB, patch)
2022-08-16 14:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Remove opac_info from GetItemsInfo (2.30 KB, patch)
2022-08-16 14:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Show html opac_info on opac-detail page (10.15 KB, patch)
2022-08-16 14:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Adjust libraries form (3.90 KB, patch)
2022-08-16 14:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Remove opac_info from a few tests (3.04 KB, patch)
2022-08-16 14:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Copy and remove branches.opac_info (dbrev) (3.91 KB, patch)
2022-08-16 14:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: (DBIx schema) Drop branches.opac_info (1.65 KB, patch)
2022-08-16 14:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: (follow-up) Fix rebase issue related to bug 31313 (1.52 KB, patch)
2022-08-16 14:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: First sort the HTML options (1.73 KB, patch)
2022-08-18 12:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29144: Add OpacBranchInfo as HTML option, show it on opac (3.45 KB, patch)
2022-08-18 12:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29144: (follow-up) Terminology: rename to OpacLibraryInfo (3.62 KB, patch)
2022-08-18 12:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29144: Add $contents->find_best_match, $library->opac_info (7.57 KB, patch)
2022-08-18 12:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29144: Use ->opac_info in opac-library template (1.94 KB, patch)
2022-08-18 12:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29144: Remove opac_info from GetItemsInfo (2.33 KB, patch)
2022-08-18 12:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29144: Show html opac_info on opac-detail page (10.18 KB, patch)
2022-08-18 12:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29144: Adjust libraries form (3.92 KB, patch)
2022-08-18 12:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29144: Remove opac_info from a few tests (3.07 KB, patch)
2022-08-18 12:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29144: Copy and remove branches.opac_info (dbrev) (3.93 KB, patch)
2022-08-18 12:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29144: (DBIx schema) Drop branches.opac_info (1.67 KB, patch)
2022-08-18 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29144: (follow-up) Fix rebase issue related to bug 31313 (1.54 KB, patch)
2022-08-18 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29144: First sort the HTML options (1.77 KB, patch)
2022-09-01 06:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Add OpacBranchInfo as HTML option, show it on opac (3.49 KB, patch)
2022-09-01 06:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: (follow-up) Terminology: rename to OpacLibraryInfo (3.66 KB, patch)
2022-09-01 06:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Add $contents->find_best_match, $library->opac_info (7.61 KB, patch)
2022-09-01 06:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Use ->opac_info in opac-library template (1.98 KB, patch)
2022-09-01 06:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Show html opac_info on opac-detail page (10.22 KB, patch)
2022-09-01 06:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Adjust libraries form (3.96 KB, patch)
2022-09-01 06:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Remove opac_info from a few tests (3.11 KB, patch)
2022-09-01 06:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: Copy and remove branches.opac_info (dbrev) (3.97 KB, patch)
2022-09-01 06:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: (DBIx schema) Drop branches.opac_info (1.71 KB, patch)
2022-09-01 06:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: (follow-up) Fix rebase issue related to bug 31313 (1.58 KB, patch)
2022-09-01 06:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29144: (QA follow-up) Make update idempotent (1.91 KB, patch)
2022-09-16 13:24 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2021-09-30 12:25:43 UTC
opac-library page shows opac_info

Would be nice to have in more than one language.
What would be the best way to achieve that?

[1] Move opac_info out of branches and add a table with branchcode, language and opac_info. Needs a separate admin page.

[2] css/js fiddling: All stuff in opac_info. Show/hide blocks etc.

[3] Your solution ?
Comment 1 Owen Leonard 2021-09-30 13:07:49 UTC
(In reply to Marcel de Rooy from comment #0)
> Would be nice to have in more than one language.
> What would be the best way to achieve that?
> 
> [3] Your solution ?

Add to "HTML customizations?"
Comment 2 Katrin Fischer 2022-02-19 23:44:51 UTC
> Add to "HTML customizations

My first thought too.
Comment 3 Marcel de Rooy 2022-08-01 14:17:52 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2022-08-01 14:17:55 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2022-08-01 14:18:33 UTC
Still work in progress
Comment 6 Marcel de Rooy 2022-08-03 11:37:23 UTC Comment hidden (obsolete)
Comment 7 Marcel de Rooy 2022-08-03 11:37:26 UTC Comment hidden (obsolete)
Comment 8 Marcel de Rooy 2022-08-04 11:58:08 UTC
Created attachment 138599 [details] [review]
Bug 29144: First sort the HTML options

Test plan:
Check result while adding a new HTML customization entry.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2022-08-04 11:58:12 UTC
Created attachment 138600 [details] [review]
Bug 29144: Add OpacBranchInfo as HTML option, show it on opac

Test plan:
Add a few OpacBranchInfo for libraries.
Verify that opac-library shows the correct info per library.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2022-08-04 11:58:16 UTC
Created attachment 138601 [details] [review]
Bug 29144: (follow-up) Terminology: rename to OpacLibraryInfo

git grep -l OpacBranchInfo | xargs sed -i -e's/OpacBranchInfo/OpacLibraryInfo/g'

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2022-08-04 11:58:19 UTC
Created attachment 138602 [details] [review]
Bug 29144: Add $contents->find_best_match, $library->opac_info

Test plan:
Run t/db_dependent/Koha/AdditionalContents.t
Run t/db_dependent/Koha/Library.t

Note: You may need to set ListOwnershipUponPatronDeletion to
delete to fix a pending issue in patron->delete with lists.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2022-08-04 11:58:23 UTC
Created attachment 138603 [details] [review]
Bug 29144: Use ->opac_info in opac-library template

The opac_info call uses find_best_match which does not result in
the All libaries plus specific library response of
AdditionalContents.get.

Test plan:
Revisit individual library page on OPAC.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2022-08-04 11:58:26 UTC
Created attachment 138604 [details] [review]
Bug 29144: Remove opac_info from GetItemsInfo

Test plan:
Run t/db_dependent/Items.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2022-08-04 11:58:30 UTC
Created attachment 138605 [details] [review]
Bug 29144: Show html opac_info on opac-detail page

Note: This patch disables the API call (which is not really needed),
and still needs attention in a following patch.

Test plan:
Add opac_info for two libraries and a branchurl for one of the two.
Select a biblio with an item having these two libraries as either
home or holding branch.
Toggle OpacLocationBranchToDisplay and check items table on opac
detail. Click on the library name.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2022-08-04 11:58:33 UTC
Created attachment 138606 [details] [review]
Bug 29144: Adjust libraries form

Test plan:
[1] Go to Administration / libraries.
[2] Check list view.
[3] Check view on individual library with/without opac info.
[4] Edit individual library with/without opac info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Marcel de Rooy 2022-08-04 11:58:36 UTC
Created attachment 138607 [details] [review]
Bug 29144: Remove opac_info from a few tests

Test plan:
Run t/db_dependent/Circulation/GetHardDueDate.t
Run t/db_dependent/Circulation/OfflineOperation.t
Run t/db_dependent/RotatingCollections.t
Run t/db_dependent/api/v1/libraries.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 17 Marcel de Rooy 2022-08-04 11:58:40 UTC
Created attachment 138608 [details] [review]
Bug 29144: Copy and remove branches.opac_info (dbrev)

Test plan:
Run dbrev.
Check api URL: /api/v1/public/libraries (with/without suffix /[branch_code].

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 18 Marcel de Rooy 2022-08-04 11:58:44 UTC
Created attachment 138609 [details] [review]
Bug 29144: (DBIx schema) Drop branches.opac_info

No test plan.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 19 Marcel de Rooy 2022-08-15 09:10:00 UTC
Martin, saw this comment on irc:

Note: You may need to set ListOwnershipUponPatronDeletion to delete to fix a pending issue in patron->delete with lists.

Is that a mistake comment in `Bug 29144: Add $contents->find_best_match, $library->opac_info`

No it wasnt. But in the meantime the fix for checking userenv on disown_or_delete has been pushed. So it is no longer relevant.
One of the tests included a patron delete and could fail on this issue.
Comment 20 Marcel de Rooy 2022-08-15 09:16:39 UTC
(In reply to Marcel de Rooy from comment #19)
> Martin, saw this comment on irc:
> 
> Note: You may need to set ListOwnershipUponPatronDeletion to delete to fix a
> pending issue in patron->delete with lists.
> 
> Is that a mistake comment in `Bug 29144: Add $contents->find_best_match,
> $library->opac_info`
> 
> No it wasnt. But in the meantime the fix for checking userenv on
> disown_or_delete has been pushed. So it is no longer relevant.
> One of the tests included a patron delete and could fail on this issue.

Bug 31288
Comment 21 Owen Leonard 2022-08-16 13:00:53 UTC
Created attachment 139182 [details] [review]
Bug 29144: First sort the HTML options

Test plan:
Check result while adding a new HTML customization entry.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 22 Owen Leonard 2022-08-16 13:00:58 UTC
Created attachment 139183 [details] [review]
Bug 29144: Add OpacBranchInfo as HTML option, show it on opac

Test plan:
Add a few OpacBranchInfo for libraries.
Verify that opac-library shows the correct info per library.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 23 Owen Leonard 2022-08-16 13:01:03 UTC
Created attachment 139184 [details] [review]
Bug 29144: (follow-up) Terminology: rename to OpacLibraryInfo

git grep -l OpacBranchInfo | xargs sed -i -e's/OpacBranchInfo/OpacLibraryInfo/g'

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 24 Owen Leonard 2022-08-16 13:01:07 UTC
Created attachment 139185 [details] [review]
Bug 29144: Add $contents->find_best_match, $library->opac_info

Test plan:
Run t/db_dependent/Koha/AdditionalContents.t
Run t/db_dependent/Koha/Library.t

Note: You may need to set ListOwnershipUponPatronDeletion to
delete to fix a pending issue in patron->delete with lists.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 25 Owen Leonard 2022-08-16 13:01:11 UTC
Created attachment 139186 [details] [review]
Bug 29144: Use ->opac_info in opac-library template

The opac_info call uses find_best_match which does not result in
the All libaries plus specific library response of
AdditionalContents.get.

Test plan:
Revisit individual library page on OPAC.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 26 Owen Leonard 2022-08-16 13:01:15 UTC
Created attachment 139187 [details] [review]
Bug 29144: Remove opac_info from GetItemsInfo

Test plan:
Run t/db_dependent/Items.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 27 Owen Leonard 2022-08-16 13:01:20 UTC
Created attachment 139188 [details] [review]
Bug 29144: Show html opac_info on opac-detail page

Note: This patch disables the API call (which is not really needed),
and still needs attention in a following patch.

Test plan:
Add opac_info for two libraries and a branchurl for one of the two.
Select a biblio with an item having these two libraries as either
home or holding branch.
Toggle OpacLocationBranchToDisplay and check items table on opac
detail. Click on the library name.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 28 Owen Leonard 2022-08-16 13:01:24 UTC
Created attachment 139189 [details] [review]
Bug 29144: Adjust libraries form

Test plan:
[1] Go to Administration / libraries.
[2] Check list view.
[3] Check view on individual library with/without opac info.
[4] Edit individual library with/without opac info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 29 Owen Leonard 2022-08-16 13:01:29 UTC
Created attachment 139190 [details] [review]
Bug 29144: Remove opac_info from a few tests

Test plan:
Run t/db_dependent/Circulation/GetHardDueDate.t
Run t/db_dependent/Circulation/OfflineOperation.t
Run t/db_dependent/RotatingCollections.t
Run t/db_dependent/api/v1/libraries.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 30 Owen Leonard 2022-08-16 13:01:33 UTC
Created attachment 139191 [details] [review]
Bug 29144: Copy and remove branches.opac_info (dbrev)

Test plan:
Run dbrev.
Check api URL: /api/v1/public/libraries (with/without suffix /[branch_code].

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 31 Owen Leonard 2022-08-16 13:01:37 UTC
Created attachment 139192 [details] [review]
Bug 29144: (DBIx schema) Drop branches.opac_info

No test plan.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 32 Marcel de Rooy 2022-08-16 14:21:36 UTC
Created attachment 139197 [details] [review]
Bug 29144: First sort the HTML options

Test plan:
Check result while adding a new HTML customization entry.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 33 Marcel de Rooy 2022-08-16 14:21:40 UTC
Created attachment 139198 [details] [review]
Bug 29144: Add OpacBranchInfo as HTML option, show it on opac

Test plan:
Add a few OpacBranchInfo for libraries.
Verify that opac-library shows the correct info per library.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 34 Marcel de Rooy 2022-08-16 14:21:44 UTC
Created attachment 139199 [details] [review]
Bug 29144: (follow-up) Terminology: rename to OpacLibraryInfo

git grep -l OpacBranchInfo | xargs sed -i -e's/OpacBranchInfo/OpacLibraryInfo/g'

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 35 Marcel de Rooy 2022-08-16 14:21:48 UTC
Created attachment 139200 [details] [review]
Bug 29144: Add $contents->find_best_match, $library->opac_info

Test plan:
Run t/db_dependent/Koha/AdditionalContents.t
Run t/db_dependent/Koha/Library.t

Note: You may need to set ListOwnershipUponPatronDeletion to
delete to fix a pending issue in patron->delete with lists.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 36 Marcel de Rooy 2022-08-16 14:21:52 UTC
Created attachment 139201 [details] [review]
Bug 29144: Use ->opac_info in opac-library template

The opac_info call uses find_best_match which does not result in
the All libaries plus specific library response of
AdditionalContents.get.

Test plan:
Revisit individual library page on OPAC.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 37 Marcel de Rooy 2022-08-16 14:21:56 UTC
Created attachment 139202 [details] [review]
Bug 29144: Remove opac_info from GetItemsInfo

Test plan:
Run t/db_dependent/Items.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 38 Marcel de Rooy 2022-08-16 14:22:00 UTC
Created attachment 139203 [details] [review]
Bug 29144: Show html opac_info on opac-detail page

Note: This patch disables the API call (which is not really needed),
and still needs attention in a following patch.

Test plan:
Add opac_info for two libraries and a branchurl for one of the two.
Select a biblio with an item having these two libraries as either
home or holding branch.
Toggle OpacLocationBranchToDisplay and check items table on opac
detail. Click on the library name.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 39 Marcel de Rooy 2022-08-16 14:22:05 UTC
Created attachment 139204 [details] [review]
Bug 29144: Adjust libraries form

Test plan:
[1] Go to Administration / libraries.
[2] Check list view.
[3] Check view on individual library with/without opac info.
[4] Edit individual library with/without opac info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 40 Marcel de Rooy 2022-08-16 14:22:09 UTC
Created attachment 139205 [details] [review]
Bug 29144: Remove opac_info from a few tests

Test plan:
Run t/db_dependent/Circulation/GetHardDueDate.t
Run t/db_dependent/Circulation/OfflineOperation.t
Run t/db_dependent/RotatingCollections.t
Run t/db_dependent/api/v1/libraries.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 41 Marcel de Rooy 2022-08-16 14:22:13 UTC
Created attachment 139206 [details] [review]
Bug 29144: Copy and remove branches.opac_info (dbrev)

Test plan:
Run dbrev.
Check api URL: /api/v1/public/libraries (with/without suffix /[branch_code].

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 42 Marcel de Rooy 2022-08-16 14:22:18 UTC
Created attachment 139207 [details] [review]
Bug 29144: (DBIx schema) Drop branches.opac_info

No test plan.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 43 Marcel de Rooy 2022-08-16 14:22:22 UTC
Created attachment 139208 [details] [review]
Bug 29144: (follow-up) Fix rebase issue related to bug 31313

Fixing display of holding branch name.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 44 Marcel de Rooy 2022-08-16 14:24:45 UTC
The GetItemsInfo created the need for a fast rebase:
Applying: Bug 29144: Show html opac_info on opac-detail page
CONFLICT (content): Merge conflict in opac/opac-detail.pl
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt

Related to 31313

It needed a tiny follow-up as well to fix a rebase issue.
Comment 45 Martin Renvoize 2022-08-18 12:54:18 UTC
Created attachment 139366 [details] [review]
Bug 29144: First sort the HTML options

Test plan:
Check result while adding a new HTML customization entry.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 46 Martin Renvoize 2022-08-18 12:54:22 UTC
Created attachment 139367 [details] [review]
Bug 29144: Add OpacBranchInfo as HTML option, show it on opac

Test plan:
Add a few OpacBranchInfo for libraries.
Verify that opac-library shows the correct info per library.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 47 Martin Renvoize 2022-08-18 12:54:26 UTC
Created attachment 139368 [details] [review]
Bug 29144: (follow-up) Terminology: rename to OpacLibraryInfo

git grep -l OpacBranchInfo | xargs sed -i -e's/OpacBranchInfo/OpacLibraryInfo/g'

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 48 Martin Renvoize 2022-08-18 12:54:31 UTC
Created attachment 139369 [details] [review]
Bug 29144: Add $contents->find_best_match, $library->opac_info

Test plan:
Run t/db_dependent/Koha/AdditionalContents.t
Run t/db_dependent/Koha/Library.t

Note: You may need to set ListOwnershipUponPatronDeletion to
delete to fix a pending issue in patron->delete with lists.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 49 Martin Renvoize 2022-08-18 12:54:35 UTC
Created attachment 139370 [details] [review]
Bug 29144: Use ->opac_info in opac-library template

The opac_info call uses find_best_match which does not result in
the All libaries plus specific library response of
AdditionalContents.get.

Test plan:
Revisit individual library page on OPAC.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 50 Martin Renvoize 2022-08-18 12:54:40 UTC
Created attachment 139371 [details] [review]
Bug 29144: Remove opac_info from GetItemsInfo

Test plan:
Run t/db_dependent/Items.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 51 Martin Renvoize 2022-08-18 12:54:44 UTC
Created attachment 139372 [details] [review]
Bug 29144: Show html opac_info on opac-detail page

Note: This patch disables the API call (which is not really needed),
and still needs attention in a following patch.

Test plan:
Add opac_info for two libraries and a branchurl for one of the two.
Select a biblio with an item having these two libraries as either
home or holding branch.
Toggle OpacLocationBranchToDisplay and check items table on opac
detail. Click on the library name.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 52 Martin Renvoize 2022-08-18 12:54:48 UTC
Created attachment 139373 [details] [review]
Bug 29144: Adjust libraries form

Test plan:
[1] Go to Administration / libraries.
[2] Check list view.
[3] Check view on individual library with/without opac info.
[4] Edit individual library with/without opac info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 53 Martin Renvoize 2022-08-18 12:54:52 UTC
Created attachment 139374 [details] [review]
Bug 29144: Remove opac_info from a few tests

Test plan:
Run t/db_dependent/Circulation/GetHardDueDate.t
Run t/db_dependent/Circulation/OfflineOperation.t
Run t/db_dependent/RotatingCollections.t
Run t/db_dependent/api/v1/libraries.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 54 Martin Renvoize 2022-08-18 12:54:56 UTC
Created attachment 139375 [details] [review]
Bug 29144: Copy and remove branches.opac_info (dbrev)

Test plan:
Run dbrev.
Check api URL: /api/v1/public/libraries (with/without suffix /[branch_code].

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 55 Martin Renvoize 2022-08-18 12:55:01 UTC
Created attachment 139376 [details] [review]
Bug 29144: (DBIx schema) Drop branches.opac_info

No test plan.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 56 Martin Renvoize 2022-08-18 12:55:06 UTC
Created attachment 139377 [details] [review]
Bug 29144: (follow-up) Fix rebase issue related to bug 31313

Fixing display of holding branch name.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 57 Martin Renvoize 2022-08-18 12:56:15 UTC
Great work, all working as expected, no regressions spotted and QA scripts are happy.

Nicely broken down, the commits were generally easy to follow.

Passing QA
Comment 58 Marcel de Rooy 2022-09-01 06:16:43 UTC
Created attachment 140020 [details] [review]
Bug 29144: First sort the HTML options

Test plan:
Check result while adding a new HTML customization entry.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 59 Marcel de Rooy 2022-09-01 06:16:48 UTC
Created attachment 140021 [details] [review]
Bug 29144: Add OpacBranchInfo as HTML option, show it on opac

Test plan:
Add a few OpacBranchInfo for libraries.
Verify that opac-library shows the correct info per library.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 60 Marcel de Rooy 2022-09-01 06:16:52 UTC
Created attachment 140022 [details] [review]
Bug 29144: (follow-up) Terminology: rename to OpacLibraryInfo

git grep -l OpacBranchInfo | xargs sed -i -e's/OpacBranchInfo/OpacLibraryInfo/g'

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 61 Marcel de Rooy 2022-09-01 06:16:57 UTC
Created attachment 140023 [details] [review]
Bug 29144: Add $contents->find_best_match, $library->opac_info

Test plan:
Run t/db_dependent/Koha/AdditionalContents.t
Run t/db_dependent/Koha/Library.t

Note: You may need to set ListOwnershipUponPatronDeletion to
delete to fix a pending issue in patron->delete with lists.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 62 Marcel de Rooy 2022-09-01 06:17:02 UTC
Created attachment 140024 [details] [review]
Bug 29144: Use ->opac_info in opac-library template

The opac_info call uses find_best_match which does not result in
the All libaries plus specific library response of
AdditionalContents.get.

Test plan:
Revisit individual library page on OPAC.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 63 Marcel de Rooy 2022-09-01 06:17:06 UTC
Created attachment 140025 [details] [review]
Bug 29144: Show html opac_info on opac-detail page

Note: This patch disables the API call (which is not really needed),
and still needs attention in a following patch.

Test plan:
Add opac_info for two libraries and a branchurl for one of the two.
Select a biblio with an item having these two libraries as either
home or holding branch.
Toggle OpacLocationBranchToDisplay and check items table on opac
detail. Click on the library name.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 64 Marcel de Rooy 2022-09-01 06:17:13 UTC
Created attachment 140026 [details] [review]
Bug 29144: Adjust libraries form

Test plan:
[1] Go to Administration / libraries.
[2] Check list view.
[3] Check view on individual library with/without opac info.
[4] Edit individual library with/without opac info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 65 Marcel de Rooy 2022-09-01 06:17:18 UTC
Created attachment 140027 [details] [review]
Bug 29144: Remove opac_info from a few tests

Test plan:
Run t/db_dependent/Circulation/GetHardDueDate.t
Run t/db_dependent/Circulation/OfflineOperation.t
Run t/db_dependent/RotatingCollections.t
Run t/db_dependent/api/v1/libraries.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 66 Marcel de Rooy 2022-09-01 06:17:23 UTC
Created attachment 140028 [details] [review]
Bug 29144: Copy and remove branches.opac_info (dbrev)

Test plan:
Run dbrev.
Check api URL: /api/v1/public/libraries (with/without suffix /[branch_code].

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 67 Marcel de Rooy 2022-09-01 06:17:27 UTC
Created attachment 140029 [details] [review]
Bug 29144: (DBIx schema) Drop branches.opac_info

No test plan.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 68 Marcel de Rooy 2022-09-01 06:17:31 UTC
Created attachment 140030 [details] [review]
Bug 29144: (follow-up) Fix rebase issue related to bug 31313

Fixing display of holding branch name.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 69 Marcel de Rooy 2022-09-01 06:21:02 UTC
Rebased for 27272
Comment 70 Tomás Cohen Arazi 2022-09-16 13:24:14 UTC
Created attachment 140707 [details] [review]
Bug 29144: (QA follow-up) Make update idempotent

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 71 Tomás Cohen Arazi 2022-09-16 13:34:25 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 72 Jonathan Druart 2022-10-14 13:22:26 UTC
Caught by Owen on IRC: DT search is broken (general search when searching for libraries).

Uncaught TypeError: col.data.split is not a function


It's because opac_info is searchable but "data" is a function:

    "data": function( row, type, val, meta ) {
        return ''; // provisional placeholder for opac_info
    },
    "searchable": true,
    "visible": false,
    "orderable": false

It could be considered a bug from datatables.js, but I don't think it is. We should fix branch.tt instead.
Comment 73 Marcel de Rooy 2022-10-17 09:24:15 UTC
(In reply to Jonathan Druart from comment #72)
> Caught by Owen on IRC: DT search is broken (general search when searching
> for libraries).
> 
> Uncaught TypeError: col.data.split is not a function
> 
> 
> It's because opac_info is searchable but "data" is a function:
> 
>     "data": function( row, type, val, meta ) {
>         return ''; // provisional placeholder for opac_info
>     },
>     "searchable": true,
>     "visible": false,
>     "orderable": false
> 
> It could be considered a bug from datatables.js, but I don't think it is. We
> should fix branch.tt instead.

Will probably open a new report for this.
Comment 74 Marcel de Rooy 2022-10-17 09:33:04 UTC
(In reply to Marcel de Rooy from comment #73)
> > It could be considered a bug from datatables.js, but I don't think it is. We
> > should fix branch.tt instead.
> 
> Will probably open a new report for this.

Bug 31842.
Comment 75 Marcel de Rooy 2022-10-17 09:39:29 UTC
(In reply to Marcel de Rooy from comment #73)
> (In reply to Jonathan Druart from comment #72)
> > Caught by Owen on IRC: DT search is broken (general search when searching
> > for libraries).
> > 
> > Uncaught TypeError: col.data.split is not a function
> > 
> > It's because opac_info is searchable but "data" is a function:
> > 
> >     "data": function( row, type, val, meta ) {
> >         return ''; // provisional placeholder for opac_info
> >     },
> >     "searchable": true,
> >     "visible": false,
> >     "orderable": false

I am seeing several columns where data is a function btw.

                        "data": function( row, type, val, meta ) {
                            const library_info = [];

                        "data": function( row, type, val, meta ) {
                            var result = '<a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/branches.pl?op=add_form&amp;branchcode='+encodeURIComponent(row.library_id)+'" role="button"><i class="fa fa-pencil" aria-hidden="true"></i> '+_("Edit")+'</a>';
Comment 76 Marcel de Rooy 2022-10-17 10:43:01 UTC
(In reply to Marcel de Rooy from comment #75)
> (In reply to Marcel de Rooy from comment #73)
> > (In reply to Jonathan Druart from comment #72)
> > > Caught by Owen on IRC: DT search is broken (general search when searching
> > > for libraries).
> > > 
> > > Uncaught TypeError: col.data.split is not a function
> > > 
> > > It's because opac_info is searchable but "data" is a function:
> > > 
> > >     "data": function( row, type, val, meta ) {
> > >         return ''; // provisional placeholder for opac_info
> > >     },
> > >     "searchable": true,
> > >     "visible": false,
> > >     "orderable": false
> 
> I am seeing several columns where data is a function btw.
> 
>                         "data": function( row, type, val, meta ) {
>                             const library_info = [];
> 
>                         "data": function( row, type, val, meta ) {
>                             var result = '<a class="btn btn-default btn-xs"
> href="/cgi-bin/koha/admin/branches.pl?op=add_form&amp;
> branchcode='+encodeURIComponent(row.library_id)+'" role="button"><i
> class="fa fa-pencil" aria-hidden="true"></i> '+_("Edit")+'</a>';

Addressed all three columns on bug 31842. Please test.
Comment 77 Lucas Gass 2022-10-31 21:35:09 UTC
Enhancement will not be backported to 22.05.x series