Bug 38330 - Make bib-level suppression a biblio table field instead of part of a MARC tag
Summary: Make bib-level suppression a biblio table field instead of part of a MARC tag
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: David Cook
URL:
Keywords: rel_25_11_candidate
Depends on:
Blocks: 37713 39336 39337
  Show dependency treegraph
 
Reported: 2024-11-01 20:08 UTC by Dani Elder
Modified: 2025-06-24 21:41 UTC (History)
8 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 38330: DB changes (4.35 KB, patch)
2024-12-18 13:14 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38330: DBIC updates (3.17 KB, patch)
2024-12-18 13:14 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38330: Map to biblio.opac_suppressed on saving record (1.75 KB, patch)
2024-12-18 13:14 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38330: Make sure biblio.opac_suppressed is populated/updated on save (4.44 KB, patch)
2024-12-18 13:14 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38330: DB changes (4.40 KB, patch)
2024-12-19 14:49 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 38330: DBIC updates (3.23 KB, patch)
2024-12-19 14:49 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 38330: Map to biblio.opac_suppressed on saving record (1.81 KB, patch)
2024-12-19 14:49 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 38330: Make sure biblio.opac_suppressed is populated/updated on save (4.50 KB, patch)
2024-12-19 14:49 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 38330: DB changes (5.17 KB, patch)
2025-03-14 15:54 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: DBIC updates (3.45 KB, patch)
2025-03-14 15:54 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: Map to biblio.opac_suppressed on saving record (1.39 KB, patch)
2025-03-14 15:54 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: Make sure biblio.opac_suppressed is populated/updated on save (4.57 KB, patch)
2025-03-14 15:54 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Remove use of metadata_extractor (1.20 KB, patch)
2025-03-14 16:11 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Remove use of metadata_extractor (3.73 KB, patch)
2025-03-14 16:16 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Add population of opac_suppressed from biblio_metadata (3.06 KB, patch)
2025-03-14 16:31 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: DB changes (5.17 KB, patch)
2025-05-08 16:38 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: DBIC updates (2.83 KB, patch)
2025-05-08 16:38 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: Map to biblio.opac_suppressed on saving record (1.39 KB, patch)
2025-05-08 16:38 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: Make sure biblio.opac_suppressed is populated/updated on save (4.57 KB, patch)
2025-05-08 16:38 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Remove use of metadata_extractor (3.73 KB, patch)
2025-05-08 16:38 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Add population of opac_suppressed from biblio_metadata (3.06 KB, patch)
2025-05-08 16:38 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Correction to field extaction (2.49 KB, patch)
2025-05-08 16:39 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Improve resilience of DB update (2.93 KB, patch)
2025-05-08 16:39 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Add index to opac_suppressed (1.97 KB, patch)
2025-05-08 16:39 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38330: DB changes (5.22 KB, patch)
2025-05-09 03:18 UTC, David Cook
Details | Diff | Splinter Review
Bug 38330: DBIC updates (2.88 KB, patch)
2025-05-09 03:18 UTC, David Cook
Details | Diff | Splinter Review
Bug 38330: Map to biblio.opac_suppressed on saving record (1.44 KB, patch)
2025-05-09 03:18 UTC, David Cook
Details | Diff | Splinter Review
Bug 38330: Make sure biblio.opac_suppressed is populated/updated on save (4.62 KB, patch)
2025-05-09 03:18 UTC, David Cook
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Remove use of metadata_extractor (3.77 KB, patch)
2025-05-09 03:18 UTC, David Cook
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Add population of opac_suppressed from biblio_metadata (3.11 KB, patch)
2025-05-09 03:18 UTC, David Cook
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Correction to field extaction (2.54 KB, patch)
2025-05-09 03:18 UTC, David Cook
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Improve resilience of DB update (2.87 KB, patch)
2025-05-09 03:18 UTC, David Cook
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Add index to opac_suppressed (2.02 KB, patch)
2025-05-09 03:18 UTC, David Cook
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Fix SQL syntax error (1.14 KB, patch)
2025-05-09 03:18 UTC, David Cook
Details | Diff | Splinter Review
Bug 38330: (QA follow-up) Fix WHERE clause in SQL update and re-word warning (1.90 KB, patch)
2025-05-09 03:19 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Dani Elder 2024-11-01 20:08:14 UTC
Suppression should be a biblio table field instead of in a marc field. Running reports with it to extract that field each time is not optimal.
Comment 1 Angela Berrett 2024-11-07 17:45:46 UTC
+1 I like this idea.
Comment 2 Tomás Cohen Arazi (tcohen) 2024-12-18 13:14:14 UTC
Created attachment 175648 [details] [review]
Bug 38330: DB changes
Comment 3 Tomás Cohen Arazi (tcohen) 2024-12-18 13:14:17 UTC
Created attachment 175649 [details] [review]
Bug 38330: DBIC updates
Comment 4 Tomás Cohen Arazi (tcohen) 2024-12-18 13:14:19 UTC
Created attachment 175650 [details] [review]
Bug 38330: Map to biblio.opac_suppressed on saving record
Comment 5 Tomás Cohen Arazi (tcohen) 2024-12-18 13:14:22 UTC
Created attachment 175651 [details] [review]
Bug 38330: Make sure biblio.opac_suppressed is populated/updated on save

