Bug 13898 - Remove unused code in moremember.pl related to permission
Summary: Remove unused code in moremember.pl related to permission
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-24 09:56 UTC by Jonathan Druart
Modified: 2015-12-03 22:11 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 13898: Remove CANDELETEUSER code (1.78 KB, patch)
2015-03-24 09:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
[Signed-off] Bug 13898: Remove CANDELETEUSER code (1.97 KB, patch)
2015-03-24 10:13 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 13898: Remove CANDELETEUSER code (2.02 KB, patch)
2015-03-24 21:47 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2015-03-24 09:56:45 UTC

    
Comment 1 Jonathan Druart 2015-03-24 09:58:11 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2015-03-24 10:13:04 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2015-03-24 21:47:05 UTC
I was curious, as this code looks basically right, but is not used, as you pointed out. This is the commit that added it to Koha:
http://git.koha-community.org/gitweb/?p=koha.git;a=commitdiff;h=05b61e4f3a57828d0bc7d400cfc4bb97c4402a71

It seems to have been broken sometime between 3.10 - 3.12:
http://git.koha-community.org/gitweb/?p=koha.git;a=commitdiff;h=8fd75227fcd87d2cea5d0cb31d7af72498fbb24d;hp=25232635f538ffb5582f56536c242d48f433018b#patch2

I wondered... should we bring it back instead of deleting it? 
I think it would just affect the behaviour for the IndependentBranches (not being able to delete another branches users).
But I guess in order to make this really work consistently, we'd have to add this variable to all the .pl scripts on the various pages that show the members toolbar? This might be the reason why this has been removed in the first place. 

Passing QA... but leaving my notes here :)
Comment 4 Katrin Fischer 2015-03-24 21:47:38 UTC
Created attachment 37199 [details] [review]
[PASSED QA] Bug 13898: Remove CANDELETEUSER code

This code is never used. The permission to delete a patron is now done
in members-toolbar.inc using CAN_user_borrowers.
It's the good way to do.

Test plan:
  git grep CANDELETEUSER
should not return any result

Followed test plan. No CANDELETEUSER found.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Jonathan Druart 2015-03-25 08:02:11 UTC
(In reply to Katrin Fischer from comment #3)
> I wondered... should we bring it back instead of deleting it? 

It's not used, so we can delete it.
If the IndependentBranches has to be checked (and I think it should), please open a new bug report.

But... This exists for 2 years now and it does not seem to affect people.
Comment 6 Katrin Fischer 2015-03-25 08:07:31 UTC
I agree... also the old code is not clean as it would only cover one page. - So I passed QA :)
Comment 7 Tomás Cohen Arazi (tcohen) 2015-03-31 14:56:42 UTC
Patch pushed to master.

Thanks Jonathan!