Bug 20271 - Merge deleted biblio, biblioitems, biblio_metadata, and items tables with their "alive" cousins
Summary: Merge deleted biblio, biblioitems, biblio_metadata, and items tables with the...
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: master
Hardware: All All
: P1 - high major (vote)
Assignee: Francesco Rivetti
QA Contact: Martin Renvoize
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 11983 20940
  Show dependency treegraph
 
Reported: 2018-02-22 13:42 UTC by Petter Goksøyr Åsen
Modified: 2018-07-06 10:22 UTC (History)
14 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 20271: merging delete biblio/items (30.19 KB, patch)
2018-03-01 08:53 UTC, Francesco Rivetti
Details | Diff | Splinter Review
Bug 20271: merging delete biblio/items (34.15 KB, patch)
2018-03-13 15:43 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271 - merge deletedbiblio and -items back - add atomic update (2.39 KB, patch)
2018-03-13 15:43 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: merging delete biblio/items (34.15 KB, patch)
2018-03-19 13:44 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271 - merge deletedbiblio and -items back - add atomic update (2.39 KB, patch)
2018-03-19 13:44 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: fix remaining tests, add checks and report in atomicupdate (9.64 KB, patch)
2018-03-19 13:44 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: merging delete biblio/items (33.69 KB, patch)
2018-04-02 22:22 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271 - merge deletedbiblio and -items back - add atomic update (2.39 KB, patch)
2018-04-02 22:22 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: fix remaining tests, add checks and report in atomicupdate (9.64 KB, patch)
2018-04-02 22:22 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: update kohastructure.sql (13.03 KB, patch)
2018-04-02 22:22 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: delete biblio and biblio_metadata in one go (813 bytes, patch)
2018-04-02 22:22 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: remove comments regarding deleteditems and -biblio* (2.63 KB, patch)
2018-04-02 22:22 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271 - Use a single pass in the OAI-PMH provider (17.99 KB, patch)
2018-04-04 07:50 UTC, Ere Maijala (away until Aug 16)
Details | Diff | Splinter Review
Bug 20271: merging delete biblio/items (33.69 KB, patch)
2018-04-06 12:24 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271 - merge deletedbiblio and -items back - add atomic update (5.05 KB, patch)
2018-04-06 12:25 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: fix remaining tests (3.94 KB, patch)
2018-04-06 12:25 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: update kohastructure.sql (13.03 KB, patch)
2018-04-06 12:25 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: delete biblio and biblio_metadata in one go (813 bytes, patch)
2018-04-06 12:25 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: remove comments regarding deleteditems and -biblio* (2.63 KB, patch)
2018-04-06 12:25 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271 - Use a single pass in the OAI-PMH provider (17.99 KB, patch)
2018-04-06 12:25 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: merging delete biblio/items (34.79 KB, patch)
2018-04-13 07:17 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: fix remaining tests (3.94 KB, patch)
2018-04-13 07:17 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: update kohastructure.sql (13.03 KB, patch)
2018-04-13 07:18 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: delete biblio and biblio_metadata in one go (813 bytes, patch)
2018-04-13 07:18 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: remove comments regarding deleteditems and -biblio* (2.63 KB, patch)
2018-04-13 07:18 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: add atomic update (5.17 KB, patch)
2018-04-13 07:18 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: Use a single pass in the OAI-PMH provider (17.99 KB, patch)
2018-04-13 07:19 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: fix whitespace issues in C4::Items (24.80 KB, patch)
2018-04-13 07:19 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: merging delete biblio/items (34.82 KB, patch)
2018-04-13 07:35 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: fix remaining tests (3.94 KB, patch)
2018-04-13 07:36 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: update kohastructure.sql (13.03 KB, patch)
2018-04-13 07:36 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: delete biblio and biblio_metadata in one go (813 bytes, patch)
2018-04-13 07:36 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: remove comments regarding deleteditems and -biblio* (2.63 KB, patch)
2018-04-13 07:36 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: add atomic update (5.17 KB, patch)
2018-04-13 07:36 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: Use a single pass in the OAI-PMH provider (17.99 KB, patch)
2018-04-13 07:36 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: fix whitespace issues in C4::Items (24.80 KB, patch)
2018-04-13 07:36 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: merging delete biblio/items (34.82 KB, patch)
2018-04-14 10:00 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: fix remaining tests (3.94 KB, patch)
2018-04-14 10:00 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: update kohastructure.sql (13.03 KB, patch)
2018-04-14 10:00 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: delete biblio and biblio_metadata in one go (813 bytes, patch)
2018-04-14 10:00 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: remove comments regarding deleteditems and -biblio* (2.63 KB, patch)
2018-04-14 10:00 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: add atomic update (5.05 KB, patch)
2018-04-14 10:00 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: Use a single pass in the OAI-PMH provider (17.99 KB, patch)
2018-04-14 10:00 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: fix whitespace issues in C4::Items (24.80 KB, patch)
2018-04-14 10:00 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: print instead of warn unmovable items (5.58 KB, patch)
2018-04-14 17:40 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: atomicupdate - support mysql < 5.7 and address foreign key issues (2.89 KB, patch)
2018-04-16 09:08 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: merging delete biblio/items (34.89 KB, patch)
2018-04-18 06:41 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 20271: fix remaining tests (4.01 KB, patch)
2018-04-18 06:41 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 20271: update kohastructure.sql (13.10 KB, patch)
2018-04-18 06:41 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 20271: delete biblio and biblio_metadata in one go (879 bytes, patch)
2018-04-18 06:41 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 20271: remove comments regarding deleteditems and -biblio* (2.70 KB, patch)
2018-04-18 06:41 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 20271: add atomic update (5.12 KB, patch)
2018-04-18 06:41 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 20271: Use a single pass in the OAI-PMH provider (18.06 KB, patch)
2018-04-18 06:41 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 20271: fix whitespace issues in C4::Items (24.86 KB, patch)
2018-04-18 06:41 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 20271: print instead of warn unmovable items (5.64 KB, patch)
2018-04-18 06:41 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 20271: atomicupdate - support mysql < 5.7 and address foreign key issues (2.96 KB, patch)
2018-04-18 06:41 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 20271: (follow-up) DBIx schema changes (25.26 KB, patch)
2018-05-11 10:19 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20271: (QA follow-up): rename to deleted_on and various fixes (42.40 KB, patch)
2018-05-15 21:39 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Additional about cleanup (2.51 KB, patch)
2018-06-08 06:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20271: (QA follow-up): rename to deleted_on and various fixes (40.55 KB, patch)
2018-06-08 07:55 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) DBIx schema changes (4.84 KB, patch)
2018-06-08 07:55 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Additional about cleanup (2.57 KB, patch)
2018-06-08 07:55 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Remove last occurrences of deleted_at (1.73 KB, patch)
2018-06-08 08:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20271: merging delete biblio/items (35.08 KB, patch)
2018-06-12 14:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: fix remaining tests (4.25 KB, patch)
2018-06-12 14:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: update kohastructure.sql (13.22 KB, patch)
2018-06-12 14:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: delete biblio and biblio_metadata in one go (951 bytes, patch)
2018-06-12 14:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: remove comments regarding deleteditems and -biblio* (2.77 KB, patch)
2018-06-12 14:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: add atomic update (5.19 KB, patch)
2018-06-12 14:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: Use a single pass in the OAI-PMH provider (18.14 KB, patch)
2018-06-12 14:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: fix whitespace issues in C4::Items (24.93 KB, patch)
2018-06-12 14:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: print instead of warn unmovable items (5.71 KB, patch)
2018-06-12 14:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: atomicupdate - support mysql < 5.7 and address foreign key issues (3.03 KB, patch)
2018-06-12 14:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: (follow-up) DBIx schema changes (25.34 KB, patch)
2018-06-12 14:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: (QA follow-up): rename to deleted_on and various fixes (40.73 KB, patch)
2018-06-12 14:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) DBIx schema changes (4.89 KB, patch)
2018-06-12 14:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Additional about cleanup (2.61 KB, patch)
2018-06-12 14:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Remove last occurrences of deleted_at (1.76 KB, patch)
2018-06-12 14:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Reword OAI-PMH:DeletedRecord systempref (3.06 KB, patch)
2018-06-12 14:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Make the atomicupdate idempotent (11.57 KB, patch)
2018-06-12 14:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: Restore original GetItem functionality (2.95 KB, patch)
2018-06-14 10:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Restore original GetItem functionality (2.97 KB, patch)
2018-06-14 11:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Fix catalogue statistics in reports module (3.10 KB, patch)
2018-06-22 08:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: merging delete biblio/items (35.01 KB, patch)
2018-06-22 10:06 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: fix remaining tests (4.27 KB, patch)
2018-06-22 10:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: update kohastructure.sql (13.22 KB, patch)
2018-06-22 10:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: delete biblio and biblio_metadata in one go (1000 bytes, patch)
2018-06-22 10:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: remove comments regarding deleteditems and -biblio* (2.81 KB, patch)
2018-06-22 10:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: add atomic update (5.24 KB, patch)
2018-06-22 10:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: Use a single pass in the OAI-PMH provider (18.18 KB, patch)
2018-06-22 10:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: fix whitespace issues in C4::Items (24.98 KB, patch)
2018-06-22 10:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: print instead of warn unmovable items (5.76 KB, patch)
2018-06-22 10:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: atomicupdate - support mysql < 5.7 and address foreign key issues (3.08 KB, patch)
2018-06-22 10:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (follow-up) DBIx schema changes (25.34 KB, patch)
2018-06-22 10:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up): rename to deleted_on and various fixes (40.63 KB, patch)
2018-06-22 10:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) DBIx schema changes (4.92 KB, patch)
2018-06-22 10:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Additional about cleanup (2.65 KB, patch)
2018-06-22 10:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Remove last occurrences of deleted_at (1.81 KB, patch)
2018-06-22 10:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Reword OAI-PMH:DeletedRecord systempref (3.11 KB, patch)
2018-06-22 10:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Make the atomicupdate idempotent (11.59 KB, patch)
2018-06-22 10:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Restore original GetItem functionality (3.00 KB, patch)
2018-06-22 10:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Fix catalogue statistics in reports module (3.10 KB, patch)
2018-06-22 10:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Zebra should not index deleted record when doing full reindex (1.31 KB, patch)
2018-06-22 10:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Does not return deleted record in GetMarcBiblio (1.12 KB, patch)
2018-06-22 10:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up): Fix test ModBiblioMarc (1.75 KB, patch)
2018-06-22 10:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: merging delete biblio/items (35.01 KB, patch)
2018-06-27 13:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: fix remaining tests (4.27 KB, patch)
2018-06-27 13:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: update kohastructure.sql (13.22 KB, patch)
2018-06-27 13:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: delete biblio and biblio_metadata in one go (1000 bytes, patch)
2018-06-27 13:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: remove comments regarding deleteditems and -biblio* (2.81 KB, patch)
2018-06-27 13:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: add atomic update (5.24 KB, patch)
2018-06-27 13:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: Use a single pass in the OAI-PMH provider (18.18 KB, patch)
2018-06-27 13:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: fix whitespace issues in C4::Items (22.32 KB, patch)
2018-06-27 13:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: print instead of warn unmovable items (5.76 KB, patch)
2018-06-27 13:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: atomicupdate - support mysql < 5.7 and address foreign key issues (3.08 KB, patch)
2018-06-27 13:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (follow-up) DBIx schema changes (25.34 KB, patch)
2018-06-27 13:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up): rename to deleted_on and various fixes (40.63 KB, patch)
2018-06-27 13:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) DBIx schema changes (4.92 KB, patch)
2018-06-27 13:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Additional about cleanup (2.65 KB, patch)
2018-06-27 13:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Remove last occurrences of deleted_at (1.81 KB, patch)
2018-06-27 13:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Reword OAI-PMH:DeletedRecord systempref (3.11 KB, patch)
2018-06-27 13:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Make the atomicupdate idempotent (11.59 KB, patch)
2018-06-27 13:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Restore original GetItem functionality (3.00 KB, patch)
2018-06-27 13:39 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Fix catalogue statistics in reports module (3.10 KB, patch)
2018-06-27 13:39 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Zebra should not index deleted record when doing full reindex (1.31 KB, patch)
2018-06-27 13:39 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up) Does not return deleted record in GetMarcBiblio (1.12 KB, patch)
2018-06-27 13:39 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20271: (QA follow-up): Fix test ModBiblioMarc (1.75 KB, patch)
2018-06-27 13:39 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Petter Goksøyr Åsen 2018-02-22 13:42:11 UTC
In Koha's DB, instead of having a "soft" deletion of entries by flagging the deleted one as so, the rows in the db is actually deleted, but only after a new row in a different table is inserted.