This patchset adds a new column to biblio/deletedbiblio for storing the
information about OPAC suppression for the record.

It works as any other framework-mapped attribute. It defaults to 0 (not
suppressed) and some sanitization is done on storing, because the DB
should only contain 0 or 1.

To test:
0. Have a couple records with 942$n set to suppress them.
1. Apply this patches
2. Run:
   $ ktd --shell
  k$ updatedatabase
=> SUCCESS: All good
3. Check the `opac_suppressed` column was added to both tables
4. Notice it tells you need to run `touch_all_biblios.pl`
5. Run:
  k$ koha-mysql kohadev
   > SELECT opac_suppressed,COUNT(opac_suppressed) FROM biblio GROUP BY opac_suppressed;
=> FAIL: There are no records with the opac_suppressed flag set to 1
6. Run:
  k$ perl misc/maintenance/touch_all_biblios.pl -v
=> SUCCESS: No failures
7. Repeat 5
=> SUCCESS: Your couple records have the flag set. This means ModBiblio
is doing the right thing
8. Play with adding new records (suppressed and not)
=> SUCCESS: Suppression status is set correctly
9. Change the status to select records
=> SUCCESS: The DB column and the MARC are in sync!
10. Sign off :-D
Comment 6 Tomás Cohen Arazi (tcohen) 2024-12-18 13:14:50 UTC
My two cents towards all this.
Comment 7 Andrew Fuerste-Henry 2024-12-19 14:49:40 UTC Comment hidden (obsolete)
Comment 8 Andrew Fuerste-Henry 2024-12-19 14:49:42 UTC Comment hidden (obsolete)
Comment 9 Andrew Fuerste-Henry 2024-12-19 14:49:45 UTC Comment hidden (obsolete)
Comment 10 Andrew Fuerste-Henry 2024-12-19 14:49:47 UTC Comment hidden (obsolete)
Comment 11 Martin Renvoize (ashimema) 2025-03-14 15:54:30 UTC Comment hidden (obsolete)
Comment 12 Martin Renvoize (ashimema) 2025-03-14 15:54:33 UTC Comment hidden (obsolete)
Comment 13 Martin Renvoize (ashimema) 2025-03-14 15:54:36 UTC Comment hidden (obsolete)
Comment 14 Martin Renvoize (ashimema) 2025-03-14 15:54:39 UTC Comment hidden (obsolete)
Comment 15 Martin Renvoize (ashimema) 2025-03-14 15:55:21 UTC
This is a great move in a positive direction.. it highlights a few API oversights though.. which I think should be corrected in a follow-up bug.

Passing QA as it is.
Comment 16 Martin Renvoize (ashimema) 2025-03-14 16:11:25 UTC Comment hidden (obsolete)
Comment 17 Martin Renvoize (ashimema) 2025-03-14 16:16:05 UTC
Created attachment 179340 [details] [review]
Bug 38330: (QA follow-up) Remove use of metadata_extractor

We had a local method for getting opac_suppressed in Koha::Biblio which
relied on extracting from the MARC record.  Now that we have an
opac_suppressed field in the Biblio table, we no longer need this
wrapper method.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize (ashimema) 2025-03-14 16:31:32 UTC
Created attachment 179341 [details] [review]
Bug 38330: (QA follow-up) Add population of opac_suppressed from biblio_metadata

This patch updates the database update to populate the new
opac_suppressed field in the bilios table using the data in the
biblio_metadata table.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 19 Martin Renvoize (ashimema) 2025-03-14 16:32:12 UTC
Wouldn't be a bad idea for someone to have a final look at those follow-ups.
Comment 20 David Cook 2025-03-16 22:50:18 UTC
This is super exciting to see, but we've got a little problem in the QA follow-up, as it's using 952$n instead of 942$n. Marking Failed QA but hopefully will be in this status only for a short time.

