[Bug 42842] New: Remove materialized patron data from tmp_holdsqueue
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Bug ID: 42842 Summary: Remove materialized patron data from tmp_holdsqueue Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 42503 Target Milestone: --- Since bug 42503 converted the holds queue page to be API-driven, the materialized patron fields in tmp_holdsqueue (surname, firstname, phone, cardnumber) are no longer used. The page now embeds patron data via the borrowernumber FK through the REST API. These columns should be removed because: 1. They are dead code â nothing reads them after bug 42503 2. They leave patron PII (name, phone, card number) in an unaudited table that is not subject to patron data cleanup/anonymization routines 3. Privacy concern: tmp_holdsqueue is rebuilt periodically but retains patron information between rebuilds, and this data is not covered by GDPR/privacy cleanup tools The fix should: - Remove surname, firstname, phone, cardnumber columns from tmp_holdsqueue - Stop populating them in C4::HoldsQueue::CreatePicklistFromItemMap - Update Koha::Schema::Result::TmpHoldsqueue - Remove the undef mappings in Koha::Hold::HoldsQueueItem::to_api_mapping Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42503 [Bug 42503] Render the holds queue using the REST API -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com | |, katrin.fischer@bsz-bw.de, | |martin.renvoize@openfifth.c | |o.uk, tomascohen@gmail.com Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Status|NEW |ASSIGNED -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200388 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200388&action=edit Bug 42842: Remove materialized patron data from tmp_holdsqueue Since bug 42503, the holds queue page is API-driven and patron data is embedded via the borrowernumber FK. The materialized surname, firstname, phone, and cardnumber columns are no longer read by any code path. Removing them because: - Dead code: nothing reads these columns - Privacy: patron PII was stored in an unaudited table not covered by GDPR/anonymization cleanup tools - Data integrity: values could become stale between queue rebuilds Changes: - Atomicupdate drops the four columns - C4::HoldsQueue::CreatePicklistFromItemMap no longer fetches/inserts patron fields - Koha::Hold::HoldsQueueItem to_api_mapping cleaned up - Tests adjusted to use borrowernumber instead of cardnumber Test plan: 1. Apply all patches 2. Run updatedatabase 3. Verify tmp_holdsqueue no longer has surname/firstname/phone/cardnumber 4. Run: perl misc/cronjobs/holds/build_holds_queue.pl 5. Verify the holds queue page still shows patron info (via API embed) 6. prove t/db_dependent/HoldsQueue/materialized_descriptions.t 7. prove t/db_dependent/api/v1/holds_queue.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200389 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200389&action=edit [DO NOT PUSH] Bug 42842: Update DBIC schema This is for testers only. The release manager will regenerate the schema files after pushing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200388|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #3 from David Nind <david@davidnind.com> --- Created attachment 200437 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200437&action=edit Bug 42842: Remove materialized patron data from tmp_holdsqueue Since bug 42503, the holds queue page is API-driven and patron data is embedded via the borrowernumber FK. The materialized surname, firstname, phone, and cardnumber columns are no longer read by any code path. Removing them because: - Dead code: nothing reads these columns - Privacy: patron PII was stored in an unaudited table not covered by GDPR/anonymization cleanup tools - Data integrity: values could become stale between queue rebuilds Changes: - Atomicupdate drops the four columns - C4::HoldsQueue::CreatePicklistFromItemMap no longer fetches/inserts patron fields - Koha::Hold::HoldsQueueItem to_api_mapping cleaned up - Tests adjusted to use borrowernumber instead of cardnumber Test plan: 1. Apply all patches 2. Run updatedatabase 3. Verify tmp_holdsqueue no longer has surname/firstname/phone/cardnumber 4. Run: perl misc/cronjobs/holds/build_holds_queue.pl 5. Verify the holds queue page still shows patron info (via API embed) 6. prove t/db_dependent/HoldsQueue/materialized_descriptions.t 7. prove t/db_dependent/api/v1/holds_queue.t Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 200438 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200438&action=edit Bug 42842: Update DBIC schema This is for testers only. The release manager will regenerate the schema files after pushing. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |bibliothek@th-wildau.de |y.org | CC| |lisette@bywatersolutions.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #5 from Jan Kissig <bibliothek@th-wildau.de> --- git bz apply fails with error: sha1 information is lacking or useless (Koha/Schema/Result/TmpHoldsqueue.pm). As both patches seem equal, may I omit the second? - [DO NOT PUSH] Bug 42842: Update DBIC schema (https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200389) - Bug 42842: Update DBIC schema (https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200438) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Jan Kissig <bibliothek@th-wildau.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Jan Kissig <bibliothek@th-wildau.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200438|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #6 from Jan Kissig <bibliothek@th-wildau.de> --- Created attachment 202784 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202784&action=edit Bug 42842: (QA follow-up): Fix broken links in circ-nav.inc The page view_holdsqueue got renamed to holds_queue. This patch changes last remaining occurences of view_holdsqueue.pl to holds_queue.pl in 'circ-nav.inc'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #7 from Jan Kissig <bibliothek@th-wildau.de> --- Created attachment 202785 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202785&action=edit Bug 42842: (QA follow-up): Remove patron data from tmp_holdsqueue This removes the remaining columns from kohastructure.sql -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #8 from Jan Kissig <bibliothek@th-wildau.de> --- Created attachment 202786 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202786&action=edit Bug 42842: (QA follow-up): Fix failing test in holds_queue.t In test HidePatronName the builder was fed with now obsolete keys surname,firstname and cardnumber which led to an error. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Jan Kissig <bibliothek@th-wildau.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #202784|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Jan Kissig <bibliothek@th-wildau.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Failed QA --- Comment #9 from Jan Kissig <bibliothek@th-wildau.de> --- (In reply to Jan Kissig from comment #5)
git bz apply fails with error: sha1 information is lacking or useless (Koha/Schema/Result/TmpHoldsqueue.pm).
As both patches seem equal, may I omit the second? - [DO NOT PUSH] Bug 42842: Update DBIC schema (https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200389) - Bug 42842: Update DBIC schema (https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200438)
Hey Tomas, I tested the patch omitting the second DBIC schema change and obsoleted it. Then it applied perfectly. Found 2 issues and attached follow ups. But nevertheless /kohadevbox/koha/t/db_dependent/HoldsQueue.t is still failing with 'DBIx::Class::ResultSource::_resolve_join(): No such relationship borrower on TmpHoldsqueue at /kohadevbox/koha/Koha/Objects.pm line 319' I don't know if this is caused by the depending 42503. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |ASSIGNED --- Comment #10 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Jan Kissig from comment #9)
(In reply to Jan Kissig from comment #5)
git bz apply fails with error: sha1 information is lacking or useless (Koha/Schema/Result/TmpHoldsqueue.pm).
As both patches seem equal, may I omit the second? - [DO NOT PUSH] Bug 42842: Update DBIC schema (https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200389) - Bug 42842: Update DBIC schema (https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200438)
Hey Tomas, I tested the patch omitting the second DBIC schema change and obsoleted it. Then it applied perfectly.
Hi, thanks! I need to resubmit this one as a bad rebase on the working branch left things wrong on both bugs schema files. I fixed the dependency one, but still need to go back here. Will do soon. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200389|0 |1 is obsolete| | Attachment #200437|0 |1 is obsolete| | Attachment #202785|0 |1 is obsolete| | Attachment #202786|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #11 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 202872 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202872&action=edit Bug 42842: Remove materialized patron data from tmp_holdsqueue Since bug 42503, the holds queue page is API-driven and patron data is embedded via the borrowernumber FK. The materialized surname, firstname, phone, and cardnumber columns are no longer read by any code path. Removing them because: - Dead code: nothing reads these columns - Privacy: patron PII was stored in an unaudited table not covered by GDPR/anonymization cleanup tools - Data integrity: values could become stale between queue rebuilds Changes: - Atomicupdate drops the four columns - C4::HoldsQueue::CreatePicklistFromItemMap no longer fetches/inserts patron fields - Koha::Hold::HoldsQueueItem to_api_mapping cleaned up - Tests adjusted to use borrowernumber instead of cardnumber Test plan: 1. Apply all patches 2. Run updatedatabase 3. Verify tmp_holdsqueue no longer has surname/firstname/phone/cardnumber 4. Run: perl misc/cronjobs/holds/build_holds_queue.pl 5. Verify the holds queue page still shows patron info (via API embed) 6. prove t/db_dependent/HoldsQueue/materialized_descriptions.t 7. prove t/db_dependent/api/v1/holds_queue.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #12 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 202873 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202873&action=edit Bug 42842: DBIC schema [DO NOT PUSH] This is for testers only. The release manager will regenerate the schema files after pushing. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #13 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 202874 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202874&action=edit Bug 42842: (QA follow-up): Fix failing test in holds_queue.t In test HidePatronName the builder was fed with now obsolete keys surname,firstname and cardnumber which led to an error. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #14 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Jan Kissig from comment #9)
Hey Tomas, I tested the patch omitting the second DBIC schema change and obsoleted it. Then it applied perfectly.
Found 2 issues and attached follow ups.
Hi. As I said earlier, a poor rebase on my worktree misaligned things and the kohastructure.sql changes got lost. My bad. I have refactored bug 42503 and this one on top of the changes from bug 41410. I kept your tests fix as I overlooked that one when writing this. Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Jan Kissig <bibliothek@th-wildau.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Jan Kissig <bibliothek@th-wildau.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #202872|0 |1 is obsolete| | Attachment #202873|0 |1 is obsolete| | Attachment #202874|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #15 from Jan Kissig <bibliothek@th-wildau.de> --- Created attachment 202883 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202883&action=edit Bug 42842: Remove materialized patron data from tmp_holdsqueue Since bug 42503, the holds queue page is API-driven and patron data is embedded via the borrowernumber FK. The materialized surname, firstname, phone, and cardnumber columns are no longer read by any code path. Removing them because: - Dead code: nothing reads these columns - Privacy: patron PII was stored in an unaudited table not covered by GDPR/anonymization cleanup tools - Data integrity: values could become stale between queue rebuilds Changes: - Atomicupdate drops the four columns - C4::HoldsQueue::CreatePicklistFromItemMap no longer fetches/inserts patron fields - Koha::Hold::HoldsQueueItem to_api_mapping cleaned up - Tests adjusted to use borrowernumber instead of cardnumber Test plan: 1. Apply all patches 2. Run updatedatabase 3. Verify tmp_holdsqueue no longer has surname/firstname/phone/cardnumber 4. Run: perl misc/cronjobs/holds/build_holds_queue.pl 5. Verify the holds queue page still shows patron info (via API embed) 6. prove t/db_dependent/HoldsQueue/materialized_descriptions.t 7. prove t/db_dependent/api/v1/holds_queue.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jan Kissig <bibliothek@th-wildau.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #16 from Jan Kissig <bibliothek@th-wildau.de> --- Created attachment 202884 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202884&action=edit Bug 42842: DBIC schema [DO NOT PUSH] This is for testers only. The release manager will regenerate the schema files after pushing. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jan Kissig <bibliothek@th-wildau.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 --- Comment #17 from Jan Kissig <bibliothek@th-wildau.de> --- Created attachment 202885 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202885&action=edit Bug 42842: (QA follow-up): Fix failing test in holds_queue.t In test HidePatronName the builder was fed with now obsolete keys surname,firstname and cardnumber which led to an error. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |BLOCKED CC| |pedro.amorim@openfifth.co.u | |k --- Comment #18 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Moving this out of the queue. This depends on bug 42503 and that hasn't been pushed / PQA yet. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|katrin.fischer@bsz-bw.de | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42842 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org