Koha's DB is relational, so there is a cascade of links between each object: a biblio is referred by many items, and each item is referred by issues, holds, messages.

E.g.: when a row in the item table is moved from "items" to "deleted_items", then all the references from holds, transfers and so on will be broken.

The suggested more orthodox approach to this would be to add a "deleted_at" column, which is normally null unless the row is deleted, then it set to the timestamp of when it was deletion.

This means that the row is still there and will still be referred by other tables, but it just won't be shown in the relevant lists.

This would improve the maintenance of Koha in many ways (e.g.: the auto_increment id on those tables caused lots of issues when you add an item and delete it immediately after. the next new item will have the same itemnumber as the previously deleted one, clashing with it in many ways).

This bug is about merging the all these tables:
deletedborrowers       => borrowers
deletedbiblio          => biblio
deletedbiblioitems     => biblioitems
deletedbiblio_metadata => biblio_metadata
deleteditems           => items
old_issues             => issues
old_reserves           => reserves
Comment 1 Katrin Fischer 2018-02-23 20:39:34 UTC
I like the idea of making it a timestamp instead of a boolean as suggested on other bugs (I think last discussed for recalls on bug 19532). Are there any reasons why one should be preferred over the other, for example for performance?
Comment 2 Petter Goksøyr Åsen 2018-02-25 05:51:00 UTC
> Are there any reasons why one should be preferred over the other, for example for performance?

I dont't think so. As long as the column is indexed, querying for one or the other should perform with equivalent speed.

The benefit of using a timestamp is that it carries more information, and mirrors the current behaviour, where the timestamp in the deleted-tables denotes when the row was deleted.
Comment 3 Francesco Rivetti 2018-02-26 09:17:31 UTC
I've started working on this and have migrated items, biblios, biblioitems and biblio_metadata successfully. it works fine on intra and seems ok in the administrative reports.

there is also a wiki page with more info here: https://wiki.koha-community.org/wiki/MergingOfTables
Comment 4 Francesco Rivetti 2018-03-01 08:53:14 UTC
Created attachment 72300 [details] [review]
Bug 20271: merging delete biblio/items

NOTE: deleteditems/biblio on OAI
not sure how to test this, but the changes should be good enough
Comment 5 Josef Moravec 2018-03-02 07:09:29 UTC
Comment on attachment 72300 [details] [review]
Bug 20271: merging delete biblio/items

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

Just a quick look:

there is missing atomic database update

also the test suite should be adjusted

I am not sure about the changes in OAI, so need to investigate more later

::: C4/Biblio.pm
@@ -3182,5 @@
> -        my $bkup_sth = $dbh->prepare($query);
> -        $bkup_sth->execute(@bind);
> -        $bkup_sth->finish;
> -
> -        _koha_delete_biblio_metadata( $biblionumber );

this is missing in your patch

::: C4/Items.pm
@@ +1061,4 @@
>       FROM items
>       LEFT JOIN branches AS holding ON items.holdingbranch = holding.branchcode
>       LEFT JOIN branches AS home ON items.homebranch=home.branchcode
> +     LEFT JOIN biblio      ON      biblio.biblionumber     = items.biblionumber AND biblio.deleted_at IS NULL

I don't think this will work, that's condition should be in where clause

