[Bug 15163] New: Patron attributes with branch limiits are not saved when invisible
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 Bug ID: 15163 Summary: Patron attributes with branch limiits are not saved when invisible Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: jweaver@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, hbraum@nekls.org, kyle.m.hall@gmail.com Because only the patron attributes that are visible are saved when editing a patron, any attributes that are hidden from the member entry screen by a branch limitation will be erased when saving. This could be solved one of two ways: a) Show all extended attributes when editing or b) Only delete the borrower attributes that are not limited or are visible at the current branch as the first step in C4::Members::Attributes::SetBorrowerAttributes. (Marking critical because data is lost.) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Jessie, Could you explain the workflow to get this issue? It looks like a configuration issue. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 --- Comment #2 from Jesse Weaver <jweaver@bywatersolutions.com> --- Steps to reproduce: 1) Log in at library "A". 2) Create a patron attribute type that is limited to only library "A". 3) Edit a patron, and add data for that attribute type. 4) Log in at library "B". 5) Edit the same patron. The attribute should, of course, not be visible. Save. 6) Log in at library "A". 7) Edit the same patron. The attribute data has disappeared. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 --- Comment #3 from Heather Braum <hbraum@nekls.org> --- Jonathan, Jesse's description is the behavior that I have seen also happen on my system. Additionally, I have seen this same behavior happen with patron categories that are limited to only certain branches (and maybe this needs to be a separate bug), but it seems to be the same type of problematic behavior: 1. Login at Library A. 2. Set a patron category to be visible to only Library A. 3. Edit a patron and change the patron category to one that is only visible to Library A. 4. Login at Library B. 5. Edit the same patron, and note that the patron category is now the first one on the list, not the one set in step 3. 6. Login at Library A. 7. Edit the same patron again; note that the patron category is not set to what was set in step 3. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 45258 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45258&action=edit Bug 15163: Do not erase patron attributes if limited to another library The patron attributes displayed on editing a patron are not displayed if limited to another library. This is the easy but dirty way to fix this issue: It supposes that the data are not sensitive as they are now displayed in the html document. A better way to fix it would be to modify C4::Members::Attributes::SetBorrowerAttributes to delete only attributes we are editing. Test plan: 1/ Create 2 patron attributes, without branch limitations. 2/ Edit a patron and set a value for these attributes 3/ Limit a patron attributes to a library (one you are not logged in with). 4/ Edit again the patron. => You should not see the limited attributes 5/ Edit the patron attributes and remove the branch limitation => Without this patch, it has been removed from the database and is not displayed anymore. => With this patch, you should see it. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |jonathan.druart@bugs.koha-c |ity.org |ommunity.org --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I have failed to provide a correct patch for this fix in a respectable time. This one should fix the issue but it is not the best way to do. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45258|0 |1 is obsolete| | --- Comment #6 from Marc Véron <veron@veron.ch> --- Created attachment 45260 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45260&action=edit Bug 15163: Do not erase patron attributes if limited to another library The patron attributes displayed on editing a patron are not displayed if limited to another library. This is the easy but dirty way to fix this issue: It supposes that the data are not sensitive as they are now displayed in the html document. A better way to fix it would be to modify C4::Members::Attributes::SetBorrowerAttributes to delete only attributes we are editing. Test plan: 1/ Create 2 patron attributes, without branch limitations. 2/ Edit a patron and set a value for these attributes 3/ Limit a patron attributes to a library (one you are not logged in with). 4/ Edit again the patron. => You should not see the limited attributes 5/ Edit the patron attributes and remove the branch limitation => Without this patch, it has been removed from the database and is not displayed anymore. => With this patch, you should see it. Followed test plan, works as described. Signed-off-by: Marc Véron <veron@veron.ch> http://bugs.koha-community.org/show_bug.cgi?id=15165 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|Needs Signoff |Signed Off CC| |veron@veron.ch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion CC| |katrin.fischer@bsz-bw.de --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This works as described. With the proposed solution the values of the hidden patron attribute are visible in the HTML source of the page. I am not super happy about this, as it seems like a potential privacy problem to me. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Katrin Fischer from comment #7)
This works as described.
With the proposed solution the values of the hidden patron attribute are visible in the HTML source of the page. I am not super happy about this, as it seems like a potential privacy problem to me.
I can add a syspref to choose between privacy and data lost :D -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am not sure there is a valid answer to this. Would it be possible to teach it not to update/change attributes not sent with the form at all? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 --- Comment #10 from Jesse Weaver <jweaver@bywatersolutions.com> --- Started working on this; Jonathan, what do you think of passing a $branchcode_limit parameter to SetBorrowerAttributes so it only deletes the correct attributes? (Either just passing 1 and using userenv, which is what AttributeTypes.pm does, or explicitly passing the branchcode, which is easier to test.) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Jesse, yes it should be the way to go but.. I have already tried something and was stuck. I have a wip branch but I don't remember what was wrong. Looking at the diff it seems that I have tried to add a DeleteBorrowerAttributes subroutines and the code looks quite complicated: +sub DeleteBorrowerAttributes { + my $borrowernumber = shift; + my $no_branch_limit = @_ ? shift : 0; + my $branch_limit = $no_branch_limit + ? 0 + : C4::Context->userenv ? C4::Context->userenv->{"branch"} : 0; + + my $dbh = C4::Context->dbh; + my $query = q{ + DELETE FROM borrower_attributes + }; + $query .= q{ + LEFT JOIN borrower_attribute_types_branches ON bat_code = code + WHERE b_branchcode = ? OR b_branchcode IS NULL + } if $branch_limit; + $query .= q{ + WHERE borrowernumber = ? + }; + + $dbh->do( $query, undef, $branch_limit ? $branch_limit : (), $borrowernumber ); +} -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 46416 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46416&action=edit Bug 15163: Wip - stuck -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #12)
Created attachment 46416 [details] [review] Bug 15163: Wip - stuck
Jesse, here is my start. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 --- Comment #14 from Heather Braum <hbraum@nekls.org> --- Thanks Jonathan and Jesse for working on this. I understand the privacy concerns of some of the ways this could be resolved. The issue really does need to be resolved. I am unable to use branch specific limits on several sets of authorised_values & patron categories until this is resolved (which is not ideal). We have patrons who use multiple branches and when their accounts are edited at another branch, we risk data being lost. Just my two cents on how important this bug is to get resolved. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|jonathan.druart@bugs.koha-c |koha-bugs@lists.koha-commun |ommunity.org |ity.org -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org