[Bug 6506] New: deleting patron history missing errors
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Bug #: 6506 Summary: deleting patron history missing errors Classification: Unclassified Change sponsored?: --- Product: Koha Version: master Platform: All OS/Version: All Status: NEW Severity: critical Priority: P5 Component: Patrons AssignedTo: kyle.m.hall@gmail.com ReportedBy: nengard@gmail.com QAContact: koha-bugs@lists.koha-community.org CC: gmcharlt@gmail.com I'm not sure what category to put this in. If you don't set up an anonymous patron you don't get errors in the OPAC when using my privacy (which requires this patron). Without AnonymousPatron set, go to the OPAC and click 'immediate deletion' on your privacy tab to delete your reading history. You are told that your history is deleted, but the history is actually still showing. I'm assuming that all of the privacy settings are not working if that patron isn't set and there should be some kind of error message explaining that instead of making the patron think that the feature works. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Robin Sheat <robin@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |robin@catalyst.net.nz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 --- Comment #1 from Robin Sheat <robin@catalyst.net.nz> --- The code (C4::Circulation->AnonymiseIssueHistory) says: # The anonymisation will fail quietly if AnonymousPatron is not a valid entry which is terrible. It should fail loudly. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Peter Kelly <peterAtKohaBugzilla@pck.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |peterAtKohaBugzilla@pck.co. | |nz Assignee|kyle.m.hall@gmail.com |peterAtKohaBugzilla@pck.co. | |nz --- Comment #2 from Peter Kelly <peterAtKohaBugzilla@pck.co.nz> --- Reproduced the bug on current origin/master (3.12 ish) -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 --- Comment #3 from Peter Kelly <peterAtKohaBugzilla@pck.co.nz> --- Planned new error message for the privacy page: "The deletion of your reading history failed, because there is a problem with the configuration of this feature. Please help to fix the system by informing your library of this error." New bug to be raised for the history-not-deleted-on-return case. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 --- Comment #4 from Peter Kelly <peterAtKohaBugzilla@pck.co.nz> --- My plan for implementing the fix for this issue is: - in Circulation.pm change AnonymiseIssueHistory so that it returns ($rows, $err_history_not_deleted) instead of $rows - consequential change to misc/cronjobs/batch_anonymise.pl to handle updated return value - ditto tools/cleanborrowers.pl - update opac-privacy.pl to check return value and display error - update opac-privacy.tt to display error if appropriate -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Peter Kelly <peterAtKohaBugzilla@pck.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |9944 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 --- Comment #5 from Peter Kelly <peterAtKohaBugzilla@pck.co.nz> --- Created attachment 16955 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16955&action=edit Patch to implement fix -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Peter Kelly <peterAtKohaBugzilla@pck.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch --- Comment #6 from Peter Kelly <peterAtKohaBugzilla@pck.co.nz> --- Remedied by: - in Circulation.pm changing AnonymiseIssueHistory so that it returns ($rows, $err_history_not_deleted) instead of $rows - consequential change to misc/cronjobs/batch_anonymise.pl to handle updated return value, and fail if there is an error - consequential change to tools/cleanborrowers.pl although this still fails silently (raised as bug 9944) - update of opac-privacy.pl to check return value and pass on error - update of opac-privacy.tt to display error if appropriate To test: 1. OPAC - enable privacy mode (preference OpacPrivacy) - leave anonymous patron set to zero (preference AnonymousPatron) - attempt to delete user history - observe error - check history - still there - change anonymous patron to a valid user - attempt to delete user history - observe success message - check history - gone 2. cleanborrowers.pl - test it functions as before. bug 9944 has been raised for it continuing to silently fail. 3. batch_anonymise.pl - enable privacy mode (preference OpacPrivacy) - leave anonymous patron set to zero (preference AnonymousPatron) - run script (I use --days -1 for testing) - script should fail with a Carp message - change anonymous patron to a valid user - run script as before - script returns quietly - check history - gone -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Can we get a sign-off on this one please? -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #16955|0 |1 is obsolete| | --- Comment #8 from Mason James <mtj@kohaaloha.com> --- Created attachment 17113 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17113&action=edit Bug 6506: When AnonymousPatron not set, deletion of issue history silently failed. Remedied by: - in Circulation.pm changing AnonymiseIssueHistory so that it returns ($rows, $err_history_not_deleted) instead of $rows - consequential change to misc/cronjobs/batch_anonymise.pl to handle updated return value, and fail if there is an error - consequential change to tools/cleanborrowers.pl although this still fails silently (raised as bug 9944) - update of opac-privacy.pl to check return value and pass on error - update of opac-privacy.tt to display error if appropriate Note bug 9942 remains unfixed, which is a similar issue upon issue return. To test: 1. OPAC - enable privacy mode (preference OpacPrivacy) - leave anonymous patron set to zero (preference AnonymousPatron) - attempt to delete user history - observe error - check history - still there - change anonymous patron to a valid user - attempt to delete user history - observe success message - check history - gone 2. cleanborrowers.pl - test it functions as before. bug 9944 has been raised for it continuing to silently fail. 3. batch_anonymise.pl - enable privacy mode (preference OpacPrivacy) - leave anonymous patron set to zero (preference AnonymousPatron) - run script (I use --days -1 for testing) - script should fail with a Carp message - change anonymous patron to a valid user - run script as before - script returns quietly - check history - gone Signed-off-by: Mason James <mtj@kohaaloha.com> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com --- Comment #9 from Mason James <mtj@kohaaloha.com> --- (In reply to comment #8)
Created attachment 17113 [details] [review] Bug 6506: When AnonymousPatron not set, deletion of issue history silently failed.
Signed-off-by: Mason James <mtj@kohaaloha.com>
tested patch on master - works as advertised for me, so signing-off nice one Peter :) -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@biblibre.co | |m --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- QA comment: 1/ In C4::Circ::AnonymiseIssueHistory: why don't return the error after checking the syspref's values? The query will raise an error only if AnonymousPatron=0 and OpacPrivacy=1 isn't it? 2/ Maybe it could be better to raise an error (die) and catch it (eval) instead of to return another variable which should be checked for each call. 3/ There is no css class error (the error is displayed without css style). 1 and 2 for discussion. Marked as Failed QA for 3. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 17305 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17305&action=edit Bug 6506: Followup add warning in the system information tab. Add a warning in the system information tab of the about page. Test plan: Set OPACPrivacy = 1 and AnonymousPatron = 0 and go on the about page, tab 'system information'. Check that the warning appears. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 --- Comment #12 from Peter Kelly <peterAtKohaBugzilla@pck.co.nz> --- Created attachment 17317 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17317&action=edit Correct CSS name This corrects the mandatory item of QA feedback. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Peter Kelly <peterAtKohaBugzilla@pck.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #13 from Peter Kelly <peterAtKohaBugzilla@pck.co.nz> --- I've attached a patch for the mandatory-fix QA feedback. I'll socialise the others with some people in my timezone since I can't catch Joubu online easily. It seems to me that the warning patch, while a good idea, should be on its own bug. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #17113|0 |1 is obsolete| | Attachment #17305|0 |1 is obsolete| | Attachment #17317|0 |1 is obsolete| | --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 17385 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17385&action=edit Bug 6506: When AnonymousPatron not set, deletion of issue history silently failed. Remedied by: - in Circulation.pm changing AnonymiseIssueHistory so that it returns ($rows, $err_history_not_deleted) instead of $rows - consequential change to misc/cronjobs/batch_anonymise.pl to handle updated return value, and fail if there is an error - consequential change to tools/cleanborrowers.pl although this still fails silently (raised as bug 9944) - update of opac-privacy.pl to check return value and pass on error - update of opac-privacy.tt to display error if appropriate Note bug 9942 remains unfixed, which is a similar issue upon issue return. To test: 1. OPAC - enable privacy mode (preference OpacPrivacy) - leave anonymous patron set to zero (preference AnonymousPatron) - attempt to delete user history - observe error - check history - still there - change anonymous patron to a valid user - attempt to delete user history - observe success message - check history - gone 2. cleanborrowers.pl - test it functions as before. bug 9944 has been raised for it continuing to silently fail. 3. batch_anonymise.pl - enable privacy mode (preference OpacPrivacy) - leave anonymous patron set to zero (preference AnonymousPatron) - run script (I use --days -1 for testing) - script should fail with a Carp message - change anonymous patron to a valid user - run script as before - script returns quietly - check history - gone Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 17386 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17386&action=edit Bug 6506: Followup add warning in the system information tab. Add a warning in the system information tab of the about page. Test plan: Set OPACPrivacy = 1 and AnonymousPatron = 0 and go on the about page, tab 'system information'. Check that the warning appears. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 17387 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17387&action=edit Bug 6506: Correct CSS class name. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #17386|0 |1 is obsolete| | --- Comment #17 from Mason James <mtj@kohaaloha.com> --- Created attachment 17753 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17753&action=edit Bug 6506: Followup add warning in the system information tab. Add a warning in the system information tab of the about page. Test plan: Set OPACPrivacy = 1 and AnonymousPatron = 0 and go on the about page, tab 'system information'. Check that the warning appears. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Mason James <mtj@kohaaloha.com> http://bugs.koha-community.org/show_bug.cgi?id=6509 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #17385|0 |1 is obsolete| | Attachment #17387|0 |1 is obsolete| | Attachment #17753|0 |1 is obsolete| | --- Comment #18 from Mason James <mtj@kohaaloha.com> --- Created attachment 17754 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17754&action=edit Bug 6506: When AnonymousPatron not set, deletion of issue history silently failed. Remedied by: - in Circulation.pm changing AnonymiseIssueHistory so that it returns ($rows, $err_history_not_deleted) instead of $rows - consequential change to misc/cronjobs/batch_anonymise.pl to handle updated return value, and fail if there is an error - consequential change to tools/cleanborrowers.pl although this still fails silently (raised as bug 9944) - update of opac-privacy.pl to check return value and pass on error - update of opac-privacy.tt to display error if appropriate Note bug 9942 remains unfixed, which is a similar issue upon issue return. To test: 1. OPAC - enable privacy mode (preference OpacPrivacy) - leave anonymous patron set to zero (preference AnonymousPatron) - attempt to delete user history - observe error - check history - still there - change anonymous patron to a valid user - attempt to delete user history - observe success message - check history - gone 2. cleanborrowers.pl - test it functions as before. bug 9944 has been raised for it continuing to silently fail. 3. batch_anonymise.pl - enable privacy mode (preference OpacPrivacy) - leave anonymous patron set to zero (preference AnonymousPatron) - run script (I use --days -1 for testing) - script should fail with a Carp message - change anonymous patron to a valid user - run script as before - script returns quietly - check history - gone Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Mason James <mtj@kohaaloha.com> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #17753|1 |0 is obsolete| | -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 --- Comment #19 from Mason James <mtj@kohaaloha.com> --- Created attachment 17755 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17755&action=edit Bug 6506: Correct CSS class name. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Mason James <mtj@kohaaloha.com> http://bugs.koha-community.org/show_bug.cgi?id=6509 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #20 from Mason James <mtj@kohaaloha.com> --- Kyle has now signed-off on all 3 patches I am now passing QA on them -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 --- Comment #21 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- With DEBUG turned on, having AnonymousPatron blank still causes a fatal error, but I will report a separate bug for that, and push this as-is, since it helps when DEBUG is off. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #22 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- This patch has been pushed to master and 3.12.x. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |10165 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6506 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED CC| |chris@bigballofwax.co.nz Resolution|--- |FIXED --- Comment #23 from Chris Cormack <chris@bigballofwax.co.nz> --- Released with 3.12.0 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org