::: misc/export_records.pl
@@ +105,5 @@
>          push @record_ids, $_->{biblionumber} for @{
> +            $dbh->selectall_arrayref(q|
> +                SELECT biblionumber
> +                FROM biblioitems
> +             |, { Slice => {} }, ( $timestamp ) x 4 );

Please include the changes made by Bug 19730
Comment 6 Francesco Rivetti 2018-03-05 08:35:03 UTC
(In reply to Josef Moravec from comment #5)
> there is missing atomic database update

It is on its way.

(It's fairly easy to do something reasonable, but there are some corner cases
which I would like to handle somehow better)

I would left the deleted* tables there with any clashing rows and let the
administrator deal with it in case there are problems. thoughts?


> I am not sure about the changes in OAI, so need to investigate more later

Thank you so much, I wish I knew more about OAI

> ::: C4/Biblio.pm
> @@ -3182,5 @@
> > -        my $bkup_sth = $dbh->prepare($query);
> > -        $bkup_sth->execute(@bind);
> > -        $bkup_sth->finish;
> > -
> > -        _koha_delete_biblio_metadata( $biblionumber );
> 
> this is missing in your patch

I might be lagging behind master, because I can't find it.

> ::: C4/Items.pm
> @@ +1061,4 @@
> >       FROM items
> >       LEFT JOIN branches AS holding ON items.holdingbranch = holding.branchcode
> >       LEFT JOIN branches AS home ON items.homebranch=home.branchcode
> > +     LEFT JOIN biblio      ON      biblio.biblionumber     = items.biblionumber AND biblio.deleted_at IS NULL
> 
> I don't think this will work, that's condition should be in where clause

I actually think it will be irrelevant. The FKs should make sure that if there is an item, there must be the biblio which it refers.

Or am I missing something?


> ::: misc/export_records.pl
> @@ +105,5 @@
> >          push @record_ids, $_->{biblionumber} for @{
> > +            $dbh->selectall_arrayref(q|
> > +                SELECT biblionumber
> > +                FROM biblioitems
> > +             |, { Slice => {} }, ( $timestamp ) x 4 );
> 
> Please include the changes made by Bug 19730

Is it in master, right?

TIA
Comment 7 Katrin Fischer 2018-03-05 13:28:42 UTC
I think we should try and provide at least a script to automate merging the two tables. You cannot assume that most admins know Koha well enough to fix the issues themselves. A problem might be the possible existence of duplicated rows. We could check that and then suggest a fix before continuing.
Comment 8 Francesco Rivetti 2018-03-05 13:54:26 UTC
(In reply to Katrin Fischer from comment #7)
> I think we should try and provide at least a script to automate merging the
> two tables. You cannot assume that most admins know Koha well enough to fix
> the issues themselves. A problem might be the possible existence of
> duplicated rows. We could check that and then suggest a fix before
> continuing.

My idea is to move all the rows properly.

if any rows clash (for some unforeseen reason, i found a "orphaned" item referring to a non-existent biblio here, but can be anything!) then they won't be moved.

at the end, if the table is empty, it gets removed, otherwise it stays there.

does it sounds good?
Comment 9 Benjamin Rokseth 2018-03-13 15:43:10 UTC
Created attachment 72799 [details] [review]
Bug 20271: merging delete biblio/items

NOTE: deleteditems/biblio on OAI
not sure how to test this, but the changes should be good enough
Comment 10 Benjamin Rokseth 2018-03-13 15:43:18 UTC
Created attachment 72800 [details] [review]
Bug 20271 - merge deletedbiblio and -items back - add atomic update
Comment 11 Benjamin Rokseth 2018-03-13 15:48:37 UTC
Updated the failing tests, except for the weird GetItemsForInventory test which needs another look.

Ready for testing, we renamed the ticket to only concern tables deletedbiblio, deletedbiblioitems, deletedbiblio_metadata and deleteditems, since they are most crucial to avoid more data loss and corruption.

Suggest to rename deleted* tables temporarily before deleting them, so any merge conflicts can be examined first

Also suggest to run misc/devel/update_dbix_class_files.pl after, so schema/apis etc work as expected
Comment 12 Jon Knight 2018-03-14 19:51:19 UTC
Just a random thought to throw into this: would it be worth creating database views to replicate the old tables based on the new ones?  That way if any installations have pre-existing reports that use the old tables in their queries, they will still continue to work.

For example something like this:

CREATE VIEW old_issues AS SELECT * FROM issues WHERE deleted_at IS NOT NULL;
Comment 13 Francesco Rivetti 2018-03-15 10:41:07 UTC
(In reply to Jon Knight from comment #12)
> Just a random thought to throw into this: would it be worth creating
> database views to replicate the old tables based on the new ones?  That way
> if any installations have pre-existing reports that use the old tables in
> their queries, they will still continue to work.
> 
> For example something like this:
> 
> CREATE VIEW old_issues AS SELECT * FROM issues WHERE deleted_at IS NOT NULL;

I thought about it, but you most likely need the view for "issues" than "old_issues", which means:

RENAME TABLE issues TO all_issues;
CREATE VIEW issues AS SELECT * FROM issues WHERE delted_at IS NULL;

but then, more code will need to be changed for updates/insert/deletes

OTOH, most reports UNION issues with old_issues, might be smart to take this opportunity to simplify them?

(note, this applies to deleteditems/biblios as well)
Comment 14 Benjamin Rokseth 2018-03-19 13:44:39 UTC
Created attachment 73091 [details] [review]
Bug 20271: merging delete biblio/items

NOTE: deleteditems/biblio on OAI
not sure how to test this, but the changes should be good enough
Comment 15 Benjamin Rokseth 2018-03-19 13:44:46 UTC
Created attachment 73092 [details] [review]
Bug 20271 - merge deletedbiblio and -items back - add atomic update
Comment 16 Benjamin Rokseth 2018-03-19 13:44:54 UTC
Created attachment 73093 [details] [review]
Bug 20271: fix remaining tests, add checks and report in atomicupdate
Comment 17 Benjamin Rokseth 2018-03-19 13:52:13 UTC
Ready for testing now, test plan:

1) make sure you have some deleted biblios and items
2) apply patch and run updatedatabase
3) if you have any clashing keys, they should be reported during updatedatabase and be left in renamed corresponding tables with _ (e.g. _deleteditems)
4) run update_dbix_class_files.pl (to make sure schemas and rest api are ok)
5) check that db_dependent tests concerning biblios and items pass OK
6) check that you can still delete items and biblio
7) for any deleted item or biblio, you should now have a timestamp in the 'deleted_at' column.
Comment 18 Benjamin Rokseth 2018-03-20 09:53:32 UTC
raised the importance to critical since it does in fact lead to data loss and possibly data corruption
Comment 19 Francesco Rivetti 2018-03-20 10:27:21 UTC
thanks Benjamin!

(In reply to Benjamin Rokseth from comment #16)
> Created attachment 73093 [details] [review] [review]
> Bug 20271: fix remaining tests, add checks and report in atomicupdate
Comment 20 Ere Maijala (away until Aug 16) 2018-03-27 10:49:34 UTC
I think the OAI-PMH provider should now be changed to return all records in a single pass. It was built to return deleted records separately because that was much faster than joining tables on the fly, but when deleted and non-deleted records are in the same table this complication is not needed anymore. I think the whole ListBase class could be removed in this case.

Please let me know if you'd rather let me do the relevant changes in the OAI classes.
Comment 21 Ere Maijala (away until Aug 16) 2018-03-27 12:13:40 UTC
One more note: I couldn't find any changes to kohastructure.sql. Are they missing from the patches or did I just miss them?
Comment 22 Ere Maijala (away until Aug 16) 2018-03-27 12:44:36 UTC
And one more: there are still comments that refer to the deleted* tables.
Comment 23 Ere Maijala (away until Aug 16) 2018-03-27 12:48:52 UTC
Sorry for the comment spam... one more: the call to _koha_delete_biblio_metadata has been removed from _koha_delete_biblio. As far as I can see nothing takes care of marking the metadata record deleted.
Comment 24 Benjamin Rokseth 2018-04-02 22:22:02 UTC
Created attachment 73535 [details] [review]
Bug 20271: merging delete biblio/items

NOTE: deleteditems/biblio on OAI
not sure how to test this, but the changes should be good enough
Comment 25 Benjamin Rokseth 2018-04-02 22:22:09 UTC
Created attachment 73536 [details] [review]
Bug 20271 - merge deletedbiblio and -items back - add atomic update
Comment 26 Benjamin Rokseth 2018-04-02 22:22:14 UTC
Created attachment 73537 [details] [review]
Bug 20271: fix remaining tests, add checks and report in atomicupdate
Comment 27 Benjamin Rokseth 2018-04-02 22:22:21 UTC
Created attachment 73538 [details] [review]
Bug 20271: update kohastructure.sql
Comment 28 Benjamin Rokseth 2018-04-02 22:22:27 UTC
Created attachment 73539 [details] [review]
Bug 20271: delete biblio and biblio_metadata in one go
Comment 29 Benjamin Rokseth 2018-04-02 22:22:32 UTC
Created attachment 73540 [details] [review]
Bug 20271: remove comments regarding deleteditems and -biblio*
Comment 30 Benjamin Rokseth 2018-04-02 22:24:56 UTC
rebased against master and added fixes to comments except OAI.

Ere, since you voluntered to add the neccessary fixes to OAI, I leave them to you ;)
Comment 31 Ere Maijala (away until Aug 16) 2018-04-04 06:32:09 UTC
I'm working on it now. 


Looks like there may be a typo in the atomic update. I got the following error:

updatedatabase.pl: DBD::mysql::db do failed: Can't DROP FOREIGN KEY `deletedbiblio_metadata_fk_1`; check that it exists [for Statement "ALTER TABLE deletedbiblio_metadata DROP FOREIGN KEY deletedbiblio_metadata_fk_1"] at (eval 1389) line 8.

