Bug 26268 - Remove items.paid for once and for all
Summary: Remove items.paid for once and for all
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 18501 19919 23463
Blocks: 34063
  Show dependency treegraph
 
Reported: 2020-08-21 10:20 UTC by Martin Renvoize
Modified: 2024-02-07 11:08 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
The `paid` field in the `items` table is removed with this patch to prevent accidental re-introduction of syncing code and overhead. The only place where the value is surfaced in the UI has been replaced with an on-demand calculated value.
Version(s) released in:
20.11.00


Attachments
Bug 26268: Remove items.paidfor from the database (3.38 KB, patch)
2020-08-21 10:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Remove final references to paidfor db field (1.80 KB, patch)
2020-08-21 10:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Remove marc framework mappings to items.paidfor (6.15 KB, patch)
2020-08-21 11:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26268: Remove items.paidfor from the database (3.37 KB, patch)
2020-08-21 11:51 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Remove final references to paidfor db field (1.80 KB, patch)
2020-08-21 11:51 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Remove marc framework mappings to items.paidfor (6.20 KB, patch)
2020-08-21 11:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Remove items.paidfor from the database (3.44 KB, patch)
2020-08-21 12:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26268: Remove final references to paidfor db field (1.87 KB, patch)
2020-08-21 12:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26268: Remove marc framework mappings to items.paidfor (6.27 KB, patch)
2020-08-21 12:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26268: Remove items.paidfor from the database (3.43 KB, patch)
2020-09-04 16:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Remove final references to paidfor db field (1.86 KB, patch)
2020-09-04 16:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Remove marc framework mappings to items.paidfor (6.26 KB, patch)
2020-09-04 16:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: (QA follow-up) Split DB update checks (1.24 KB, patch)
2020-09-04 16:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Remove items.paidfor from the database (3.43 KB, patch)
2020-09-11 11:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Remove final references to paidfor db field (1.86 KB, patch)
2020-09-11 11:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Remove marc framework mappings to items.paidfor (6.26 KB, patch)
2020-09-11 11:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: (QA follow-up) Split DB update checks (1.24 KB, patch)
2020-09-11 11:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: (QA follow-up) Conditionally remove (2.46 KB, patch)
2020-09-11 11:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Remove items.paidfor from the database (3.48 KB, patch)
2020-09-26 21:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26268: Remove final references to paidfor db field (1.94 KB, patch)
2020-09-26 21:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26268: (QA follow-up) Split DB update checks (1.30 KB, patch)
2020-09-26 21:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26268: (QA follow-up) Conditionally remove (2.52 KB, patch)
2020-09-26 21:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26268: Correct mapping for internal note to items.itemnotes_nonpublic (6.70 KB, patch)
2020-09-26 21:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26268: Remove invalid mappings from MARC frameworks when items.paidfor is removed (1.17 KB, patch)
2020-09-26 21:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26268: Remove items.paidfor from the database (3.54 KB, patch)
2020-09-28 09:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Remove final references to paidfor db field (1.99 KB, patch)
2020-09-28 09:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: (QA follow-up) Split DB update checks (1.36 KB, patch)
2020-09-28 09:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: (QA follow-up) Conditionally remove (2.58 KB, patch)
2020-09-28 09:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Correct mapping for internal note to items.itemnotes_nonpublic (6.73 KB, patch)
2020-09-28 09:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Remove invalid mappings from MARC frameworks when items.paidfor is removed (1.24 KB, patch)
2020-09-28 09:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26268: Fix deleteditems query in DB rev (912 bytes, patch)
2020-09-29 15:06 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2020-08-21 10:20:05 UTC
In bug 19919 we removed all references to setting, unsettling and referring to the paidfor field but we neglected to actually drop the field from the database (I think I intended to do this, but was asking for feedback on the approach before adding the actual db change.. then completely forgot when it went through SO/QA quickly).

In bug 23463 we inadvertently re-introduced some of the setting code during a rebase.

In bug 18501 we then moved that inadvertently re-introduced setter into Koha::Item->store.

