Bug 25826 - Hiding biblionumber in the frameworks breaks links in result list
Summary: Hiding biblionumber in the frameworks breaks links in result list
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: 20.05
Hardware: All All
: P3 critical (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on: 24458
Blocks:
  Show dependency treegraph
 
Reported: 2020-06-19 20:02 UTC by Liz Rea
Modified: 2021-01-13 22:06 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
20.11.00,20.05.08


Attachments
Bug 25826: Add a warning to the about page (6.61 KB, patch)
2020-07-22 14:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25826: Add a warning to the about page (6.66 KB, patch)
2020-07-27 15:26 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25826: Forbid changing the hidden attributes for biblionumber (5.71 KB, patch)
2020-07-27 15:26 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25826: Add a warning to the about page (6.73 KB, patch)
2020-08-06 12:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25826: Forbid changing the hidden attributes for biblionumber (5.79 KB, patch)
2020-08-06 12:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25826: (QA follow-up) Add filters (1.20 KB, patch)
2020-08-06 12:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25826: fix failing test (2.10 KB, patch)
2020-08-10 08:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
[20.05.x] Bug 25826: Add a warning to the about page (6.22 KB, patch)
2021-01-05 12:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[20.05.x] Bug 25826: Forbid changing the hidden attributes for biblionumber (5.78 KB, patch)
2021-01-05 12:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[20.05.x] Bug 25826: (QA follow-up) Add filters (1.26 KB, patch)
2021-01-05 12:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[20.05.x] Bug 25826: fix failing test (2.17 KB, patch)
2021-01-05 12:54 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 Liz Rea 2020-06-19 20:02:11 UTC
So Bug 23309 does this: ALTER TABLE marc_subfield_structure CHANGE COLUMN hidden hidden TINYINT(1) DEFAULT 8 NOT NULL;

Which on SOME vintages of Koha that I have not yet quite ascertained, causes at least the default framework and apparently all of the frameworks added before a certain version, to have 404 errors on the result page. The links look like this from the opac and staff results: https://domain.com/cgi-bin/koha/opac-detail.pl?biblionumber=&query_desc=kw%2Cwrdl%3A%20washington

Note that there is no Biblionumber. We found that this was because now the 999c was set to hidden/hidden in the framework visibility.

The problem is easy enough to fix - set the visibility to opac/intranet/collapsed on the 999c in the framework editor, and all is restored.

Probably it would be better to avoid this problem altogether though?
Comment 1 Katrin Fischer 2020-06-22 21:27:46 UTC

*** This bug has been marked as a duplicate of bug 25686 ***
Comment 2 Katrin Fischer 2020-06-25 20:11:26 UTC
The other related bug is closed - reopening this one for a fix.
Comment 3 Tomás Cohen Arazi 2020-06-25 20:22:11 UTC
Taking this one. I will add a 'deny_list' for fields that cannot be hidden by configuration, probably just 999 for now.
Comment 4 Fridolin Somers 2020-06-26 07:34:15 UTC
Looks like its related to Bug 19261

> I will add a 'deny_list'
where ? in DB ?
Comment 5 Tomás Cohen Arazi 2020-06-30 18:08:39 UTC
(In reply to Fridolin SOMERS from comment #4)
> Looks like its related to Bug 19261
> 
> > I will add a 'deny_list'
> where ? in DB ?

On ViewPolicy.pm
Comment 6 Fridolin Somers 2020-07-03 14:09:37 UTC
(In reply to Tomás Cohen Arazi from comment #5)
> (In reply to Fridolin SOMERS from comment #4)
> > Looks like its related to Bug 19261
> > 
> > > I will add a 'deny_list'
> > where ? in DB ?
> 
> On ViewPolicy.pm

Oki super.
Add field+subfield of biblio.biblionumber.

Add field+subfield 999x used by Koha::Filter::MARC::EmbedItemsAvailability.

Maybe add field+subfield of items.itemnumber ?
Comment 7 Jonathan Druart 2020-07-22 14:13:27 UTC
I'd like to add a warning to the about page and/or the search_for_inconsistencies script, but I am not able to recreate this problem.
What are the different steps?

I edited the default framework, 999$c and unticked all the "visibility" checkboxes, restart_all and (even) rebuild zebra's index (is that needed?). But the opac detail page is still displaying nicely.
Comment 8 Jonathan Druart 2020-07-22 14:15:17 UTC
(In reply to Jonathan Druart from comment #7)
> I'd like to add a warning to the about page and/or the
> search_for_inconsistencies script, but I am not able to recreate this
> problem.
> What are the different steps?
> 
> I edited the default framework, 999$c and unticked all the "visibility"
> checkboxes, restart_all and (even) rebuild zebra's index (is that needed?).
> But the opac detail page is still displaying nicely.

Forget that, *the link from the result list is broken*!
Comment 9 Jonathan Druart 2020-07-22 14:44:51 UTC
Created attachment 107179 [details] [review]
Bug 25826: Add a warning to the about page

This patch adds a warning to the about page if the biblionumber field is
hidden at the OPAC or staff interface.

Test plan:
Hide the biblionumber to some framework
Confirm that there is a warning on the about page
Comment 10 David Cook 2020-07-23 04:08:09 UTC
Ok I just reproduced this in master on koha-testing-docker after hiding 999$c in Default and Books* frameworks, and then doing this staff interface search:

local-number:29 or local-number:30

It created search results with the following links:

http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=&searchid=scs_1595477248050

http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=&searchid=scs_1595477248050

As you can see, the biblionumber is missing.
Comment 11 David Cook 2020-07-23 04:16:09 UTC
Since Bug 19261 fixed the problem, I'm thinking we just rename this bug report to be about adding the warning?
Comment 12 Jonathan Druart 2020-07-27 08:01:06 UTC
Tomas, what about this one? And do you think the patch from bug 19261 is correct?
Comment 13 Tomás Cohen Arazi 2020-07-27 15:26:10 UTC
Created attachment 107437 [details] [review]
Bug 25826: Add a warning to the about page

This patch adds a warning to the about page if the biblionumber field is
hidden at the OPAC or staff interface.

Test plan:
Hide the biblionumber to some framework
Confirm that there is a warning on the about page

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Tomás Cohen Arazi 2020-07-27 15:26:19 UTC
Created attachment 107438 [details] [review]
Bug 25826: Forbid changing the hidden attributes for biblionumber

This patch tweaks the UI so it forbids changing the hidden values when
they are correct (i.e. when they are not hidden in OPAC and intranet).

To test:
1. Apply the first patch
2. Play with the hidden values of the subfield you have mapped to
   biblio.biblionumber in your picked framework. This is usually 999$c
   in MARC21, and 001 in UNIMARC.
=> SUCCESS: When you verify the about.pl, it mentions issues when you
hide in some of the interfaces
3. Apply this patch and reload everything
4. Repeat 2
=> SUCCESS: If your framework is 'ok', it prevents you from editing this
values, otherwise you can change them.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 David Cook 2020-07-28 23:19:05 UTC
Failing QA, as I don't think this patch will work for the reasons I've cited at https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19261#c22

Perhaps I'm wrong, but this patch doesn't take into account existing installs, imported frameworks, or manual SQL updates. 

I think that the patch on Bug 19261 is a more robust solution.
Comment 16 Katrin Fischer 2020-07-29 07:15:06 UTC
Wouldn't they work well together? Having a solution GUI and code wise does make sense to me. Otherwise you can configure something, that doesn't take any effect.
Comment 17 Katrin Fischer 2020-07-29 07:45:38 UTC
(In reply to Katrin Fischer from comment #16)
> Wouldn't they work well together? Having a solution GUI and code wise does
> make sense to me. Otherwise you can configure something, that doesn't take
> any effect.

I realize now I missed out on a bunch more of the conversation, I still believe tackling this from mulitple angles would be good.
Comment 18 David Cook 2020-07-31 01:48:28 UTC
(In reply to Katrin Fischer from comment #17)
> I realize now I missed out on a bunch more of the conversation, I still
> believe tackling this from mulitple angles would be good.

Agreed
Comment 19 Jonathan Druart 2020-08-06 09:55:27 UTC
(In reply to David Cook from comment #18)
> (In reply to Katrin Fischer from comment #17)
> > I realize now I missed out on a bunch more of the conversation, I still
> > believe tackling this from mulitple angles would be good.
> 
> Agreed

Back to Signed Off then
Comment 20 Nick Clemens 2020-08-06 12:41:24 UTC
Created attachment 107885 [details] [review]
Bug 25826: Add a warning to the about page

This patch adds a warning to the about page if the biblionumber field is
hidden at the OPAC or staff interface.

Test plan:
Hide the biblionumber to some framework
Confirm that there is a warning on the about page

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 21 Nick Clemens 2020-08-06 12:41:27 UTC
Created attachment 107886 [details] [review]
Bug 25826: Forbid changing the hidden attributes for biblionumber

This patch tweaks the UI so it forbids changing the hidden values when
they are correct (i.e. when they are not hidden in OPAC and intranet).

To test:
1. Apply the first patch
2. Play with the hidden values of the subfield you have mapped to
   biblio.biblionumber in your picked framework. This is usually 999$c
   in MARC21, and 001 in UNIMARC.
=> SUCCESS: When you verify the about.pl, it mentions issues when you
hide in some of the interfaces
3. Apply this patch and reload everything
4. Repeat 2
=> SUCCESS: If your framework is 'ok', it prevents you from editing this
values, otherwise you can change them.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 22 Nick Clemens 2020-08-06 12:41:30 UTC
Created attachment 107887 [details] [review]
Bug 25826: (QA follow-up) Add filters

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 23 Nick Clemens 2020-08-06 12:42:41 UTC
I don't think this one is in conflict with 19261 - it adds a wrning - even if we ultimately choose to display the biblionumber no matter what this just tells you there is a bad config
Comment 24 Jonathan Druart 2020-08-07 14:56:00 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 25 Jonathan Druart 2020-08-10 08:05:16 UTC
Created attachment 107982 [details] [review]
Bug 25826: fix failing test
Comment 26 Jonathan Druart 2020-08-10 10:32:38 UTC
(In reply to Jonathan Druart from comment #25)
> Created attachment 107982 [details] [review] [review]
> Bug 25826: fix failing test

Pushed to master for 20.11
Comment 27 Lucas Gass 2020-08-14 21:27:17 UTC
many parts of this do not apply clean to 20.05.x, no backport

If needed please rebase
Comment 28 Jonathan Druart 2020-11-06 09:32:16 UTC
This is needed in 20.05. Can we get a rebase please?
Comment 29 Tomás Cohen Arazi 2021-01-05 12:54:06 UTC
Created attachment 114868 [details] [review]
[20.05.x] Bug 25826: Add a warning to the about page

This patch adds a warning to the about page if the biblionumber field is
hidden at the OPAC or staff interface.

Test plan:
Hide the biblionumber to some framework
Confirm that there is a warning on the about page

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 30 Tomás Cohen Arazi 2021-01-05 12:54:10 UTC
Created attachment 114869 [details] [review]
[20.05.x] Bug 25826: Forbid changing the hidden attributes for biblionumber

This patch tweaks the UI so it forbids changing the hidden values when
they are correct (i.e. when they are not hidden in OPAC and intranet).

To test:
1. Apply the first patch
2. Play with the hidden values of the subfield you have mapped to
   biblio.biblionumber in your picked framework. This is usually 999$c
   in MARC21, and 001 in UNIMARC.
=> SUCCESS: When you verify the about.pl, it mentions issues when you
hide in some of the interfaces
3. Apply this patch and reload everything
4. Repeat 2
=> SUCCESS: If your framework is 'ok', it prevents you from editing this
values, otherwise you can change them.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 31 Tomás Cohen Arazi 2021-01-05 12:54:14 UTC
Created attachment 114870 [details] [review]
[20.05.x] Bug 25826: (QA follow-up) Add filters

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 32 Tomás Cohen Arazi 2021-01-05 12:54:18 UTC
Created attachment 114871 [details] [review]
[20.05.x] Bug 25826: fix failing test

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 33 Andrew Fuerste-Henry 2021-01-13 22:06:27 UTC
Pushed to 20.05.x for 20.05.08