Looking at the old kohastructure.sql I believe it should have tried to drop 'deletedrecord_metadata_fk_1'.
Comment 32 Benjamin Rokseth 2018-04-04 07:18:49 UTC
(In reply to Ere Maijala from comment #31)
> I'm working on it now. 
> 
> 
> Looks like there may be a typo in the atomic update. I got the following
> error:
> 
> updatedatabase.pl: DBD::mysql::db do failed: Can't DROP FOREIGN KEY
> `deletedbiblio_metadata_fk_1`; check that it exists [for Statement "ALTER
> TABLE deletedbiblio_metadata DROP FOREIGN KEY deletedbiblio_metadata_fk_1"]
> at (eval 1389) line 8.
> 
> Looking at the old kohastructure.sql I believe it should have tried to drop
> 'deletedrecord_metadata_fk_1'.

hmm, sad to say, but no, it's not a typo but db design issues.
this commit seems to have added to the confusion: https://github.com/Koha-Community/Koha/commit/739e2e0c5dc26502e195e2cc4c687ababdcaf381 (Bug 17196)

and following Bug 18284 you can see that most agree on deleted* tables being a bad idea in the first place.

Problem is, kohastructure.sql and updatedatabase.pl is not in sync and libraries having migrated from 16.05 and before would need to delete this foreign key. And this patch is most important for them anyways.

I suppose we could add a delete foreign key for both and ignore errors rather than die in the final version
Comment 33 Ere Maijala (away until Aug 16) 2018-04-04 07:45:12 UTC
Oh, that's a nasty one. But do you even have to drop the constraints to delete the table? I think it's enough to drop the table and all the constraints there would be dropped too, no?
Comment 34 Ere Maijala (away until Aug 16) 2018-04-04 07:50:02 UTC
Created attachment 73597 [details] [review]
Bug 20271 - Use a single pass in the OAI-PMH provider
Comment 35 Ere Maijala (away until Aug 16) 2018-04-04 07:51:06 UTC
Changes for the OAI-PMH provider have been attached. I'm really happy to make it so much more simple. Please feel free to rebase or whatever is necessary.
Comment 36 Benjamin Rokseth 2018-04-04 08:02:53 UTC
(In reply to Benjamin Rokseth from comment #32)
> (In reply to Ere Maijala from comment #31)
> > I'm working on it now. 
> > 
> > 
> > Looks like there may be a typo in the atomic update. I got the following
> > error:
> > 
> > updatedatabase.pl: DBD::mysql::db do failed: Can't DROP FOREIGN KEY
> > `deletedbiblio_metadata_fk_1`; check that it exists [for Statement "ALTER
> > TABLE deletedbiblio_metadata DROP FOREIGN KEY deletedbiblio_metadata_fk_1"]
> > at (eval 1389) line 8.
> > 
> > Looking at the old kohastructure.sql I believe it should have tried to drop
> > 'deletedrecord_metadata_fk_1'.
> 
> hmm, sad to say, but no, it's not a typo but db design issues.
> this commit seems to have added to the confusion:
> https://github.com/Koha-Community/Koha/commit/
> 739e2e0c5dc26502e195e2cc4c687ababdcaf381 (Bug 17196)
> 
> and following Bug 18284 you can see that most agree on deleted* tables being
> a bad idea in the first place.
> 
> Problem is, kohastructure.sql and updatedatabase.pl is not in sync and
> libraries having migrated from 16.05 and before would need to delete this
> foreign key. And this patch is most important for them anyways.
> 
> I suppose we could add a delete foreign key for both and ignore errors
> rather than die in the final version

fail to remember, but probably I put it there because otherwise it complained, perhaps due to the "rename table". the idea was that any leftovers from conflicts should be moved to temporary tables (prepended with _) so that one could check and delete them at convenience

I will remove the dies in the atomicupdate and see if it works for us
Comment 37 Francesco Rivetti 2018-04-04 08:09:23 UTC
(In reply to Benjamin Rokseth from comment #36)
> fail to remember, but probably I put it there because otherwise it
> complained, perhaps due to the "rename table". the idea was that any
> leftovers from conflicts should be moved to temporary tables (prepended with
> _) so that one could check and delete them at convenience

We had to drop the constraint to avoid the delete on cascade. When we clean up the deletedbiblio andt able, we don't want to drop any items which were left behind because not properly merged.
Comment 38 Ere Maijala (away until Aug 16) 2018-04-04 08:10:55 UTC
Ok, fair enough. I think Benjamin's proposal of dropping both without die'ing makes sense.
Comment 39 Benjamin Rokseth 2018-04-06 12:24:59 UTC
Created attachment 73759 [details] [review]
Bug 20271: merging delete biblio/items

NOTE: deleteditems/biblio on OAI
not sure how to test this, but the changes should be good enough
Comment 40 Benjamin Rokseth 2018-04-06 12:25:06 UTC
Created attachment 73760 [details] [review]
Bug 20271 - merge deletedbiblio and -items back - add atomic update
Comment 41 Benjamin Rokseth 2018-04-06 12:25:12 UTC
Created attachment 73761 [details] [review]
Bug 20271: fix remaining tests
Comment 42 Benjamin Rokseth 2018-04-06 12:25:18 UTC
Created attachment 73762 [details] [review]
Bug 20271: update kohastructure.sql
Comment 43 Benjamin Rokseth 2018-04-06 12:25:25 UTC
Created attachment 73763 [details] [review]
Bug 20271: delete biblio and biblio_metadata in one go
Comment 44 Benjamin Rokseth 2018-04-06 12:25:32 UTC
Created attachment 73764 [details] [review]
Bug 20271: remove comments regarding deleteditems and -biblio*
Comment 45 Benjamin Rokseth 2018-04-06 12:25:38 UTC
Created attachment 73765 [details] [review]
Bug 20271 - Use a single pass in the OAI-PMH provider
Comment 46 Benjamin Rokseth 2018-04-06 12:27:33 UTC
Replaced die with warn in atomicupdate, and rebased and squashed a bit.
Should be ready for testing now
Comment 47 Benjamin Rokseth 2018-04-11 11:13:06 UTC
Comment on attachment 73537 [details] [review]
Bug 20271: fix remaining tests, add checks and report in atomicupdate

Forgot to obsolete this
Comment 48 Benjamin Rokseth 2018-04-13 07:17:36 UTC
Created attachment 74127 [details] [review]
Bug 20271: merging delete biblio/items

The present bug merges the following tables:
deletedbiblio          -> biblio
deletedbiblioitems     -> biblioitems
deletedbiblio_metadata -> biblio_metadata
deleteditems           -> items

and adds a column deleted_at to signify time of deletion, if deleted

it replaces all occurrences of the mentioned occurences and uses
of mentioned deleted* tables

upgrade is handled by moving rows from deleted* tables to the live
ones. If all rows are moved, the corresponding tables are dropped.
If there are duplicates or conflicts (duplicate barcodes e.g.) they
reminders are left for scrutiny and table renamed to _deleted*

Test plan:

1) Make sure you have a db with at least a few deleted biblios,
   biblioitems and items.
2) Apply patch and run updatedatabase.pl
3) Make note of the db upgrade messages, if you get a message like:
   There were x deleteditems that could not be moved, please check _deleteditems
   you should check the _deleteditems table and verify that they are
   actual conflicts
4) Click around in the web interface and make sure you can delete items
   and biblios
5) "Undelete" an item by NULLing the deleted_at column for an item
   and verify that it returns in interface (Not neccessary really)
Comment 49 Benjamin Rokseth 2018-04-13 07:17:52 UTC
Created attachment 74128 [details] [review]
Bug 20271: fix remaining tests
Comment 50 Benjamin Rokseth 2018-04-13 07:18:04 UTC
Created attachment 74129 [details] [review]
Bug 20271: update kohastructure.sql
Comment 51 Benjamin Rokseth 2018-04-13 07:18:16 UTC
Created attachment 74130 [details] [review]
Bug 20271: delete biblio and biblio_metadata in one go
Comment 52 Benjamin Rokseth 2018-04-13 07:18:25 UTC
Created attachment 74131 [details] [review]
Bug 20271: remove comments regarding deleteditems and -biblio*
Comment 53 Benjamin Rokseth 2018-04-13 07:18:45 UTC
Created attachment 74132 [details] [review]
Bug 20271: add atomic update
Comment 54 Benjamin Rokseth 2018-04-13 07:19:05 UTC
Created attachment 74133 [details] [review]
Bug 20271: Use a single pass in the OAI-PMH provider
Comment 55 Benjamin Rokseth 2018-04-13 07:19:18 UTC
Created attachment 74134 [details] [review]
Bug 20271: fix whitespace issues in C4::Items
Comment 56 Benjamin Rokseth 2018-04-13 07:35:58 UTC
Created attachment 74135 [details] [review]
Bug 20271: merging delete biblio/items

The present bug merges the following tables:
deletedbiblio          -> biblio
deletedbiblioitems     -> biblioitems
deletedbiblio_metadata -> biblio_metadata
deleteditems           -> items

and adds a column deleted_at to signify time of deletion, if deleted

it replaces all occurrences of the mentioned occurences and uses
of mentioned deleted* tables

upgrade is handled by moving rows from deleted* tables to the live
ones. If all rows are moved, the corresponding tables are dropped.
If there are duplicates or conflicts (duplicate barcodes e.g.) they
reminders are left for scrutiny and table renamed to _deleted*

Test plan:

1) Make sure you have a db with at least a few deleted biblios,
   biblioitems and items.
2) Apply patch and run updatedatabase.pl
3) Make note of the db upgrade messages, if you get a message like:
   There were x deleteditems that could not be moved, please check _deleteditems
   you should check the _deleteditems table and verify that they are
   actual conflicts
4) Click around in the web interface and make sure you can delete items
   and biblios
5) "Undelete" an item by NULLing the deleted_at column for an item
   and verify that it returns in interface (Not neccessary really)
Comment 57 Benjamin Rokseth 2018-04-13 07:36:05 UTC
Created attachment 74136 [details] [review]
Bug 20271: fix remaining tests
Comment 58 Benjamin Rokseth 2018-04-13 07:36:11 UTC
Created attachment 74137 [details] [review]
Bug 20271: update kohastructure.sql
Comment 59 Benjamin Rokseth 2018-04-13 07:36:16 UTC
Created attachment 74138 [details] [review]
Bug 20271: delete biblio and biblio_metadata in one go
Comment 60 Benjamin Rokseth 2018-04-13 07:36:22 UTC
Created attachment 74139 [details] [review]
Bug 20271: remove comments regarding deleteditems and -biblio*
Comment 61 Benjamin Rokseth 2018-04-13 07:36:28 UTC
Created attachment 74140 [details] [review]
Bug 20271: add atomic update
Comment 62 Benjamin Rokseth 2018-04-13 07:36:33 UTC
Created attachment 74141 [details] [review]
Bug 20271: Use a single pass in the OAI-PMH provider
Comment 63 Benjamin Rokseth 2018-04-13 07:36:39 UTC
Created attachment 74142 [details] [review]
Bug 20271: fix whitespace issues in C4::Items
Comment 64 Benjamin Rokseth 2018-04-14 10:00:05 UTC
Created attachment 74182 [details] [review]
Bug 20271: merging delete biblio/items

The present bug merges the following tables:
deletedbiblio          -> biblio
deletedbiblioitems     -> biblioitems
deletedbiblio_metadata -> biblio_metadata
deleteditems           -> items

and adds a column deleted_at to signify time of deletion, if deleted

it replaces all occurrences of the mentioned occurences and uses
of mentioned deleted* tables

upgrade is handled by moving rows from deleted* tables to the live
ones. If all rows are moved, the corresponding tables are dropped.
If there are duplicates or conflicts (duplicate barcodes e.g.) they
reminders are left for scrutiny and table renamed to _deleted*

Test plan:

1) Make sure you have a db with at least a few deleted biblios,
   biblioitems and items.
2) Apply patch and run updatedatabase.pl
3) Make note of the db upgrade messages, if you get a message like:
   There were x deleteditems that could not be moved, please check _deleteditems
   you should check the _deleteditems table and verify that they are
   actual conflicts
4) Click around in the web interface and make sure you can delete items
   and biblios
5) "Undelete" an item by NULLing the deleted_at column for an item
   and verify that it returns in interface (Not neccessary really)