We should kill it off once and for all!
Comment 1 Martin Renvoize 2020-08-21 10:29:53 UTC
Created attachment 108797 [details] [review]
Bug 26268: Remove items.paidfor from the database

In bug 19919 we removed all references to setting, unsettling and
referring to the paidfor field but we neglected to actually drop the
field from the database (I think I intended to do this, but was asking
for feedback on the approach before adding the actual db change.. then
completely forgot when it went through SO/QA quickly).

In bug 23463 we inadvertently re-introduced some of the setting code
during a rebase.

In bug 18501 we then moved that inadvertently re-introduced setter into
Koha::Item->store.

We should kill it off once and for all!
Comment 2 Martin Renvoize 2020-08-21 10:29:57 UTC
Created attachment 108799 [details] [review]
Bug 26268: Remove final references to paidfor db field
Comment 3 Kyle M Hall 2020-08-21 11:34:32 UTC
if( !column_exists( 'biblio', 'biblionumber' ) ) {
Comment 4 Kyle M Hall 2020-08-21 11:35:35 UTC
Created attachment 108807 [details] [review]
Bug 26268: Remove marc framework mappings to items.paidfor
Comment 5 Martin Renvoize 2020-08-21 11:51:56 UTC
Created attachment 108808 [details] [review]
Bug 26268: Remove items.paidfor from the database

In bug 19919 we removed all references to setting, unsettling and
referring to the paidfor field but we neglected to actually drop the
field from the database (I think I intended to do this, but was asking
for feedback on the approach before adding the actual db change.. then
completely forgot when it went through SO/QA quickly).

In bug 23463 we inadvertently re-introduced some of the setting code
during a rebase.

In bug 18501 we then moved that inadvertently re-introduced setter into
Koha::Item->store.

We should kill it off once and for all!
Comment 6 Martin Renvoize 2020-08-21 11:51:59 UTC
Created attachment 108809 [details] [review]
Bug 26268: Remove final references to paidfor db field
Comment 7 Martin Renvoize 2020-08-21 11:52:03 UTC
Created attachment 108810 [details] [review]
Bug 26268: Remove marc framework mappings to items.paidfor

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2020-08-21 11:52:26 UTC
(In reply to Kyle M Hall from comment #3)
> if( !column_exists( 'biblio', 'biblionumber' ) ) {

Doh!

Fixed now
Comment 9 Kyle M Hall 2020-08-21 12:26:43 UTC
Created attachment 108812 [details] [review]
Bug 26268: Remove items.paidfor from the database

In bug 19919 we removed all references to setting, unsettling and
referring to the paidfor field but we neglected to actually drop the
field from the database (I think I intended to do this, but was asking
for feedback on the approach before adding the actual db change.. then
completely forgot when it went through SO/QA quickly).

In bug 23463 we inadvertently re-introduced some of the setting code
during a rebase.

In bug 18501 we then moved that inadvertently re-introduced setter into
Koha::Item->store.

We should kill it off once and for all!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Kyle M Hall 2020-08-21 12:26:51 UTC
Created attachment 108813 [details] [review]
Bug 26268: Remove final references to paidfor db field

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Kyle M Hall 2020-08-21 12:26:54 UTC
Created attachment 108814 [details] [review]
Bug 26268: Remove marc framework mappings to items.paidfor

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Katrin Fischer 2020-08-22 10:46:14 UTC
I see that there is a lot of history here, but I am not quite following yet.

1) Before dropping the column: Did we move the information stored there in previous developments? 

If we didn't, we might just want to print a warning if there is any content to be found in the db and ask people to drop themselves. It has been done in the past with some of the columns in borrowers (ethnicity?) I think. If there is no data, we can drop it in the update.

2) Looking for "paidfor" I found some more references this patch is not taking care of:

catalogue/moredetail.pl:    # Add paidfor info
catalogue/moredetail.pl:                $item->{paidfor} = { patron => $patron, created_on => $payment_offset->created_on };

koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt:                [% IF ( ITEM_DAT.paidfor ) %]<li><span class="label">Paid for?:</span> [% INCLUDE 'patron-title.inc' patron=ITEM_DAT.paidfor.patron hide_patron_infos_if_needed=1 %] on [% ITEM_DAT.paidfor.created_on | $KohaDates %]</li>[% END %]
Comment 13 Martin Renvoize 2020-09-02 15:43:24 UTC
(In reply to Katrin Fischer from comment #12)
> I see that there is a lot of history here, but I am not quite following yet.
> 
> 1) Before dropping the column: Did we move the information stored there in
> previous developments?

Nope, we didn't drop the data.. we just started using the fields it used to be populated by inconsistently to construct the display on on the fly. The data in the field is no longer maintained and historically wasn't accurate so I think we should be OK to drop it regardless.

> 
> If we didn't, we might just want to print a warning if there is any content
> to be found in the db and ask people to drop themselves. It has been done in
> the past with some of the columns in borrowers (ethnicity?) I think. If
> there is no data, we can drop it in the update.
> 
> 2) Looking for "paidfor" I found some more references this patch is not
> taking care of:

As noted above, this is where we now construct the data on the fly. `$item->{paidfor} = { patron => $patron, created_on => $payment_offset->created_on }` is an assignment for the template rather than a getter or setter for the DB data.

> 
> catalogue/moredetail.pl:    # Add paidfor info
> catalogue/moredetail.pl:                $item->{paidfor} = { patron =>
> $patron, created_on => $payment_offset->created_on };
> 
> koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt:            
> [% IF ( ITEM_DAT.paidfor ) %]<li><span class="label">Paid for?:</span> [%
> INCLUDE 'patron-title.inc' patron=ITEM_DAT.paidfor.patron
> hide_patron_infos_if_needed=1 %] on [% ITEM_DAT.paidfor.created_on |
> $KohaDates %]</li>[% END %]
Comment 14 Julian Maurice 2020-09-04 07:25:19 UTC
Comment on attachment 108812 [details] [review]
Bug 26268: Remove items.paidfor from the database

Review of attachment 108812 [details] [review]:
-----------------------------------------------------------------

::: installer/data/mysql/atomicupdate/bug_26268.perl
@@ +1,4 @@
> +$DBversion = 'XXX'; # will be replaced by the RM
> +if( CheckVersion( $DBversion ) ) {
> +
> +    if( !column_exists( 'items', 'paidfor' ) ) {

It should be column_exists, not !column_exists.
And in my opinion we should also check for deleteditems.paidfor existence
Comment 15 Martin Renvoize 2020-09-04 16:07:03 UTC
Created attachment 109674 [details] [review]
Bug 26268: Remove items.paidfor from the database

In bug 19919 we removed all references to setting, unsettling and
referring to the paidfor field but we neglected to actually drop the
field from the database (I think I intended to do this, but was asking
for feedback on the approach before adding the actual db change.. then
completely forgot when it went through SO/QA quickly).

In bug 23463 we inadvertently re-introduced some of the setting code
during a rebase.

In bug 18501 we then moved that inadvertently re-introduced setter into
Koha::Item->store.

We should kill it off once and for all!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 16 Martin Renvoize 2020-09-04 16:07:07 UTC
Created attachment 109675 [details] [review]
Bug 26268: Remove final references to paidfor db field

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 17 Martin Renvoize 2020-09-04 16:07:13 UTC
Created attachment 109676 [details] [review]
Bug 26268: Remove marc framework mappings to items.paidfor

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 18 Martin Renvoize 2020-09-04 16:07:17 UTC
Created attachment 109677 [details] [review]
Bug 26268: (QA follow-up) Split DB update checks

This patch splits the DB update such that the 'paidfor' field is checked
for distinctly in the deleteditems table rather than assuming if it
exists in the items table it will also exist in deleteditems.

We also correct the check as highlights by Julian.
Comment 19 Martin Renvoize 2020-09-04 16:08:09 UTC
Good catch, corrected in the followup (which should probably be merged on push, but kept it separate to keep it clear the work has been done).
Comment 20 Julian Maurice 2020-09-11 08:11:36 UTC
Sorry, but I think there is still one thing missing : the updatedatabase should take care of removing any remaining reference to 'items.paidfor' in marc_subfield_structure
Comment 21 Katrin Fischer 2020-09-11 09:22:24 UTC
(In reply to Julian Maurice from comment #20)
> Sorry, but I think there is still one thing missing : the updatedatabase
> should take care of removing any remaining reference to 'items.paidfor' in
> marc_subfield_structure

Hm, I think that could be an issue we haven't thought about yet. For a long time there was not column for internal note in items and the default mapping we shipped Koha with mapped 952$x to items.paidfor. I think this is another reason not to drop the data if the column still contains information and to only output a note. And we shoudl not remove the mapping. If people have not actively remapped (which requires server action etc.) they might still store the internal note there.
Comment 22 Katrin Fischer 2020-09-11 09:26:20 UTC
= Only remove the mapping if there is no data in the column etc.
Comment 23 Martin Renvoize 2020-09-11 11:53:04 UTC
Created attachment 109930 [details] [review]
Bug 26268: Remove items.paidfor from the database

In bug 19919 we removed all references to setting, unsettling and
referring to the paidfor field but we neglected to actually drop the
field from the database (I think I intended to do this, but was asking
for feedback on the approach before adding the actual db change.. then
completely forgot when it went through SO/QA quickly).

In bug 23463 we inadvertently re-introduced some of the setting code
during a rebase.

In bug 18501 we then moved that inadvertently re-introduced setter into
Koha::Item->store.

We should kill it off once and for all!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 24 Martin Renvoize 2020-09-11 11:53:08 UTC
Created attachment 109931 [details] [review]
Bug 26268: Remove final references to paidfor db field

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 25 Martin Renvoize 2020-09-11 11:53:12 UTC
Created attachment 109932 [details] [review]
Bug 26268: Remove marc framework mappings to items.paidfor

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 26 Martin Renvoize 2020-09-11 11:53:17 UTC
Created attachment 109933 [details] [review]
Bug 26268: (QA follow-up) Split DB update checks

This patch splits the DB update such that the 'paidfor' field is checked
for distinctly in the deleteditems table rather than assuming if it
exists in the items table it will also exist in deleteditems.

We also correct the check as highlights by Julian.
Comment 27 Martin Renvoize 2020-09-11 11:53:22 UTC
Created attachment 109934 [details] [review]
Bug 26268: (QA follow-up) Conditionally remove

This patch adds conditions to prevent the removal of items.paidfor if it
is populated and also adds the removal fo the marc_subfield_structure
row if it exists.
Comment 28 Martin Renvoize 2020-09-11 11:53:49 UTC
Follow-up added, back to NQA
Comment 29 Katrin Fischer 2020-09-26 21:55:07 UTC
Created attachment 110801 [details] [review]
Bug 26268: Remove items.paidfor from the database

In bug 19919 we removed all references to setting, unsettling and
referring to the paidfor field but we neglected to actually drop the
field from the database (I think I intended to do this, but was asking
for feedback on the approach before adding the actual db change.. then
completely forgot when it went through SO/QA quickly).

In bug 23463 we inadvertently re-introduced some of the setting code
during a rebase.

In bug 18501 we then moved that inadvertently re-introduced setter into
Koha::Item->store.

We should kill it off once and for all!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 30 Katrin Fischer 2020-09-26 21:55:13 UTC
Created attachment 110802 [details] [review]
Bug 26268: Remove final references to paidfor db field

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 31 Katrin Fischer 2020-09-26 21:55:20 UTC
Created attachment 110803 [details] [review]
Bug 26268: (QA follow-up) Split DB update checks

This patch splits the DB update such that the 'paidfor' field is checked
for distinctly in the deleteditems table rather than assuming if it
exists in the items table it will also exist in deleteditems.

We also correct the check as highlights by Julian.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 32 Katrin Fischer 2020-09-26 21:55:28 UTC
Created attachment 110804 [details] [review]
Bug 26268: (QA follow-up) Conditionally remove

This patch adds conditions to prevent the removal of items.paidfor if it
is populated and also adds the removal fo the marc_subfield_structure
row if it exists.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 33 Katrin Fischer 2020-09-26 21:55:33 UTC
Created attachment 110805 [details] [review]
Bug 26268: Correct mapping for internal note to items.itemnotes_nonpublic

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 34 Katrin Fischer 2020-09-26 21:55:39 UTC
Created attachment 110806 [details] [review]
Bug 26268: Remove invalid mappings from MARC frameworks when items.paidfor is removed

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 35 Katrin Fischer 2020-09-26 21:59:42 UTC
Hi Martin,

952$x is the internal note field that was mapped to items.paidfor in the past before we added a proper column for it: itemsnotes_nonpublic. In some frameworks the old mappings had survived, but we only want to correct the mapping, not remove the subfield leaving these languages with no internal note field in their items.

So I made some changes:

- I killed your patch changing the framework files as you deleted the 952$x lines instead of just correcting the mapping. I replaced it by a follow-up patch - it was just a little easier this way.

- I changed your database update. If the column items.paidfor is successfully removed, we don't care which field was mapped to it - we want to reset it as it's now an invalid mapping.

Can you please have a look at my follow-ups and sign-off/PQA if you agree?
Comment 36 Martin Renvoize 2020-09-28 09:19:08 UTC
Created attachment 110836 [details] [review]
Bug 26268: Remove items.paidfor from the database

In bug 19919 we removed all references to setting, unsettling and
referring to the paidfor field but we neglected to actually drop the
field from the database (I think I intended to do this, but was asking
for feedback on the approach before adding the actual db change.. then
completely forgot when it went through SO/QA quickly).

In bug 23463 we inadvertently re-introduced some of the setting code
during a rebase.

In bug 18501 we then moved that inadvertently re-introduced setter into
Koha::Item->store.

We should kill it off once and for all!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 37 Martin Renvoize 2020-09-28 09:19:12 UTC
Created attachment 110837 [details] [review]
Bug 26268: Remove final references to paidfor db field

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 38 Martin Renvoize 2020-09-28 09:19:17 UTC
Created attachment 110838 [details] [review]
Bug 26268: (QA follow-up) Split DB update checks

This patch splits the DB update such that the 'paidfor' field is checked
for distinctly in the deleteditems table rather than assuming if it
exists in the items table it will also exist in deleteditems.

We also correct the check as highlights by Julian.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 39 Martin Renvoize 2020-09-28 09:19:21 UTC
Created attachment 110839 [details] [review]
Bug 26268: (QA follow-up) Conditionally remove

This patch adds conditions to prevent the removal of items.paidfor if it
is populated and also adds the removal fo the marc_subfield_structure
row if it exists.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 40 Martin Renvoize 2020-09-28 09:19:26 UTC
Created attachment 110840 [details] [review]
Bug 26268: Correct mapping for internal note to items.itemnotes_nonpublic

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 41 Martin Renvoize 2020-09-28 09:19:30 UTC
Created attachment 110841 [details] [review]
Bug 26268: Remove invalid mappings from MARC frameworks when items.paidfor is removed

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 42 Martin Renvoize 2020-09-28 09:20:10 UTC
All works well and I agree with the follow-ups.

Setting to PQA as requested :).. Yeay!
Comment 43 Jonathan Druart 2020-09-29 15:06:24 UTC
Created attachment 110935 [details] [review]
Bug 26268: Fix deleteditems query in DB rev
Comment 44 Jonathan Druart 2020-10-01 09:45:14 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 45 Lucas Gass 2020-10-20 17:36:51 UTC
enhancement will not be backported to 20.05.x
Comment 46 David Cook 2023-06-20 01:05:24 UTC
(In reply to Martin Renvoize from comment #27)
> Created attachment 109934 [details] [review] [review]
> Bug 26268: (QA follow-up) Conditionally remove
> 
> This patch adds conditions to prevent the removal of items.paidfor if it
> is populated and also adds the removal fo the marc_subfield_structure
> row if it exists.

Unfortunately, it seems like this conditional removal can lead to inconsistent database schemas...

issues_stats.pl is breaking because my items table has 47 columns and deleteditems table has 46 columns. The "paidfor" warning message must've been missed in the CLI output at upgrade time.

Not sure what the answer is here. Maybe an audit script...