Description
Chris Cormack
2011-03-20 21:13:24 UTC
Created attachment 3364 [details] [review] Proposed patch Created attachment 3365 [details] [review] Follow up patch This bug is mentioned in: Bug 5904 : ysearch.pl members improvements (Independant branches behaviour) http://lists.koha-community.org/pipermail/koha-patches/2011-March/014143.html Bug 5904 - Follow up patch, with the db changes http://lists.koha-community.org/pipermail/koha-patches/2011-March/014144.html Produce this query: SELECT surname, firstname, cardnumber, address, city, zipcode, country FROM borrowers WHERE surname LIKE 'a%' OR firstname LIKE 'a%' OR cardnumber LIKE 'a%' AND borrowers.branchcode ='MPL' ORDER BY surname, firstname I think it should produce ... WHERE (surname LIKE 'a%' OR firstname LIKE 'a%' OR cardnumber LIKE 'a%') AND borrowers.branchcode ='MPL' ORDER BY surname, firstname Is .pref file missing? Created attachment 3809 [details] [review] Follow up patch Bug 5904 - Follow up : .pref and fix - adds parenthesis to query - modify patron.pref file This no longer applies. I would be helpful if we could also get a good explanation of what this patch is supposed to fix and how to test. Patches need to be rewritten by now, because we handle system preferences differently now and less files have to be changed. Created attachment 19259 [details] [review] Bug 5904: ysearch.pl members improvements (Independent branches behaviour) Test plan: - Add a staff user foo in a library A - Add a staff user bar in a library B - log in as foo and try to search 'bar' - Switch on (Prevent) the new pref IndependentBranchPatron - the search returns no result - log in as a superlibrarian - Check that you are allow to search all patrons. Created attachment 20122 [details] [review] [SIGNED-OFF] Bug 5904: ysearch.pl members improvements (Independent branches behaviour) Test plan: - Add a staff user foo in a library A - Add a staff user bar in a library B - log in as foo and try to search 'bar' - Switch on (Prevent) the new pref IndependentBranchPatron - the search returns no result - log in as a superlibrarian - Check that you are allow to search all patrons. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> I've signed this off, but not sure how much sense it makes. It is just for auto-complete, so used only for the checkouts. If you search eg patrons on /members/member.pl it is not used. (In reply to Srdjan Jankovic from comment #11) > I've signed this off, but not sure how much sense it makes. It is just for > auto-complete, so used only for the checkouts. If you search eg patrons on > /members/member.pl it is not used. I agree. Independent branches already hides patrons from other branches, so having a separate system preference for this doesn't make much sense to me. Jonathan, can you think of any compelling reason we should have a separate system preference for this? I would consider the original functionality to be a bug, and this to be a bug fix. If you agree, a followup to remove the new system preference should be sufficient for this to pass qa. (In reply to Kyle M Hall from comment #12) > (In reply to Srdjan Jankovic from comment #11) > > I've signed this off, but not sure how much sense it makes. It is just for > > auto-complete, so used only for the checkouts. If you search eg patrons on > > /members/member.pl it is not used. > > I agree. Independent branches already hides patrons from other branches, so > having a separate system preference for this doesn't make much sense to me. > > Jonathan, can you think of any compelling reason we should have a separate > system preference for this? I would consider the original functionality to > be a bug, and this to be a bug fix. Kyle, Yes you are right, I stupidly rebased this patch but it does not make sense for me neither. Created attachment 20742 [details] [review] Bug 5904: ysearch.pl members improvements (Independent branches behaviour) Test plan: - Add a staff user foo in a library A - Add a staff user bar in a library B - log in as foo and try to search 'bar' - Switch on (Prevent) the new pref IndependentBranches - the search returns no result - log in as a superlibrarian - Check that you are allow to search all patrons. Created attachment 20750 [details] [review] Bug 5904: ysearch.pl members improvements (Independent branches behaviour) Test plan: - Add a staff user foo in a library A - Add a staff user bar in a library B - log in as foo and try to search 'bar' - Switch on (Prevent) the new pref IndependentBranches - the search returns no result - log in as a superlibrarian - Check that you are allow to search all patrons. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Created attachment 20766 [details] [review] Bug 5904: ysearch.pl members improvements (Independent branches behaviour) Test plan: - Add a staff user foo in a library A - Add a staff user bar in a library B - log in as foo and try to search 'bar' - Switch on (Prevent) the new pref IndependentBranches - the search returns no result - log in as a superlibrarian - Check that you are allow to search all patrons. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Passes koha-qa.pl, works as advertised. Pushed to master. Thanks, Jonathan! This patch has been pushed to 3.12.x, will be in 3.12.5. Thanks Jonathan! Pushed to 3.10.x, will be in 3.10.12 |