Comment 65 Benjamin Rokseth 2018-04-14 10:00:10 UTC
Created attachment 74183 [details] [review]
Bug 20271: fix remaining tests
Comment 66 Benjamin Rokseth 2018-04-14 10:00:16 UTC
Created attachment 74184 [details] [review]
Bug 20271: update kohastructure.sql
Comment 67 Benjamin Rokseth 2018-04-14 10:00:21 UTC
Created attachment 74185 [details] [review]
Bug 20271: delete biblio and biblio_metadata in one go
Comment 68 Benjamin Rokseth 2018-04-14 10:00:27 UTC
Created attachment 74186 [details] [review]
Bug 20271: remove comments regarding deleteditems and -biblio*
Comment 69 Benjamin Rokseth 2018-04-14 10:00:32 UTC
Created attachment 74187 [details] [review]
Bug 20271: add atomic update

Bug 20271: disable foreign keys on deletedbiblio_metadata
Comment 70 Benjamin Rokseth 2018-04-14 10:00:38 UTC
Created attachment 74188 [details] [review]
Bug 20271: Use a single pass in the OAI-PMH provider
Comment 71 Benjamin Rokseth 2018-04-14 10:00:44 UTC
Created attachment 74189 [details] [review]
Bug 20271: fix whitespace issues in C4::Items
Comment 72 Benjamin Rokseth 2018-04-14 17:40:47 UTC
Created attachment 74190 [details] [review]
Bug 20271: print instead of warn unmovable items
Comment 73 Benjamin Rokseth 2018-04-16 09:08:05 UTC
Created attachment 74223 [details] [review]
Bug 20271: atomicupdate - support mysql < 5.7 and address foreign key issues
Comment 74 Brendan Gallagher 2018-04-18 06:41:10 UTC
Created attachment 74407 [details] [review]
Bug 20271: merging delete biblio/items

The present bug merges the following tables:
deletedbiblio          -> biblio
deletedbiblioitems     -> biblioitems
deletedbiblio_metadata -> biblio_metadata
deleteditems           -> items

and adds a column deleted_at to signify time of deletion, if deleted

it replaces all occurrences of the mentioned occurences and uses
of mentioned deleted* tables

upgrade is handled by moving rows from deleted* tables to the live
ones. If all rows are moved, the corresponding tables are dropped.
If there are duplicates or conflicts (duplicate barcodes e.g.) they
reminders are left for scrutiny and table renamed to _deleted*

Test plan:

1) Make sure you have a db with at least a few deleted biblios,
   biblioitems and items.
2) Apply patch and run updatedatabase.pl
3) Make note of the db upgrade messages, if you get a message like:
   There were x deleteditems that could not be moved, please check _deleteditems
   you should check the _deleteditems table and verify that they are
   actual conflicts
4) Click around in the web interface and make sure you can delete items
   and biblios
5) "Undelete" an item by NULLing the deleted_at column for an item
   and verify that it returns in interface (Not neccessary really)

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Worked on two different databases.
Comment 75 Brendan Gallagher 2018-04-18 06:41:20 UTC
Created attachment 74408 [details] [review]
Bug 20271: fix remaining tests

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Comment 76 Brendan Gallagher 2018-04-18 06:41:24 UTC
Created attachment 74409 [details] [review]
Bug 20271: update kohastructure.sql

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Comment 77 Brendan Gallagher 2018-04-18 06:41:28 UTC
Created attachment 74410 [details] [review]
Bug 20271: delete biblio and biblio_metadata in one go

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Comment 78 Brendan Gallagher 2018-04-18 06:41:32 UTC
Created attachment 74411 [details] [review]
Bug 20271: remove comments regarding deleteditems and -biblio*

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Comment 79 Brendan Gallagher 2018-04-18 06:41:36 UTC
Created attachment 74412 [details] [review]
Bug 20271: add atomic update

Bug 20271: disable foreign keys on deletedbiblio_metadata

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Comment 80 Brendan Gallagher 2018-04-18 06:41:40 UTC
Created attachment 74413 [details] [review]
Bug 20271: Use a single pass in the OAI-PMH provider

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Comment 81 Brendan Gallagher 2018-04-18 06:41:44 UTC
Created attachment 74414 [details] [review]
Bug 20271: fix whitespace issues in C4::Items

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Comment 82 Brendan Gallagher 2018-04-18 06:41:48 UTC
Created attachment 74415 [details] [review]
Bug 20271: print instead of warn unmovable items

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Comment 83 Brendan Gallagher 2018-04-18 06:41:52 UTC
Created attachment 74416 [details] [review]
Bug 20271: atomicupdate - support mysql < 5.7 and address foreign key issues

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Comment 84 Marcel de Rooy 2018-05-11 09:41:16 UTC
Looking here now, but I feel that this patch set will not make it anymore to 18.05.
Comment 85 Marcel de Rooy 2018-05-11 10:19:01 UTC
Created attachment 75265 [details] [review]
Bug 20271: (follow-up) DBIx schema changes
Comment 86 Marcel de Rooy 2018-05-11 10:23:04 UTC
Great job, Francesco
I did not finish my QA now, but want to report some issues I found already. And they unfortunately confirm that this patch set will not make it anymore into 18.05 ;)
But it would be good to have in master early.

qa tools: FAIL   Koha/Patron.pm, FAIL   pod, Spurious text after =cut
deleted_at should probably be better: deleted_on; we also have updated_on etc.
$dbh->do( "ALTER TABLE biblio ADD COLUMN deleted_at datetime DEFAULT NULL" ) or warn $DBI::errstr;
Probably it warns already and you should die here ?
Running OAI/Server.t
ok 12 - use Koha::OAI::Server::ResumptionToken;
DBD::mysql::db do failed: Table 'koha_dev.deletedbiblio' doesn't exist [for Statement "DELETE FROM deletedbiblio"] at t/db_dependent/OAI/Server.t line 68.
DBD::mysql::db do failed: Table 'koha_dev.deletedbiblioitems' doesn't exist [for Statement "DELETE FROM deletedbiblioitems"] at t/db_dependent/OAI/Server.t line 69.
DBD::mysql::db do failed: Table 'koha_dev.deleteditems' doesn't exist [for Statement "DELETE FROM deleteditems"] at t/db_dependent/OAI/Server.t line 70.
git grep deletedbiblio
about.pl:        q|select b.biblionumber from biblio b join deletedbiblio db on b.biblionumber=db.biblionumber|,
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref (a few other refs too)
git grep deleteditems
about.pl again
koha-tmpl/intranet-tmpl/prog/en/modules/reports/catalogue_stats.tt:                <td><input type="radio" name="Line" value="deleteditems.timestamp" /></td>
might not be a problem?
misc/export_records.pl:            WHERE deleteditems.biblionumber = ? AND deleted_at IS NOT NULL
does not look good
reports/catalogue_stats.pl:    $linefilter[0] = @$filters[15] if ( $line =~ /deleteditems\.timestamp/ );
we should probably get rid of those things; they are confusing now
Comment 87 Marcel de Rooy 2018-05-11 10:25:17 UTC
I am not sure if we should mark this one as critical btw. I would go for major when viewing it as a bugfix. But enh could be argued too.
Not so sure if the RMaints want to backport this one?
Comment 88 Benjamin Rokseth 2018-05-11 10:56:43 UTC
Marcel, thanks for thorough feedback!
will address the issues asap so it gets into master sooner than later.
Downgraded to major, enhancement it is definitely not ;)
Comment 89 Benjamin Rokseth 2018-05-15 21:39:30 UTC
Created attachment 75347 [details] [review]
Bug 20271: (QA follow-up): rename to deleted_on and various fixes

- renamed deleted_at => deleted_on
- removed failing sql in t/db_dependent/OAI/Server.t
- removed deprecated autoincrement checks in about.pl
- fixed errors in catalogue_stats.tt and catalogue_stats.pl
Comment 90 Benjamin Rokseth 2018-05-15 21:43:07 UTC
Adjusted to all qa feedback, except:
- left warn instead of die in atomicupdate - it would be removed by RM when moved to updatedatabase.pl anyways I suppose
- qa tools: FAIL   Koha/Patron.pm, FAIL   pod, Spurious text after =cut
I could not reproduce. Ran qa tools and it passed.
Comment 91 Marcel de Rooy 2018-05-25 07:48:27 UTC
about.pl has compilation errors now.
Look at this line:

if ( @$patrons or @$biblios or @$items or @$checkouts or @$holds ) {

You removed $biblios and $items..
Comment 92 Marcel de Rooy 2018-06-08 06:48:21 UTC
QA: Revisiting this one again
Comment 93 Marcel de Rooy 2018-06-08 06:52:31 UTC Comment hidden (obsolete)
Comment 94 Marcel de Rooy 2018-06-08 07:55:19 UTC
Created attachment 75897 [details] [review]
Bug 20271: (QA follow-up): rename to deleted_on and various fixes

- renamed deleted_at => deleted_on
- removed failing sql in t/db_dependent/OAI/Server.t
- removed deprecated autoincrement checks in about.pl
- fixed errors in catalogue_stats.tt and catalogue_stats.pl

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Removed the DBIx schema changes. Please keep them separated.
We missed changes for Biblioitem btw. Adding them in follow-up.
Comment 95 Marcel de Rooy 2018-06-08 07:55:25 UTC
Created attachment 75898 [details] [review]
Bug 20271: (QA follow-up) DBIx schema changes

Please separate these changes from the other patches.
Note that Biblioitem was not in the last set of changes too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 96 Marcel de Rooy 2018-06-08 07:55:30 UTC
Created attachment 75899 [details] [review]
Bug 20271: (QA follow-up) Additional about cleanup

Still found references to biblios and items in script and template.

Test plan:
Check about page again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 97 Marcel de Rooy 2018-06-08 07:55:51 UTC
Not ready yet !
Comment 98 Marcel de Rooy 2018-06-08 08:37:25 UTC
Created attachment 75910 [details] [review]
Bug 20271: (QA follow-up) Remove last occurrences of deleted_at

Trivial replace of deleted_at by deleted_on.

Test plan:
Run t/db_dependent/Koha/ItemTypes.t
Run t/db_dependent/Items_DelItemCheck.t
git grep -E "deleted_at\W"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 99 Marcel de Rooy 2018-06-08 09:17:24 UTC
t/db_dependent/Search.t
    not ok 44 - All records have at least one item available
    #   Failed test 'All records have at least one item available'
    #   at t/db_dependent/Search.t line 508.
    #          got: 'false'
    #     expected: 'true'

Pass on fresh db, so needs some digging..
Comment 100 Marcel de Rooy 2018-06-08 10:13:27 UTC
Asked QA team for additional feedback; cant finish this one now.
Comment 101 Martin Renvoize 2018-06-12 10:58:35 UTC
(In reply to Marcel de Rooy from comment #86)

> deleted_at should probably be better: deleted_on; we also have updated_on

Not sure I agree with the logic here.. I would say the field within the patron (borrowers) table is misnamed rather than the other way around.. _on suggests date only precision whereas _at suggests a higher precision including time.  As the field is a datetime/timestamp rather than a simple date I'd go with _at here personally.
Comment 102 Martin Renvoize 2018-06-12 12:39:01 UTC
`installer/data/mysql/sysprefs.sql` still references deletedbiblio in the OAI-PMH:DeletedRecord preference.
Comment 103 Martin Renvoize 2018-06-12 13:52:00 UTC
I'm going to let the _at vs _on piece pass as although I feel both set should use _at rather than _on in the name of consistency _on is OK and switching to _at could be done consistently at a later date.

However.. the atomicupdate is not currently idempotent... I'll convert it as a QA followup.
Comment 104 Martin Renvoize 2018-06-12 14:17:30 UTC
Created attachment 75977 [details] [review]
Bug 20271: merging delete biblio/items

The present bug merges the following tables:
deletedbiblio          -> biblio
deletedbiblioitems     -> biblioitems
deletedbiblio_metadata -> biblio_metadata
deleteditems           -> items

and adds a column deleted_at to signify time of deletion, if deleted

it replaces all occurrences of the mentioned occurences and uses
of mentioned deleted* tables

upgrade is handled by moving rows from deleted* tables to the live
ones. If all rows are moved, the corresponding tables are dropped.
If there are duplicates or conflicts (duplicate barcodes e.g.) they
reminders are left for scrutiny and table renamed to _deleted*

Test plan:

1) Make sure you have a db with at least a few deleted biblios,
   biblioitems and items.
2) Apply patch and run updatedatabase.pl
3) Make note of the db upgrade messages, if you get a message like:
   There were x deleteditems that could not be moved, please check _deleteditems
   you should check the _deleteditems table and verify that they are
   actual conflicts
4) Click around in the web interface and make sure you can delete items
   and biblios
5) "Undelete" an item by NULLing the deleted_at column for an item
   and verify that it returns in interface (Not neccessary really)

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 105 Martin Renvoize 2018-06-12 14:17:34 UTC
Created attachment 75978 [details] [review]
Bug 20271: fix remaining tests

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 106 Martin Renvoize 2018-06-12 14:17:39 UTC
Created attachment 75979 [details] [review]
Bug 20271: update kohastructure.sql

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 107 Martin Renvoize 2018-06-12 14:17:43 UTC
Created attachment 75980 [details] [review]
Bug 20271: delete biblio and biblio_metadata in one go

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 108 Martin Renvoize 2018-06-12 14:17:48 UTC
Created attachment 75981 [details] [review]
Bug 20271: remove comments regarding deleteditems and -biblio*

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 109 Martin Renvoize 2018-06-12 14:17:53 UTC
Created attachment 75982 [details] [review]
Bug 20271: add atomic update

Bug 20271: disable foreign keys on deletedbiblio_metadata

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 110 Martin Renvoize 2018-06-12 14:17:58 UTC
Created attachment 75983 [details] [review]
Bug 20271: Use a single pass in the OAI-PMH provider

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 111 Martin Renvoize 2018-06-12 14:18:03 UTC
Created attachment 75984 [details] [review]
Bug 20271: fix whitespace issues in C4::Items

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 112 Martin Renvoize 2018-06-12 14:18:08 UTC
Created attachment 75985 [details] [review]
Bug 20271: print instead of warn unmovable items

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 113 Martin Renvoize 2018-06-12 14:18:12 UTC
Created attachment 75986 [details] [review]
Bug 20271: atomicupdate - support mysql < 5.7 and address foreign key issues

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 114 Martin Renvoize 2018-06-12 14:18:17 UTC
Created attachment 75987 [details] [review]
Bug 20271: (follow-up) DBIx schema changes

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 115 Martin Renvoize 2018-06-12 14:18:22 UTC
Created attachment 75988 [details] [review]
Bug 20271: (QA follow-up): rename to deleted_on and various fixes

- renamed deleted_at => deleted_on
- removed failing sql in t/db_dependent/OAI/Server.t
- removed deprecated autoincrement checks in about.pl
- fixed errors in catalogue_stats.tt and catalogue_stats.pl

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Removed the DBIx schema changes. Please keep them separated.
We missed changes for Biblioitem btw. Adding them in follow-up.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 116 Martin Renvoize 2018-06-12 14:18:27 UTC
Created attachment 75989 [details] [review]
Bug 20271: (QA follow-up) DBIx schema changes

Please separate these changes from the other patches.
Note that Biblioitem was not in the last set of changes too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 117 Martin Renvoize 2018-06-12 14:18:31 UTC
Created attachment 75990 [details] [review]
Bug 20271: (QA follow-up) Additional about cleanup

Still found references to biblios and items in script and template.

Test plan:
Check about page again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 118 Martin Renvoize 2018-06-12 14:18:36 UTC
Created attachment 75991 [details] [review]
Bug 20271: (QA follow-up) Remove last occurrences of deleted_at

Trivial replace of deleted_at by deleted_on.

Test plan:
Run t/db_dependent/Koha/ItemTypes.t
Run t/db_dependent/Items_DelItemCheck.t
git grep -E "deleted_at\W"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 119 Martin Renvoize 2018-06-12 14:18:40 UTC
Created attachment 75992 [details] [review]
Bug 20271: (QA follow-up) Reword OAI-PMH:DeletedRecord systempref

The preference still refered to the deletedbiblio table made defunct by
this patchset.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 120 Martin Renvoize 2018-06-12 14:18:45 UTC
Created attachment 75993 [details] [review]
Bug 20271: (QA follow-up) Make the atomicupdate idempotent

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 121 Martin Renvoize 2018-06-12 14:21:04 UTC
I'm pretty happy with all this now. Well done to marcelr for the first QA run, he did a great job and made my life much easier to get it over the finishing line.

Please note that the manual should get a corresponding minor patch to update the OAI reference to the  deletedbiblio table that this removes: https://koha-community.org/manual/17.11/en/html/02_administration.html?highlight=oai

Passing QA
Comment 122 Petter Goksøyr Åsen 2018-06-14 06:11:43 UTC
Thanks a lot to the QA team!