I'll try to take a look at this again shortly (once I've gotten through the rest of my emails...)
Comment 21 David Cook 2025-03-16 22:53:39 UTC
(In reply to Martin Renvoize (ashimema) from comment #19)
> Wouldn't be a bad idea for someone to have a final look at those follow-ups.

I'll fix up the 952$n to 942$n... and I might do a different UPDATE since historically any true 942$n value was valid to trigger suppression. I'll have a think about that one.
Comment 22 David Cook 2025-03-16 23:29:09 UTC
(In reply to David Cook from comment #21)
> (In reply to Martin Renvoize (ashimema) from comment #19)
> > Wouldn't be a bad idea for someone to have a final look at those follow-ups.
> 
> I'll fix up the 952$n to 942$n... and I might do a different UPDATE since
> historically any true 942$n value was valid to trigger suppression. I'll
> have a think about that one.

I'm thinking about using boolean() in the Xpath.

The following returns 1 for any true value and should return 0 for any false value, so it'll return 1 for new value of 1 or old values like "YES" or "yes" or whatever. 0 will be returned for 0 or empty or non-existent.

IF(ExtractValue(metadata,'//datafield[@tag="942"]/subfield[@code="n" and boolean(text())]/text()'),1,0)
Comment 23 David Cook 2025-03-16 23:45:59 UTC
Keeping in mind bug 39337 it might be worthwhile adding an index on the opac_suppressed column.

That said, it might not be used by the query planner. I suppose that's something that we could explore later.
Comment 24 Martin Renvoize (ashimema) 2025-05-08 16:38:43 UTC
Created attachment 182094 [details] [review]
Bug 38330: DB changes

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 25 Martin Renvoize (ashimema) 2025-05-08 16:38:46 UTC
Created attachment 182095 [details] [review]
Bug 38330: DBIC updates

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize (ashimema) 2025-05-08 16:38:49 UTC
Created attachment 182096 [details] [review]
Bug 38330: Map to biblio.opac_suppressed on saving record

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 27 Martin Renvoize (ashimema) 2025-05-08 16:38:52 UTC
Created attachment 182097 [details] [review]
Bug 38330: Make sure biblio.opac_suppressed is populated/updated on save

This patchset adds a new column to biblio/deletedbiblio for storing the
information about OPAC suppression for the record.

It works as any other framework-mapped attribute. It defaults to 0 (not
suppressed) and some sanitization is done on storing, because the DB
should only contain 0 or 1.

To test:
0. Have a couple records with 942$n set to suppress them.
1. Apply this patches
2. Run:
   $ ktd --shell
  k$ updatedatabase
=> SUCCESS: All good
3. Check the `opac_suppressed` column was added to both tables
4. Notice it tells you need to run `touch_all_biblios.pl`
5. Run:
  k$ koha-mysql kohadev
   > SELECT opac_suppressed,COUNT(opac_suppressed) FROM biblio GROUP BY opac_suppressed;
=> FAIL: There are no records with the opac_suppressed flag set to 1
6. Run:
  k$ perl misc/maintenance/touch_all_biblios.pl -v
=> SUCCESS: No failures
7. Repeat 5
=> SUCCESS: Your couple records have the flag set. This means ModBiblio
is doing the right thing
8. Play with adding new records (suppressed and not)
=> SUCCESS: Suppression status is set correctly
9. Change the status to select records
=> SUCCESS: The DB column and the MARC are in sync!
10. Sign off :-D

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 28 Martin Renvoize (ashimema) 2025-05-08 16:38:54 UTC
Created attachment 182098 [details] [review]
Bug 38330: (QA follow-up) Remove use of metadata_extractor

We had a local method for getting opac_suppressed in Koha::Biblio which
relied on extracting from the MARC record.  Now that we have an
opac_suppressed field in the Biblio table, we no longer need this
wrapper method.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 29 Martin Renvoize (ashimema) 2025-05-08 16:38:57 UTC
Created attachment 182099 [details] [review]
Bug 38330: (QA follow-up) Add population of opac_suppressed from biblio_metadata

This patch updates the database update to populate the new
opac_suppressed field in the bilios table using the data in the
biblio_metadata table.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 30 Martin Renvoize (ashimema) 2025-05-08 16:39:00 UTC
Created attachment 182100 [details] [review]
Bug 38330: (QA follow-up) Correction to field extaction

As pointed out by David.. it's the 942 not the 952 we have this data in.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 31 Martin Renvoize (ashimema) 2025-05-08 16:39:03 UTC
Created attachment 182101 [details] [review]
Bug 38330: (QA follow-up) Improve resilience of DB update

David considered using boolean, but this would catch 'no' and 0 as
truthy.  I instead look for common values here more explicitly.
Comment 32 Martin Renvoize (ashimema) 2025-05-08 16:39:07 UTC
Created attachment 182102 [details] [review]
Bug 38330: (QA follow-up) Add index to opac_suppressed

As suggested by David, this one adds an index on the new column
Comment 33 Martin Renvoize (ashimema) 2025-05-08 16:40:06 UTC
Any chance of a final QA run on this David
Comment 34 David Cook 2025-05-09 01:04:07 UTC
(In reply to Martin Renvoize (ashimema) from comment #33)
> Any chance of a final QA run on this David

Sure thing, mon ami.
Comment 35 David Cook 2025-05-09 02:12:19 UTC
The /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_38330.pl has a syntax error in the SQL because of the ADD INDEX but I'll add a follow-up for that.
Comment 36 David Cook 2025-05-09 02:26:32 UTC
The test plan is missing a "koha-plack --restart kohadev", so the test actually uses the pre-patch behaviour it seems.
Comment 37 David Cook 2025-05-09 02:34:46 UTC
So /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_38330.pl was bugging me...

Why would it try to SET b.opac_suppressed AND say "You need to run the `maintenance/touch_all_biblios.pl` script"?

And the reason is that the WHERE clause in the UPDATE SQL will never match. 

Currently it is this:
'%<datafield tag="942">%'

But it should be this:
'%<datafield tag="942" %'

There should be no reason to run maintenance/touch_all_biblios.pl, since we're just updating the database and not changing the indexing.
Comment 38 David Cook 2025-05-09 02:40:39 UTC
Funny enough... in ktd biblionumber 232 and 234 are already marked suppressed. The things we learn when all you need to find suppressed records is: select biblionumber,opac_suppressed from biblio where opac_suppressed <> 0
Comment 39 David Cook 2025-05-09 02:58:19 UTC
After applying the patch, and running running "reset_all", "opac_suppressed" is 0 for KTD test data (e.g. biblionumber 232 and 234), but it should be 1. I think this is a problem with the KTD tools though. They must be doing direct SQL inserts instead of using Koha's internal functions... (yeah see /koha-misc4dev/-/tree/main/data/sql/marc21/2412/after_26684). That means there will be some  buggy OpacSuppression behaviour in KTD for a transitional time. I've raised issue #88 in koha-miscdev to address this.

Fortunately, the MARC bibliographic frameworks are created correctly out of the box, which is good.
Comment 40 David Cook 2025-05-09 03:00:07 UTC
Worth noting that the "Suppressed in OPAC" in the search results still comes from the MARC rather than the DB.
Comment 41 David Cook 2025-05-09 03:05:54 UTC
(In reply to David Cook from comment #40)
> Worth noting that the "Suppressed in OPAC" in the search results still comes
> from the MARC rather than the DB.

Same with the detail page actually. In future, we should probably move that out of the XSLT and into its own spot. Not a dealbreaker at this point though...

There are references to 942$n in Koha/ILL/Backend/Standard.pm and Koha/Filter/MARC/ExpandCodedFields.pm but they both make sense and they're irrelevant for our work here.
Comment 42 David Cook 2025-05-09 03:10:50 UTC
(In reply to David Cook from comment #37)
> There should be no reason to run maintenance/touch_all_biblios.pl, since
> we're just updating the database and not changing the indexing.

Theoretically, this is true. However, practically/technically it is possible for there to be MARCXML in the database which isn't formed exactly as we expect. For instance, it could be '<datafield ind1="" ind2="" tag="942">'. From memory, I think that Koha will still work with that, and that would fail our WHERE clause test. It's totally an edge case... but it's technically possible.

So... I'm changing the warning to the following:
"The biblio.opac_suppressed column should be updated, but you can run the `maintenance/touch_all_biblios.pl` script to be certain"

That lets people know that the data should be updated, but that if they have issues... just run touch_all_biblios.pl.
Comment 43 David Cook 2025-05-09 03:18:29 UTC
Created attachment 182135 [details] [review]
Bug 38330: DB changes

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 44 David Cook 2025-05-09 03:18:33 UTC
Created attachment 182136 [details] [review]
Bug 38330: DBIC updates

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 45 David Cook 2025-05-09 03:18:36 UTC
Created attachment 182137 [details] [review]
Bug 38330: Map to biblio.opac_suppressed on saving record

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 46 David Cook 2025-05-09 03:18:39 UTC
Created attachment 182138 [details] [review]
Bug 38330: Make sure biblio.opac_suppressed is populated/updated on save

This patchset adds a new column to biblio/deletedbiblio for storing the
information about OPAC suppression for the record.

It works as any other framework-mapped attribute. It defaults to 0 (not
suppressed) and some sanitization is done on storing, because the DB
should only contain 0 or 1.

To test:
0. Have a couple records with 942$n set to suppress them.
1. Apply this patches
2. Run:
   $ ktd --shell
  k$ updatedatabase
=> SUCCESS: All good
3. Check the `opac_suppressed` column was added to both tables
4. Notice it tells you need to run `touch_all_biblios.pl`
5. Run:
  k$ koha-mysql kohadev
   > SELECT opac_suppressed,COUNT(opac_suppressed) FROM biblio GROUP BY opac_suppressed;
=> FAIL: There are no records with the opac_suppressed flag set to 1
6. Run:
  k$ perl misc/maintenance/touch_all_biblios.pl -v
=> SUCCESS: No failures
7. Repeat 5
=> SUCCESS: Your couple records have the flag set. This means ModBiblio
is doing the right thing
8. Play with adding new records (suppressed and not)
=> SUCCESS: Suppression status is set correctly
9. Change the status to select records
=> SUCCESS: The DB column and the MARC are in sync!
10. Sign off :-D

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 47 David Cook 2025-05-09 03:18:42 UTC
Created attachment 182139 [details] [review]
Bug 38330: (QA follow-up) Remove use of metadata_extractor

We had a local method for getting opac_suppressed in Koha::Biblio which
relied on extracting from the MARC record.  Now that we have an
opac_suppressed field in the Biblio table, we no longer need this
wrapper method.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 48 David Cook 2025-05-09 03:18:45 UTC
Created attachment 182140 [details] [review]
Bug 38330: (QA follow-up) Add population of opac_suppressed from biblio_metadata

This patch updates the database update to populate the new
opac_suppressed field in the bilios table using the data in the
biblio_metadata table.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 49 David Cook 2025-05-09 03:18:48 UTC
Created attachment 182141 [details] [review]
Bug 38330: (QA follow-up) Correction to field extaction

As pointed out by David.. it's the 942 not the 952 we have this data in.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 50 David Cook 2025-05-09 03:18:52 UTC
Created attachment 182142 [details] [review]
Bug 38330: (QA follow-up) Improve resilience of DB update

David considered using boolean, but this would catch 'no' and 0 as
truthy.  I instead look for common values here more explicitly.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 51 David Cook 2025-05-09 03:18:55 UTC
Created attachment 182143 [details] [review]
Bug 38330: (QA follow-up) Add index to opac_suppressed

As suggested by David, this one adds an index on the new column

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 52 David Cook 2025-05-09 03:18:58 UTC
Created attachment 182144 [details] [review]
Bug 38330: (QA follow-up) Fix SQL syntax error

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 53 David Cook 2025-05-09 03:19:02 UTC
Created attachment 182145 [details] [review]
Bug 38330: (QA follow-up) Fix WHERE clause in SQL update and re-word warning

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 54 Katrin Fischer 2025-05-09 07:05:47 UTC
I need to ponder this one. It feels like it might be too big and risky for 25.05 at this point in time (being after hard feature freeze).
Comment 55 Katrin Fischer 2025-05-15 06:19:53 UTC
(In reply to Katrin Fischer from comment #54)
> I need to ponder this one. It feels like it might be too big and risky for
> 25.05 at this point in time (being after hard feature freeze).

No-one has argued against this, so I am setting this rel_25_11_candidate. Let's get this in early (but you'll have to talk to Lucas ;))
Comment 56 David Cook 2025-05-15 23:21:49 UTC
(In reply to Katrin Fischer from comment #54)
> I need to ponder this one. It feels like it might be too big and risky for
> 25.05 at this point in time (being after hard feature freeze).

I meant to reply to this before. I think that's a good decision.
Comment 57 Lucas Gass (lukeg) 2025-06-24 21:41:37 UTC
[FAIL] installer/data/mysql/kohastructure.sql
   FAIL	  tinyint_has_boolean_flag
		The new column biblio.opac_suppressed is using TINYINT(1) as type but is not defined as boolean in the schema file (Koha/Schema/Result/Biblio.pm), see the SQL12 coding guideline
		The new column deletedbiblio.opac_suppressed is using TINYINT(1) as type but is not defined as boolean in the schema file (Koha/Schema/Result/Deletedbiblio.pm), see the SQL12 coding guideline