[Bug 16966] New: Koha:: Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Bug ID: 16966 Summary: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Depends on: 16961, 16965 Blocks: 16846 The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed to return the patrons with an issue history older than a given date. It would make more sense to return a list of Koha::Patrons. On the way, the code from AnonymiseIssueHistory will be moved as well to anonymise_issue_history. Note that these 2 subroutines are strongly linked: one is used to know the number of patrons we will anonymise the history, the other one is used to anonymise the issues history. The problem is that the first one is not used to do the action, but only for displayed purpose. In some cases, these 2 values can differ, which could be confusing. Case 1: The logged in librarian is not superlibrarian and IndependentBranches is set: if 2+ patrons from different libraries match the date parameter, the interface will display "Checkout history for 2 patrons will be anonymized", when actually only 1 will be. Case 2: If 2+ patrons match the date parameter but one of them has his privacy set to forever (privacy=0), the same issue will appear. The patches provided will refactor the code and fix these 2 issues. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16846 [Bug 16846] Move patron related code to Koha::Patron https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16961 [Bug 16961] Add the Koha::Objects->update method https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16965 [Bug 16965] Add the Koha::Objects->search_related method -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=16966 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 53658 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53658&action=edit Bug 16966: GetBorrowersWithIssuesHistoryOlderThan - Highlight the existing issue in tests If IndependentBranches is set, the code is buggy. This patch only highlight the bug by providing a test. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 53659 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53659&action=edit Bug 16966: Koha::Patrons - Move AnonymiseIssueHistory tests The next patch will move C4::Circulation::AnonymiseIssueHistory and C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons This patch move the history anonymisation code to the Patrons.t test file and the entire subtest related to StoreLastBorrower to StoreLastBorrower.t It just moves and add some minor adjustements. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 53660 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53660&action=edit Bug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed to return the patrons with an issue history older than a given date. It would make more sense to return a list of Koha::Patrons. On the way, the code from AnonymiseIssueHistory will be moved as well to anonymise_issue_history. Note that these 2 subroutines are strongly linked: one is used to know the number of patrons we will anonymise the history, the other one is used to anonymise the issues history. The problem is that the first one is not used to do the action, but only for displayed purpose. In some cases, these 2 values can differ, which could be confusing. Case 1: The logged in librarian is not superlibrarian and IndependentBranches is set: if 2+ patrons from different libraries match the date parameter, the interface will display "Checkout history for 2 patrons will be anonymized", when actually only 1 will be. Case 2: If 2+ patrons match the date parameter but one of them has his privacy set to forever (privacy=0), the same issue will appear. This patch moves the code from C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons->search_patrons_to_anonymise and from C4::Circulation::AnonymiseIssueHistory to Koha::Patrons->anonymise_issue_history Test plan: 1/ Confirm the 2 issues and make sure they are fixed using the Batch patron anonymization tool (tools/cleanborrowers.pl) 2/ At the OPAC, use the 'Immediate deletion' button to delete all your reading history (regardless the setting of the privacy rule) 3/ Use the cronjob script (misc/cronjobs/batch_anonymise.pl) to anonymise patrons. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #53660|0 |1 is obsolete| | --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 53661 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53661&action=edit Bug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed to return the patrons with an issue history older than a given date. It would make more sense to return a list of Koha::Patrons. On the way, the code from AnonymiseIssueHistory will be moved as well to anonymise_issue_history. Note that these 2 subroutines are strongly linked: one is used to know the number of patrons we will anonymise the history, the other one is used to anonymise the issues history. The problem is that the first one is not used to do the action, but only for displayed purpose. In some cases, these 2 values can differ, which could be confusing. Case 1: The logged in librarian is not superlibrarian and IndependentBranches is set: if 2+ patrons from different libraries match the date parameter, the interface will display "Checkout history for 2 patrons will be anonymized", when actually only 1 will be. Case 2: If 2+ patrons match the date parameter but one of them has his privacy set to forever (privacy=0), the same issue will appear. This patch moves the code from C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons->search_patrons_to_anonymise and from C4::Circulation::AnonymiseIssueHistory to Koha::Patrons->anonymise_issue_history Test plan: 1/ Confirm the 2 issues and make sure they are fixed using the Batch patron anonymization tool (tools/cleanborrowers.pl) 2/ At the OPAC, use the 'Immediate deletion' button to delete all your reading history (regardless the setting of the privacy rule) 3/ Use the cronjob script (misc/cronjobs/batch_anonymise.pl) to anonymise patrons. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=9944 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #53658|0 |1 is obsolete| | Attachment #53659|0 |1 is obsolete| | Attachment #53661|0 |1 is obsolete| | --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56376 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56376&action=edit Bug 16966: GetBorrowersWithIssuesHistoryOlderThan - Highlight the existing issue in tests If IndependentBranches is set, the code is buggy. This patch only highlight the bug by providing a test. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56377 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56377&action=edit Bug 16966: Koha::Patrons - Move AnonymiseIssueHistory tests The next patch will move C4::Circulation::AnonymiseIssueHistory and C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons This patch move the history anonymisation code to the Patrons.t test file and the entire subtest related to StoreLastBorrower to StoreLastBorrower.t It just moves and add some minor adjustements. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56378 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56378&action=edit Bug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed to return the patrons with an issue history older than a given date. It would make more sense to return a list of Koha::Patrons. On the way, the code from AnonymiseIssueHistory will be moved as well to anonymise_issue_history. Note that these 2 subroutines are strongly linked: one is used to know the number of patrons we will anonymise the history, the other one is used to anonymise the issues history. The problem is that the first one is not used to do the action, but only for displayed purpose. In some cases, these 2 values can differ, which could be confusing. Case 1: The logged in librarian is not superlibrarian and IndependentBranches is set: if 2+ patrons from different libraries match the date parameter, the interface will display "Checkout history for 2 patrons will be anonymized", when actually only 1 will be. Case 2: If 2+ patrons match the date parameter but one of them has his privacy set to forever (privacy=0), the same issue will appear. This patch moves the code from C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons->search_patrons_to_anonymise and from C4::Circulation::AnonymiseIssueHistory to Koha::Patrons->anonymise_issue_history Test plan: 1/ Confirm the 2 issues and make sure they are fixed using the Batch patron anonymization tool (tools/cleanborrowers.pl) 2/ At the OPAC, use the 'Immediate deletion' button to delete all your reading history (regardless the setting of the privacy rule) 3/ Use the cronjob script (misc/cronjobs/batch_anonymise.pl) to anonymise patrons. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56379 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56379&action=edit Bug 16966: Koha::Patrons - Move AnonymiseIssueHistory tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Josef Moravec <josef.moravec@gmail.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=16966 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56376|0 |1 is obsolete| | --- Comment #9 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 56639 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56639&action=edit [SIGNED-OFF] Bug 16966: GetBorrowersWithIssuesHistoryOlderThan - Highlight the existing issue in tests If IndependentBranches is set, the code is buggy. This patch only highlight the bug by providing a test. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56377|0 |1 is obsolete| | --- Comment #10 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 56640 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56640&action=edit [SIGNED-OFF] Bug 16966: Koha::Patrons - Move AnonymiseIssueHistory tests The next patch will move C4::Circulation::AnonymiseIssueHistory and C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons This patch move the history anonymisation code to the Patrons.t test file and the entire subtest related to StoreLastBorrower to StoreLastBorrower.t It just moves and add some minor adjustements. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56378|0 |1 is obsolete| | --- Comment #11 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 56641 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56641&action=edit [SIGNED-OFF] Bug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed to return the patrons with an issue history older than a given date. It would make more sense to return a list of Koha::Patrons. On the way, the code from AnonymiseIssueHistory will be moved as well to anonymise_issue_history. Note that these 2 subroutines are strongly linked: one is used to know the number of patrons we will anonymise the history, the other one is used to anonymise the issues history. The problem is that the first one is not used to do the action, but only for displayed purpose. In some cases, these 2 values can differ, which could be confusing. Case 1: The logged in librarian is not superlibrarian and IndependentBranches is set: if 2+ patrons from different libraries match the date parameter, the interface will display "Checkout history for 2 patrons will be anonymized", when actually only 1 will be. Case 2: If 2+ patrons match the date parameter but one of them has his privacy set to forever (privacy=0), the same issue will appear. This patch moves the code from C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons->search_patrons_to_anonymise and from C4::Circulation::AnonymiseIssueHistory to Koha::Patrons->anonymise_issue_history Test plan: 1/ Confirm the 2 issues and make sure they are fixed using the Batch patron anonymization tool (tools/cleanborrowers.pl) 2/ At the OPAC, use the 'Immediate deletion' button to delete all your reading history (regardless the setting of the privacy rule) 3/ Use the cronjob script (misc/cronjobs/batch_anonymise.pl) to anonymise patrons. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56379|0 |1 is obsolete| | --- Comment #12 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 56642 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56642&action=edit [SIGNED-OFF] Bug 16966: Koha::Patrons - Move AnonymiseIssueHistory tests Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56639|0 |1 is obsolete| | Attachment #56640|0 |1 is obsolete| | Attachment #56641|0 |1 is obsolete| | Attachment #56642|0 |1 is obsolete| | --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 57170 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57170&action=edit Bug 16966: GetBorrowersWithIssuesHistoryOlderThan - Highlight the existing issue in tests If IndependentBranches is set, the code is buggy. This patch only highlight the bug by providing a test. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 57171 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57171&action=edit Bug 16966: Koha::Patrons - Move AnonymiseIssueHistory tests The next patch will move C4::Circulation::AnonymiseIssueHistory and C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons This patch move the history anonymisation code to the Patrons.t test file and the entire subtest related to StoreLastBorrower to StoreLastBorrower.t It just moves and add some minor adjustements. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 57172 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57172&action=edit Bug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed to return the patrons with an issue history older than a given date. It would make more sense to return a list of Koha::Patrons. On the way, the code from AnonymiseIssueHistory will be moved as well to anonymise_issue_history. Note that these 2 subroutines are strongly linked: one is used to know the number of patrons we will anonymise the history, the other one is used to anonymise the issues history. The problem is that the first one is not used to do the action, but only for displayed purpose. In some cases, these 2 values can differ, which could be confusing. Case 1: The logged in librarian is not superlibrarian and IndependentBranches is set: if 2+ patrons from different libraries match the date parameter, the interface will display "Checkout history for 2 patrons will be anonymized", when actually only 1 will be. Case 2: If 2+ patrons match the date parameter but one of them has his privacy set to forever (privacy=0), the same issue will appear. This patch moves the code from C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons->search_patrons_to_anonymise and from C4::Circulation::AnonymiseIssueHistory to Koha::Patrons->anonymise_issue_history Test plan: 1/ Confirm the 2 issues and make sure they are fixed using the Batch patron anonymization tool (tools/cleanborrowers.pl) 2/ At the OPAC, use the 'Immediate deletion' button to delete all your reading history (regardless the setting of the privacy rule) 3/ Use the cronjob script (misc/cronjobs/batch_anonymise.pl) to anonymise patrons. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Patches rebased (and 2 squashed). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17553 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17553 [Bug 17553] Move GetOverduesForPatron to Koha::Patron -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17580 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17580 [Bug 17580] Add the Koha::Patron->get_overdues method -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|17553 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17553 [Bug 17553] Move GetOverduesForPatron to Koha::Patron -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|17580 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17580 [Bug 17580] Add the Koha::Patron->get_overdues method -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply CC| |tomascohen@gmail.com --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #16)
Patches rebased (and 2 squashed).
I didn't succeed applying them in current master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57170|0 |1 is obsolete| | Attachment #57171|0 |1 is obsolete| | Attachment #57172|0 |1 is obsolete| | --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 57885 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57885&action=edit Bug 16966: GetBorrowersWithIssuesHistoryOlderThan - Highlight the existing issue in tests If IndependentBranches is set, the code is buggy. This patch only highlight the bug by providing a test. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 57886 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57886&action=edit Bug 16966: Koha::Patrons - Move AnonymiseIssueHistory tests The next patch will move C4::Circulation::AnonymiseIssueHistory and C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons This patch move the history anonymisation code to the Patrons.t test file and the entire subtest related to StoreLastBorrower to StoreLastBorrower.t It just moves and add some minor adjustements. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #20 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 57887 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57887&action=edit Bug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed to return the patrons with an issue history older than a given date. It would make more sense to return a list of Koha::Patrons. On the way, the code from AnonymiseIssueHistory will be moved as well to anonymise_issue_history. Note that these 2 subroutines are strongly linked: one is used to know the number of patrons we will anonymise the history, the other one is used to anonymise the issues history. The problem is that the first one is not used to do the action, but only for displayed purpose. In some cases, these 2 values can differ, which could be confusing. Case 1: The logged in librarian is not superlibrarian and IndependentBranches is set: if 2+ patrons from different libraries match the date parameter, the interface will display "Checkout history for 2 patrons will be anonymized", when actually only 1 will be. Case 2: If 2+ patrons match the date parameter but one of them has his privacy set to forever (privacy=0), the same issue will appear. This patch moves the code from C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons->search_patrons_to_anonymise and from C4::Circulation::AnonymiseIssueHistory to Koha::Patrons->anonymise_issue_history Test plan: 1/ Confirm the 2 issues and make sure they are fixed using the Batch patron anonymization tool (tools/cleanborrowers.pl) 2/ At the OPAC, use the 'Immediate deletion' button to delete all your reading history (regardless the setting of the privacy rule) 3/ Use the cronjob script (misc/cronjobs/batch_anonymise.pl) to anonymise patrons. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Kyle M Hall <kyle@bywatersolutions.com> 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=16966 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57885|0 |1 is obsolete| | Attachment #57886|0 |1 is obsolete| | Attachment #57887|0 |1 is obsolete| | --- Comment #21 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 57924 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57924&action=edit Bug 16966: GetBorrowersWithIssuesHistoryOlderThan - Highlight the existing issue in tests If IndependentBranches is set, the code is buggy. This patch only highlight the bug by providing a test. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #22 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 57925 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57925&action=edit Bug 16966: Koha::Patrons - Move AnonymiseIssueHistory tests The next patch will move C4::Circulation::AnonymiseIssueHistory and C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons This patch move the history anonymisation code to the Patrons.t test file and the entire subtest related to StoreLastBorrower to StoreLastBorrower.t It just moves and add some minor adjustements. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #23 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 57926 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57926&action=edit Bug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed to return the patrons with an issue history older than a given date. It would make more sense to return a list of Koha::Patrons. On the way, the code from AnonymiseIssueHistory will be moved as well to anonymise_issue_history. Note that these 2 subroutines are strongly linked: one is used to know the number of patrons we will anonymise the history, the other one is used to anonymise the issues history. The problem is that the first one is not used to do the action, but only for displayed purpose. In some cases, these 2 values can differ, which could be confusing. Case 1: The logged in librarian is not superlibrarian and IndependentBranches is set: if 2+ patrons from different libraries match the date parameter, the interface will display "Checkout history for 2 patrons will be anonymized", when actually only 1 will be. Case 2: If 2+ patrons match the date parameter but one of them has his privacy set to forever (privacy=0), the same issue will appear. This patch moves the code from C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons->search_patrons_to_anonymise and from C4::Circulation::AnonymiseIssueHistory to Koha::Patrons->anonymise_issue_history Test plan: 1/ Confirm the 2 issues and make sure they are fixed using the Batch patron anonymization tool (tools/cleanborrowers.pl) 2/ At the OPAC, use the 'Immediate deletion' button to delete all your reading history (regardless the setting of the privacy rule) 3/ Use the cronjob script (misc/cronjobs/batch_anonymise.pl) to anonymise patrons. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Patch doesn't apply CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57924|0 |1 is obsolete| | Attachment #57925|0 |1 is obsolete| | Attachment #57926|0 |1 is obsolete| | --- Comment #24 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 58282 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58282&action=edit Bug 16966: GetBorrowersWithIssuesHistoryOlderThan - Highlight the existing issue in tests If IndependentBranches is set, the code is buggy. This patch only highlight the bug by providing a test. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #25 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 58283 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58283&action=edit Bug 16966: Koha::Patrons - Move AnonymiseIssueHistory tests The next patch will move C4::Circulation::AnonymiseIssueHistory and C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons This patch move the history anonymisation code to the Patrons.t test file and the entire subtest related to StoreLastBorrower to StoreLastBorrower.t It just moves and add some minor adjustements. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #26 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 58284 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58284&action=edit Bug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed to return the patrons with an issue history older than a given date. It would make more sense to return a list of Koha::Patrons. On the way, the code from AnonymiseIssueHistory will be moved as well to anonymise_issue_history. Note that these 2 subroutines are strongly linked: one is used to know the number of patrons we will anonymise the history, the other one is used to anonymise the issues history. The problem is that the first one is not used to do the action, but only for displayed purpose. In some cases, these 2 values can differ, which could be confusing. Case 1: The logged in librarian is not superlibrarian and IndependentBranches is set: if 2+ patrons from different libraries match the date parameter, the interface will display "Checkout history for 2 patrons will be anonymized", when actually only 1 will be. Case 2: If 2+ patrons match the date parameter but one of them has his privacy set to forever (privacy=0), the same issue will appear. This patch moves the code from C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons->search_patrons_to_anonymise and from C4::Circulation::AnonymiseIssueHistory to Koha::Patrons->anonymise_issue_history Test plan: 1/ Confirm the 2 issues and make sure they are fixed using the Batch patron anonymization tool (tools/cleanborrowers.pl) 2/ At the OPAC, use the 'Immediate deletion' button to delete all your reading history (regardless the setting of the privacy rule) 3/ Use the cronjob script (misc/cronjobs/batch_anonymise.pl) to anonymise patrons. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #27 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 58285 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58285&action=edit Bug 16966: Make the tests pass again -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #28 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 58286 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58286&action=edit Bug 16966: The method should return a Koha::Patrons object -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #29 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking at this one now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply --- Comment #30 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Merge conflicts on several patches. Subtests in subtests. Please rebase yourself. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58282|0 |1 is obsolete| | --- Comment #31 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 60377 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60377&action=edit Bug 16966: GetBorrowersWithIssuesHistoryOlderThan - Highlight the existing issue in tests If IndependentBranches is set, the code is buggy. This patch only highlight the bug by providing a test. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58283|0 |1 is obsolete| | --- Comment #32 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 60378 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60378&action=edit Bug 16966: Koha::Patrons - Move AnonymiseIssueHistory tests The next patch will move C4::Circulation::AnonymiseIssueHistory and C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons This patch move the history anonymisation code to the Patrons.t test file and the entire subtest related to StoreLastBorrower to StoreLastBorrower.t It just moves and add some minor adjustements. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58284|0 |1 is obsolete| | --- Comment #33 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 60379 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60379&action=edit Bug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed to return the patrons with an issue history older than a given date. It would make more sense to return a list of Koha::Patrons. On the way, the code from AnonymiseIssueHistory will be moved as well to anonymise_issue_history. Note that these 2 subroutines are strongly linked: one is used to know the number of patrons we will anonymise the history, the other one is used to anonymise the issues history. The problem is that the first one is not used to do the action, but only for displayed purpose. In some cases, these 2 values can differ, which could be confusing. Case 1: The logged in librarian is not superlibrarian and IndependentBranches is set: if 2+ patrons from different libraries match the date parameter, the interface will display "Checkout history for 2 patrons will be anonymized", when actually only 1 will be. Case 2: If 2+ patrons match the date parameter but one of them has his privacy set to forever (privacy=0), the same issue will appear. This patch moves the code from C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons->search_patrons_to_anonymise and from C4::Circulation::AnonymiseIssueHistory to Koha::Patrons->anonymise_issue_history Test plan: 1/ Confirm the 2 issues and make sure they are fixed using the Batch patron anonymization tool (tools/cleanborrowers.pl) 2/ At the OPAC, use the 'Immediate deletion' button to delete all your reading history (regardless the setting of the privacy rule) 3/ Use the cronjob script (misc/cronjobs/batch_anonymise.pl) to anonymise patrons. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58285|0 |1 is obsolete| | --- Comment #34 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 60380 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60380&action=edit Bug 16966: Make the tests pass again -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58286|0 |1 is obsolete| | --- Comment #35 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 60381 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60381&action=edit Bug 16966: The method should return a Koha::Patrons object -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #36 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 60382 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60382&action=edit Bug 16966: Teach K::S::R::OldIssue its Koha objects class -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #37 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Patrons.t crashes now on overdrive_auth_token in subtest "move_to_deleted". See bug 16034. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #38 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 2999-12-12 ?? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #39 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA Comments: FAIL Koha/Patrons.pm FAIL pod *** ERROR: empty =head3 in file Koha/Patrons.pm The year 2999 in Patrons.pm should not be there. Joubu promises to fix it on a new report. See IRC :) Please add a link. tools/cleanborrowers.pl +use C4::Members; # GetBorrowersWhoHavexxxBorrowed. Not sure if you need this module any more. Seems not. search_patrons_to_anonymise anonymise_issue_history I'd rather have a parameter hash here now instead of these individual ones. There will be another parameter some day. Yes, article_requests_finished should have it too :) Typo should return Koha::Patrons => Patron in t/db_dependent/Koha/Patrons.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #60379|0 |1 is obsolete| | Attachment #60380|0 |1 is obsolete| | Attachment #60381|0 |1 is obsolete| | Attachment #60382|0 |1 is obsolete| | --- Comment #40 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 60648 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60648&action=edit Bug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed to return the patrons with an issue history older than a given date. It would make more sense to return a list of Koha::Patrons. On the way, the code from AnonymiseIssueHistory will be moved as well to anonymise_issue_history. Note that these 2 subroutines are strongly linked: one is used to know the number of patrons we will anonymise the history, the other one is used to anonymise the issues history. The problem is that the first one is not used to do the action, but only for displayed purpose. In some cases, these 2 values can differ, which could be confusing. Case 1: The logged in librarian is not superlibrarian and IndependentBranches is set: if 2+ patrons from different libraries match the date parameter, the interface will display "Checkout history for 2 patrons will be anonymized", when actually only 1 will be. Case 2: If 2+ patrons match the date parameter but one of them has his privacy set to forever (privacy=0), the same issue will appear. This patch moves the code from C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons->search_patrons_to_anonymise and from C4::Circulation::AnonymiseIssueHistory to Koha::Patrons->anonymise_issue_history Test plan: 1/ Confirm the 2 issues and make sure they are fixed using the Batch patron anonymization tool (tools/cleanborrowers.pl) 2/ At the OPAC, use the 'Immediate deletion' button to delete all your reading history (regardless the setting of the privacy rule) 3/ Use the cronjob script (misc/cronjobs/batch_anonymise.pl) to anonymise patrons. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #41 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 60649 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60649&action=edit Bug 16966: Make the tests pass again -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #42 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 60650 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60650&action=edit Bug 16966: The method should return a Koha::Patrons object -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #43 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 60651 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60651&action=edit Bug 16966: Teach K::S::R::OldIssue its Koha objects class -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #44 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 60652 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60652&action=edit Bug 16966: Fix small QA issues -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #45 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 60653 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60653&action=edit Bug 16966: move parameters to hashref -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #46 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #39)
QA Comments:
FAIL Koha/Patrons.pm FAIL pod *** ERROR: empty =head3 in file Koha/Patrons.pm
Fixed!
The year 2999 in Patrons.pm should not be there. Joubu promises to fix it on a new report. See IRC :) Please add a link.
It's not in module, but opac-privacy. I will!
tools/cleanborrowers.pl +use C4::Members; # GetBorrowersWhoHavexxxBorrowed. Not sure if you need this module any more. Seems not.
I remove the GetBorrowersWhoHavexxxBorrowed part, caused by a bad merge conflict (re-edited original patch). C4::Members is still used (GetBorrowersToExpunge)
search_patrons_to_anonymise anonymise_issue_history I'd rather have a parameter hash here now instead of these individual ones. There will be another parameter some day. Yes, article_requests_finished should have it too :)
Done in last patch.
Typo should return Koha::Patrons => Patron in t/db_dependent/Koha/Patrons.t
Fixed! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #47 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Will come back here on Monday -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18169 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18169 [Bug 18169] Date like 2999 should not be used arbitrarily -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #48 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #46)
The year 2999 in Patrons.pm should not be there. Joubu promises to fix it on a new report. See IRC :) Please add a link.
It's not in module, but opac-privacy. I will!
See bug 18169. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #60377|0 |1 is obsolete| | --- Comment #49 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 60701 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60701&action=edit Bug 16966: GetBorrowersWithIssuesHistoryOlderThan - Highlight the existing issue in tests If IndependentBranches is set, the code is buggy. This patch only highlight the bug by providing a test. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #60378|0 |1 is obsolete| | --- Comment #50 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 60702 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60702&action=edit Bug 16966: Koha::Patrons - Move AnonymiseIssueHistory tests The next patch will move C4::Circulation::AnonymiseIssueHistory and C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons This patch move the history anonymisation code to the Patrons.t test file and the entire subtest related to StoreLastBorrower to StoreLastBorrower.t It just moves and add some minor adjustements. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #60648|0 |1 is obsolete| | --- Comment #51 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 60703 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60703&action=edit Bug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed to return the patrons with an issue history older than a given date. It would make more sense to return a list of Koha::Patrons. On the way, the code from AnonymiseIssueHistory will be moved as well to anonymise_issue_history. Note that these 2 subroutines are strongly linked: one is used to know the number of patrons we will anonymise the history, the other one is used to anonymise the issues history. The problem is that the first one is not used to do the action, but only for displayed purpose. In some cases, these 2 values can differ, which could be confusing. Case 1: The logged in librarian is not superlibrarian and IndependentBranches is set: if 2+ patrons from different libraries match the date parameter, the interface will display "Checkout history for 2 patrons will be anonymized", when actually only 1 will be. Case 2: If 2+ patrons match the date parameter but one of them has his privacy set to forever (privacy=0), the same issue will appear. This patch moves the code from C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons->search_patrons_to_anonymise and from C4::Circulation::AnonymiseIssueHistory to Koha::Patrons->anonymise_issue_history Test plan: 1/ Confirm the 2 issues and make sure they are fixed using the Batch patron anonymization tool (tools/cleanborrowers.pl) 2/ At the OPAC, use the 'Immediate deletion' button to delete all your reading history (regardless the setting of the privacy rule) 3/ Use the cronjob script (misc/cronjobs/batch_anonymise.pl) to anonymise patrons. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #60649|0 |1 is obsolete| | --- Comment #52 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 60704 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60704&action=edit Bug 16966: Make the tests pass again Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #60650|0 |1 is obsolete| | --- Comment #53 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 60705 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60705&action=edit Bug 16966: The method should return a Koha::Patrons object Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #60651|0 |1 is obsolete| | --- Comment #54 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 60706 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60706&action=edit Bug 16966: Teach K::S::R::OldIssue its Koha objects class Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #60652|0 |1 is obsolete| | --- Comment #55 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 60707 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60707&action=edit Bug 16966: Fix small QA issues Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #60653|0 |1 is obsolete| | --- Comment #56 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 60708 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60708&action=edit Bug 16966: move parameters to hashref Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 --- Comment #57 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 60709 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60709&action=edit Bug 16966: [QA Follow-up] Check count, not size in template The method size is not covered by tests at koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt line 163. The method size is not covered by tests at koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt line 183. Script cleanborrowers passes patrons_to_delete and patrons_to_anonymize to the template. One is an array ref where we should use 'size', the other is a Koha::Patrons object where we should use 'count'. If we don't, we will not anonymize a lot of records ;) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
To make sure all the conditions are met, the caller has the responsability to call search_patrons_to_anonymise to filter the Koha::Patrons set. I am not sure if this is the most ideal solution. Koha::Patrons->anonymise_issue_history just does all records and ignores
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #58 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA Comment: Looks good to me. Added another follow-up. No blocker, but just mentioning: privacy settings. A parameter for patrons might be safer? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #59 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 17.05, thanks Jonathan, Marcel! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED CC| |katrin.fischer@bsz-bw.de --- Comment #60 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This won't get ported back to 16.11.x as it is an enhancement. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |19599 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19599 [Bug 19599] anonymise_issue_history can be very slow on large systems -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |20363 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20363 [Bug 20363] Privacy management shows misleading "No reading history to delete" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16966 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |22944 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22944 [Bug 22944] avoid AnonymousPatron in search_patrons_to_anonymise -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org