I found an issue now: we forgot to update CanBookBeIssued to check if item is deleted. So ATM it is possible to check out a deleted book (but it cannot be checked in again). We could either return UNKNOWN_BARCODE from CanBookBeIssued, or add a separate error code DELETED?. Opened a new bug for this, see bug #20940
Comment 123 Martin Renvoize 2018-06-14 08:07:30 UTC
Ack, How did I miss that!.. I'm going to give this a third pass today to make sure I've not missed anything else :(

In related news, I've added my proposed manual update as a merge request here https://gitlab.com/koha-community/koha-manual/merge_requests/135/diffs
Comment 124 Martin Renvoize 2018-06-14 10:30:18 UTC
(In reply to Petter Goksøyr Åsen from comment #122)
> Thanks a lot to the QA team!
> 
> I found an issue now: we forgot to update CanBookBeIssued to check if item
> is deleted. So ATM it is possible to check out a deleted book (but it cannot
> be checked in again). We could either return UNKNOWN_BARCODE from
> CanBookBeIssued, or add a separate error code DELETED?. Opened a new bug for
> this, see bug #20940

By that logic, we should also add handling to CanBookBeRenewed ;)
Comment 125 Martin Renvoize 2018-06-14 10:33:46 UTC
:( Looks like a missed a whole swathe of issues now I dig into this.

I believe we should NOT be changing the expected return of GetItem (i.e deleted items should not be returned from GetItem) to stay compliant with existing functions. I can't believe I missed this in the changed test file :(.  We are not consistently dealing with deleted_on after every invocation of GetItem currently and as such this patchset currently introduces a fairly significant number of untested bugs :(.

Thanks to Petter for spotting the issue that lead me to realise the extent of my failings as QA here :(.
Comment 126 Martin Renvoize 2018-06-14 10:44:52 UTC
Created attachment 76047 [details] [review]
Bug 20271: Restore original GetItem functionality

The return value of GetItem had been changed (as it would now return
deleted items as well as current items). This patch restores the
original functionality (as we've not catered for all calls to GetItem
independantly), restores the original test as well as adding a further
test to catch the correct setting of deleted_on by DelItem
Comment 127 Martin Renvoize 2018-06-14 11:04:58 UTC
Created attachment 76048 [details] [review]
Bug 20271: (QA follow-up) Restore original GetItem functionality

The return value of GetItem had been changed (as it would now return
deleted items as well as current items). This patch restores the
original functionality (as we've not catered for all calls to GetItem
independantly), restores the original test as well as adding a further
test to catch the correct setting of deleted_on by DelItem
Comment 128 Martin Renvoize 2018-06-14 11:10:42 UTC
C4::Items::ModItem should probably not be allowed to alter deleted_on as we have a DelItem routine.. save a future misuse.
Comment 129 Katrin Fischer 2018-06-19 05:56:05 UTC
Apart from the other issues that still need fixing, is there a way we could run a performance test on this? I know we got libraries around with many years (10+) of circulation data in these tables. Could this chance cause performance issues?
Comment 130 Martin Renvoize 2018-06-19 06:59:50 UTC
That'll be interesting to test.. the db update is certainly going to be painful at the very least.. it would be good to see if there's anything we could do about that.
Comment 131 Benjamin Rokseth 2018-06-20 08:33:47 UTC
Hi, regarding performance and db update/migrate:
we have it in production now and tested quite abit up front.

db migrate took ~40sec as I recall (1,5M items and 400k biblios) which is far less than the updates of statistics column took, for instance. We had 830 duplicate items that could not be moved. 

regarding performance there is nothing in this patchset that will degrade performance, unless, say, you have a report query for all items, which now would need to add 'WHERE deleted_on IS NULL' not to include deleted items

on the other hand, union queries of items and deleteditems are no longer neccessary so statistical queries will be faster

When we move on to merge issues and reserves, the question is more relevant, as 10 years of checkout history might lead to 100M+ rows, but still, MySql is capable of billions, and at that point its more a matter of indexes, RAM and physics.
Comment 132 Katrin Fischer 2018-06-20 08:48:49 UTC
Hi Benjamin, you are right, I was mostly thinking about issues/old_issues. But another question: When we moved marcxml we managed to add a nice conversion tool for the reports that seems to work quite well. Could we do the same for those tables? I think detecting the problematic reports and highlighting them should be no issue, converting might be harder.
Comment 133 Martin Renvoize 2018-06-21 06:59:51 UTC
I've done another round of testing and I'm reasonably happy that everything is caught now.. though I was happy the first time.. Hoping to get another pair of QA eye's on it and awaiting a followup with at least a warning during the update database about reports needing to change if not a more thorough handling to convert the simple cases automagically.
Comment 134 Josef Moravec 2018-06-22 08:35:02 UTC
I started to look in to it too now
Comment 135 Josef Moravec 2018-06-22 08:37:24 UTC
Created attachment 76255 [details] [review]
Bug 20271: (QA follow-up) Fix catalogue statistics in reports module

Test plan:
1) Have some deleted and some not deleted items
2) Use catalogue statistics in reports module and try to filter
total/deleted items
--> without this patch the deleted options returns not deleted items and
total returns deleted items
--> with this patch you should be able to filter the results to deleted,
    not deleted and all items and everyone of this filter should work

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 136 Josef Moravec 2018-06-22 10:06:57 UTC
Created attachment 76256 [details] [review]
Bug 20271: merging delete biblio/items

The present bug merges the following tables:
deletedbiblio          -> biblio
deletedbiblioitems     -> biblioitems
deletedbiblio_metadata -> biblio_metadata
deleteditems           -> items

and adds a column deleted_at to signify time of deletion, if deleted

it replaces all occurrences of the mentioned occurences and uses
of mentioned deleted* tables

upgrade is handled by moving rows from deleted* tables to the live
ones. If all rows are moved, the corresponding tables are dropped.
If there are duplicates or conflicts (duplicate barcodes e.g.) they
reminders are left for scrutiny and table renamed to _deleted*

Test plan:

1) Make sure you have a db with at least a few deleted biblios,
   biblioitems and items.
2) Apply patch and run updatedatabase.pl
3) Make note of the db upgrade messages, if you get a message like:
   There were x deleteditems that could not be moved, please check _deleteditems
   you should check the _deleteditems table and verify that they are
   actual conflicts
4) Click around in the web interface and make sure you can delete items
   and biblios
5) "Undelete" an item by NULLing the deleted_at column for an item
   and verify that it returns in interface (Not neccessary really)

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 137 Josef Moravec 2018-06-22 10:07:03 UTC
Created attachment 76257 [details] [review]
Bug 20271: fix remaining tests

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 138 Josef Moravec 2018-06-22 10:07:09 UTC
Created attachment 76258 [details] [review]
Bug 20271: update kohastructure.sql

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 139 Josef Moravec 2018-06-22 10:07:14 UTC
Created attachment 76259 [details] [review]
Bug 20271: delete biblio and biblio_metadata in one go

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 140 Josef Moravec 2018-06-22 10:07:20 UTC
Created attachment 76260 [details] [review]
Bug 20271: remove comments regarding deleteditems and -biblio*

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 141 Josef Moravec 2018-06-22 10:07:25 UTC
Created attachment 76261 [details] [review]
Bug 20271: add atomic update

Bug 20271: disable foreign keys on deletedbiblio_metadata

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 142 Josef Moravec 2018-06-22 10:07:31 UTC
Created attachment 76262 [details] [review]
Bug 20271: Use a single pass in the OAI-PMH provider

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 143 Josef Moravec 2018-06-22 10:07:36 UTC
Created attachment 76263 [details] [review]
Bug 20271: fix whitespace issues in C4::Items

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 144 Josef Moravec 2018-06-22 10:07:42 UTC
Created attachment 76264 [details] [review]
Bug 20271: print instead of warn unmovable items

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 145 Josef Moravec 2018-06-22 10:07:47 UTC
Created attachment 76265 [details] [review]
Bug 20271: atomicupdate - support mysql < 5.7 and address foreign key issues

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 146 Josef Moravec 2018-06-22 10:07:52 UTC
Created attachment 76266 [details] [review]
Bug 20271: (follow-up) DBIx schema changes

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 147 Josef Moravec 2018-06-22 10:07:59 UTC
Created attachment 76267 [details] [review]
Bug 20271: (QA follow-up): rename to deleted_on and various fixes

- renamed deleted_at => deleted_on
- removed failing sql in t/db_dependent/OAI/Server.t
- removed deprecated autoincrement checks in about.pl
- fixed errors in catalogue_stats.tt and catalogue_stats.pl

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Removed the DBIx schema changes. Please keep them separated.
We missed changes for Biblioitem btw. Adding them in follow-up.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 148 Josef Moravec 2018-06-22 10:08:04 UTC
Created attachment 76268 [details] [review]
Bug 20271: (QA follow-up) DBIx schema changes

Please separate these changes from the other patches.
Note that Biblioitem was not in the last set of changes too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 149 Josef Moravec 2018-06-22 10:08:09 UTC
Created attachment 76269 [details] [review]
Bug 20271: (QA follow-up) Additional about cleanup

Still found references to biblios and items in script and template.

Test plan:
Check about page again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 150 Josef Moravec 2018-06-22 10:08:15 UTC
Created attachment 76270 [details] [review]
Bug 20271: (QA follow-up) Remove last occurrences of deleted_at

Trivial replace of deleted_at by deleted_on.

Test plan:
Run t/db_dependent/Koha/ItemTypes.t
Run t/db_dependent/Items_DelItemCheck.t
git grep -E "deleted_at\W"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 151 Josef Moravec 2018-06-22 10:08:20 UTC
Created attachment 76271 [details] [review]
Bug 20271: (QA follow-up) Reword OAI-PMH:DeletedRecord systempref

The preference still refered to the deletedbiblio table made defunct by
this patchset.

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 152 Josef Moravec 2018-06-22 10:08:26 UTC
Created attachment 76272 [details] [review]
Bug 20271: (QA follow-up) Make the atomicupdate idempotent

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 153 Josef Moravec 2018-06-22 10:08:31 UTC
Created attachment 76273 [details] [review]
Bug 20271: (QA follow-up) Restore original GetItem functionality

The return value of GetItem had been changed (as it would now return
deleted items as well as current items). This patch restores the
original functionality (as we've not catered for all calls to GetItem
independantly), restores the original test as well as adding a further
test to catch the correct setting of deleted_on by DelItem

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 154 Josef Moravec 2018-06-22 10:08:36 UTC
Created attachment 76274 [details] [review]
Bug 20271: (QA follow-up) Fix catalogue statistics in reports module

Test plan:
1) Have some deleted and some not deleted items
2) Use catalogue statistics in reports module and try to filter
total/deleted items
--> without this patch the deleted options returns not deleted items and
total returns deleted items
--> with this patch you should be able to filter the results to deleted,
    not deleted and all items and everyone of this filter should work

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 155 Josef Moravec 2018-06-22 10:08:42 UTC
Created attachment 76275 [details] [review]
Bug 20271: (QA follow-up) Zebra should not index deleted record when doing full reindex

Test plan:
Run koha-rebuild-zebra -f [instance]
--> without patch the deleted records are indexed and shown when
searching
--> with this patch the deleted records are not indexed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 156 Josef Moravec 2018-06-22 10:08:47 UTC
Created attachment 76276 [details] [review]
Bug 20271: (QA follow-up) Does not return deleted record in GetMarcBiblio

Test plan:
Go to url /cgi-bin/koha/catalogue/detail.pl?biblionumber=[deleted
biblionumber]
--> without patch you see the deleted record
--> with patch you should see message "The record you requested does not
exist"

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 157 Josef Moravec 2018-06-22 10:08:53 UTC
Created attachment 76277 [details] [review]
Bug 20271: (QA follow-up): Fix test ModBiblioMarc

Test plan:
prove t/db_dependent/Biblio/ModBiblioMarc.t

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 158 Josef Moravec 2018-06-22 10:10:04 UTC
If somebody could test my follow-ups (last 4) and confirm they are ok, it would be great ;)
Comment 159 Josef Moravec 2018-06-27 13:37:32 UTC
Created attachment 76493 [details] [review]
Bug 20271: merging delete biblio/items

The present bug merges the following tables:
deletedbiblio          -> biblio
deletedbiblioitems     -> biblioitems
deletedbiblio_metadata -> biblio_metadata
deleteditems           -> items

and adds a column deleted_at to signify time of deletion, if deleted

it replaces all occurrences of the mentioned occurences and uses
of mentioned deleted* tables

upgrade is handled by moving rows from deleted* tables to the live
ones. If all rows are moved, the corresponding tables are dropped.
If there are duplicates or conflicts (duplicate barcodes e.g.) they
reminders are left for scrutiny and table renamed to _deleted*

Test plan:

1) Make sure you have a db with at least a few deleted biblios,
   biblioitems and items.
2) Apply patch and run updatedatabase.pl
3) Make note of the db upgrade messages, if you get a message like:
   There were x deleteditems that could not be moved, please check _deleteditems
   you should check the _deleteditems table and verify that they are
   actual conflicts
4) Click around in the web interface and make sure you can delete items
   and biblios
5) "Undelete" an item by NULLing the deleted_at column for an item
   and verify that it returns in interface (Not neccessary really)

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 160 Josef Moravec 2018-06-27 13:37:38 UTC
Created attachment 76494 [details] [review]
Bug 20271: fix remaining tests

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 161 Josef Moravec 2018-06-27 13:37:43 UTC
Created attachment 76495 [details] [review]
Bug 20271: update kohastructure.sql

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 162 Josef Moravec 2018-06-27 13:37:49 UTC
Created attachment 76496 [details] [review]
Bug 20271: delete biblio and biblio_metadata in one go

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 163 Josef Moravec 2018-06-27 13:37:54 UTC
Created attachment 76497 [details] [review]
Bug 20271: remove comments regarding deleteditems and -biblio*

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 164 Josef Moravec 2018-06-27 13:37:59 UTC
Created attachment 76498 [details] [review]
Bug 20271: add atomic update

Bug 20271: disable foreign keys on deletedbiblio_metadata

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 165 Josef Moravec 2018-06-27 13:38:05 UTC
Created attachment 76499 [details] [review]
Bug 20271: Use a single pass in the OAI-PMH provider

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 166 Josef Moravec 2018-06-27 13:38:10 UTC
Created attachment 76500 [details] [review]
Bug 20271: fix whitespace issues in C4::Items

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 167 Josef Moravec 2018-06-27 13:38:15 UTC
Created attachment 76501 [details] [review]
Bug 20271: print instead of warn unmovable items

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 168 Josef Moravec 2018-06-27 13:38:21 UTC
Created attachment 76502 [details] [review]
Bug 20271: atomicupdate - support mysql < 5.7 and address foreign key issues

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 169 Josef Moravec 2018-06-27 13:38:26 UTC
Created attachment 76503 [details] [review]
Bug 20271: (follow-up) DBIx schema changes

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 170 Josef Moravec 2018-06-27 13:38:32 UTC
Created attachment 76504 [details] [review]
Bug 20271: (QA follow-up): rename to deleted_on and various fixes

- renamed deleted_at => deleted_on
- removed failing sql in t/db_dependent/OAI/Server.t
- removed deprecated autoincrement checks in about.pl
- fixed errors in catalogue_stats.tt and catalogue_stats.pl

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Removed the DBIx schema changes. Please keep them separated.
We missed changes for Biblioitem btw. Adding them in follow-up.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 171 Josef Moravec 2018-06-27 13:38:37 UTC
Created attachment 76505 [details] [review]
Bug 20271: (QA follow-up) DBIx schema changes

Please separate these changes from the other patches.
Note that Biblioitem was not in the last set of changes too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 172 Josef Moravec 2018-06-27 13:38:43 UTC
Created attachment 76506 [details] [review]
Bug 20271: (QA follow-up) Additional about cleanup

Still found references to biblios and items in script and template.

Test plan:
Check about page again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 173 Josef Moravec 2018-06-27 13:38:48 UTC
Created attachment 76507 [details] [review]
Bug 20271: (QA follow-up) Remove last occurrences of deleted_at

Trivial replace of deleted_at by deleted_on.

Test plan:
Run t/db_dependent/Koha/ItemTypes.t
Run t/db_dependent/Items_DelItemCheck.t
git grep -E "deleted_at\W"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 174 Josef Moravec 2018-06-27 13:38:53 UTC
Created attachment 76508 [details] [review]
Bug 20271: (QA follow-up) Reword OAI-PMH:DeletedRecord systempref

The preference still refered to the deletedbiblio table made defunct by
this patchset.

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 175 Josef Moravec 2018-06-27 13:38:59 UTC
Created attachment 76509 [details] [review]
Bug 20271: (QA follow-up) Make the atomicupdate idempotent

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 176 Josef Moravec 2018-06-27 13:39:05 UTC
Created attachment 76510 [details] [review]
Bug 20271: (QA follow-up) Restore original GetItem functionality

The return value of GetItem had been changed (as it would now return
deleted items as well as current items). This patch restores the
original functionality (as we've not catered for all calls to GetItem
independantly), restores the original test as well as adding a further
test to catch the correct setting of deleted_on by DelItem

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 177 Josef Moravec 2018-06-27 13:39:10 UTC
Created attachment 76511 [details] [review]
Bug 20271: (QA follow-up) Fix catalogue statistics in reports module

Test plan:
1) Have some deleted and some not deleted items
2) Use catalogue statistics in reports module and try to filter
total/deleted items
--> without this patch the deleted options returns not deleted items and
total returns deleted items
--> with this patch you should be able to filter the results to deleted,
    not deleted and all items and everyone of this filter should work

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 178 Josef Moravec 2018-06-27 13:39:16 UTC
Created attachment 76512 [details] [review]
Bug 20271: (QA follow-up) Zebra should not index deleted record when doing full reindex

Test plan:
Run koha-rebuild-zebra -f [instance]
--> without patch the deleted records are indexed and shown when
searching
--> with this patch the deleted records are not indexed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 179 Josef Moravec 2018-06-27 13:39:21 UTC
Created attachment 76513 [details] [review]
Bug 20271: (QA follow-up) Does not return deleted record in GetMarcBiblio

Test plan:
Go to url /cgi-bin/koha/catalogue/detail.pl?biblionumber=[deleted
biblionumber]
--> without patch you see the deleted record
--> with patch you should see message "The record you requested does not
exist"

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 180 Josef Moravec 2018-06-27 13:39:27 UTC
Created attachment 76514 [details] [review]
Bug 20271: (QA follow-up): Fix test ModBiblioMarc

Test plan:
prove t/db_dependent/Biblio/ModBiblioMarc.t

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 181 Josef Moravec 2018-06-27 13:40:23 UTC
Rebased on master, there was conflict in C4/Items.pm because of bug 20702
Comment 182 Jonathan Druart 2018-06-27 14:49:04 UTC
Quick test:
- check an item out, then check it in
- delete the item

1. on /members/readingrec.pl?borrowernumber=51
a) before "This patron has no circulation history."
b) after we see a table with all item's info

2. on /catalogue/issuehistory.pl?biblionumber=42
a) before "Amy's diary, by Lee has never been checked out."
b) after "Checked out 1 times" with the table (and a barcode with a link to the item)

Same if we remove the bibliographic record.

3. Perform a search, /catalogue/search.pl?q=d
the search result will display "1 item, 1 available" in the "Location" column whereas there are no more items for this bibliographic record.

I did not see anything about these changes in the previous comments, and it seems that we lack a proper test plan.

What about acquisition module? Fines? Authority ("used in X"), etc?
Comment 183 Josef Moravec 2018-06-29 06:58:40 UTC
I do a retest:

(In reply to Jonathan Druart from comment #182)
> Quick test:
> - check an item out, then check it in
> - delete the item
> 
> 1. on /members/readingrec.pl?borrowernumber=51
> a) before "This patron has no circulation history."
> b) after we see a table with all item's info

The same for me, you are right

> 2. on /catalogue/issuehistory.pl?biblionumber=42
> a) before "Amy's diary, by Lee has never been checked out."
> b) after "Checked out 1 times" with the table (and a barcode with a link to
> the item)

You are right

> 
> Same if we remove the bibliographic record.
>

True

> 
> 3. Perform a search, /catalogue/search.pl?q=d
> the search result will display "1 item, 1 available" in the "Location"
> column whereas there are no more items for this bibliographic record.

When I delete an item, it can not be anymore seen in search results - so ok for me.

> I did not see anything about these changes in the previous comments, and it
> seems that we lack a proper test plan.
>
> What about acquisition module? Fines? Authority ("used in X"), etc?

Authority "used in X records" is done by search engine, so  when it works, it gets the correct number - I just tested it and it is ok when i remove record with authority linked.

Fines linked with items remains linked and show the name of item in description - but it is the question if we do wan't to show it or not then - it could be useful information even if the item/biblio no longer "exists".

So I agree, there is more things to test and it is not easy to find all places which should be tested...
Comment 184 Jonathan Druart 2018-06-29 18:48:14 UTC
virtualshelfcontents entries are not deleted when a bibliographic record is removed:
- Add 3 biblios to a list
- Remove one
- /virtualshelves/shelves.pl?op=list&category=1
says "3 item(s)"
- click on the list
=> There are only 2
Comment 185 Katrin Fischer 2018-07-02 06:18:18 UTC
Should we move this to FQA for now?
Comment 186 Marcel de Rooy 2018-07-06 10:22:17 UTC
(In reply to Katrin Fischer from comment #185)
> Should we move this to FQA for now?

Yeah I think so.
Is anyone planning to work on the issues mentioned by Jonathan/Josef ?