Only superlibrarian users and users with superserials permission could override this limitation.
Created attachment 11778 [details] [review] Bug 8438: Users can only claim for serials related to their branch. Only superlibrarian users and users with superserials permission can override this limitation. The original author of this patch is Julian Maurice.
Not sure about this, it is a change of existing behavour. I think you should link it to independent branches syspref. This new feature would indeed be a regression of a lot of my libraries. Where they have a serials librarian who looks after serials for multiple branches. But can't do circulation, cataloguing etc. Now they simply can't do that. Because that user would have to be super librarian.
Actually I take that back, they would need superserials. So instead my only issue now is that this is going to change behaviour, and without warning (in updatedatabase or anything) So people upgrading will not have any idea what happened, we need some way to warn people (this is a general koha issue)
Hm, maybe we could assign the users with serials permissions now the superserial permission on update? This way we would have no change of existing behaviour and now surprises. But not sure how hard that is - I know the permission flags are a bit cryptic.
I too am worried about this changing the current behavior. When the change was made to the purchase suggestions by limiting to one branch it confused a lot of our users and isn't really the desired behavior for them either. So I say either a preference or some way to make it less confusing.
Marking as Failed QA until the concerns in comments have been addressed.
Created attachment 12842 [details] [review] Bug 8438: Followup Set the superserials permission for all users with the claim_serials permission
Patches apply correctly here
Created attachment 24074 [details] [review] Bug 8438: Followup Set the superserials permission for all users with the claim_serials permission
Rebased patch.
Created attachment 26399 [details] [review] Bug 8438: Users can only claim for serials related to their branch. Only superlibrarian users and users with superserials permission can override this limitation. The original author of this patch is Julian Maurice.
Created attachment 26400 [details] [review] Bug 8438: Followup Set the superserials permission for all users with the claim_serials permission
When logged as : superlibrarian => OK : you can select subs from all branches user without superserials permission => OK : only checkbox corresponding to loggin branch subscriptions are displayed. You can't select others user with superserials permission => NOT OK : no difference from previous case. Only loggin branch subscriptions display checkboxes. Olivier Crouzet
Created attachment 29098 [details] [review] Bug 8438: Users can only claim for serials related to their branch. Only superlibrarian users and users with superserials permission can override this limitation. The original author of this patch is Julian Maurice.
Created attachment 29099 [details] [review] Bug 8438: Followup Set the superserials permission for all users with the claim_serials permission
Sorry Jonathan, when clicking on Claims link, I got this : SOFTWARE ERROR Global symbol "$subscription" requires explicit package name at /home/kohabac/src/serials/claims.pl line 71. Olivier Crouzet
Created attachment 29191 [details] [review] Bug 8438: Users can only claim for serials related to their branch. Only superlibrarian users and users with superserials permission can override this limitation. The original author of this patch is Julian Maurice.
Created attachment 29192 [details] [review] Bug 8438: Followup Set the superserials permission for all users with the claim_serials permission
(In reply to Koha Team Lyon 3 from comment #16) > Sorry Jonathan, when clicking on Claims link, I got this : Erk, sorry. Should be fixed now.
It's OK now for superlibrarians and users with superserials permissions, but it doesn't work any more for simple users : they are not restricted any more to the login branch...
(In reply to Koha Team Lyon 3 from comment #20) > It's OK now for superlibrarians and users with superserials permissions, but > it doesn't work any more for simple users : they are not restricted any more > to the login branch... Yes sorry, I changed the behavior without notifying the change. But actually I am not sure. I decided to use the same method as on editing a subscription, but it's wrong! I will submit another patch.
Created attachment 29211 [details] [review] Bug 8438: Users can only claim for serials related to their branch. Only superlibrarian users and users with superserials permission can override this limitation. This patch adds a new subroutine C4::Serials::can_claim_subscription.
Created attachment 29212 [details] [review] Bug 8438: Followup Set the superserials permission for all users with the claim_serials permission
I didn't test all cases but the unit tests told me that should work! :)
I've tried to apply the patch against master 3.19.00.005 head 12868 Applying: Bug 8438: Users can only claim for serials related to their branch. Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Serials_2.t CONFLICT (content): Merge conflict in t/db_dependent/Serials_2.t Auto-merging serials/claims.pl CONFLICT (content): Merge conflict in serials/claims.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt Auto-merging C4/Serials.pm Failed to merge in the changes. Patch failed at 0001 Bug 8438: Users can only claim for serials related to their branch. So I pass the patch to "Patch doesn't apply" status.
Created attachment 35177 [details] [review] Bug 8438: Users can only claim for serials related to their branch. Only superlibrarian users and users with superserials permission can override this limitation. This patch adds a new subroutine C4::Serials::can_claim_subscription.
Created attachment 35178 [details] [review] Bug 8438: Followup Set the superserials permission for all users with the claim_serials permission
I've applied the patches against master 3.19.00.006 head 12889 On Home › Serials , clicking on Claims' link: Software error: Global symbol "@missingissues" requires explicit package name at [...]/serials/claims.pl line 83. Execution of [...]/serials/claims.pl aborted due to compilation errors. I pass the patch to "Failed QA" status.
Created attachment 35305 [details] [review] Bug 8438: Users can only claim for serials related to their branch. Only superlibrarian users and users with superserials permission can override this limitation. This patch adds a new subroutine C4::Serials::can_claim_subscription.
Created attachment 35306 [details] [review] Bug 8438: Followup Set the superserials permission for all users with the claim_serials permission
(In reply to Paola Rossi from comment #28) > I've applied the patches against master 3.19.00.006 head 12889 > > On Home › Serials , clicking on Claims' link: > > Software error: Sorry about that.
Created attachment 35311 [details] [review] [SIGNED OFF] Bug 8438: Users can only claim for serials related to their branch. I've applied the patch against master 3.19.00.006 head 12889 The upgrading of the DB was as required: for each [PT/S] patron with the permission "claim_serials" ON, the permission "superserials" became[/was kept to] ON. Then, after having checked the DB upgrading, to test the currently adding limitation: > Users can only claim for serials related to their branch , I reset 2 PT/S patrons-users to: permission claim_serials: YES permission superserials: NO and I set the syspref "IndependendeBranches" to "Prevent". For: > Only superlibrarian users can override this limitation. the S patron-user could list AND claim: A) subscriptions of his own branch, B) subscriptions of other branch, C) subscriptions without branch. For: > Only users with superserials permission can override this limitation. the PT patron-user could list: A) subscriptions of his own branch, B) subscriptions of other branch, C) subscriptions without branch, and could claim only: A) subscriptions of his own branch, C) subscriptions without branch. NB: a subscription is selected to be claimed. Then I set the syspref "IndependendeBranches" to "Don't prevent". The behaviour was [exactly the same as in master] without the added limitation. On [S/PT] patrons, if permission claim_serials was NO, no Claims' link was available on Serials' page, either under "Prevent" or under "Don't prevent". So everything was OK just as required. I pass the patch to "Signed Off" status.
Created attachment 35312 [details] [review] [SIGNED OFF] Bug 8438: Followup Set the superserials permission for all users with the claim_serials permission
I add to my comment 32: I reset the PT patrons-users to: [permission claim_serials: YES] permission superserials: YES under the syspref "IndependentBranches" set either to "Prevent" or to "Don't prevent". For: > Only users with superserials permission can override this limitation. the PT patron-user could list AND claim: A) subscriptions of his own branch, B) subscriptions of other branch, C) subscriptions without branch.
Ok, here it is: - We failed to get superserials and this patch set into the same version. - Superserials permission has been in use for quite a while now (see bug 8435) - This patch currently updates your permission set to add 'superserials' to it, if you had the permission to claim serials before. - We have a time gap here that will cause some change in behaviour, no matter how we do it. So this is the situation as I see it: 1) Not using IndependentBranches = Nothing needs to be done, because nothing should change. No need to update the permissions in that case. We could add an additional check for IndependentBranches to the database update. 2) Using IndependentBranches a) Having superserials permission already = Nothing changes b) Not having superserials permission before update = Can now suddenly manage serials from other branches after the update. If we don't update the permission: = Can suddenly no longer claim issues from other branches So what can we do here? - Introducing a 'superserialclaim permission' seems silly. - Taking away a smaller permission seems making more sense than giving access to even more data than before. - I'd vote for 'don't add the superserials permission on update'. Another thing: If I am not allowed to claim the issues from other branches, should these issues be visible to me? A user without superlibrarian/superserials will still see all issues that need claiming in the serial claim view, but they will miss the checkbox for claiming them. This doesn't seem to make sense, as I can't find the subscription using the search... but I can access it form the link. This is not very clean. I think the access problem could be on a separate bug ("Can still access other branches subsriptions with IndyBranches and no superserials"), but I'd really like to see a filtered view for the claims. Jonathan, could you take a look? Woudl also be good to get more opinions about b).
Comment on attachment 35312 [details] [review] [SIGNED OFF] Bug 8438: Followup Set the superserials permission for all users with the claim_serials permission So it is preferable not to update the user permissions and maybe let a note in the release notes to explain that if they have issues with serial permissions to check the user permissions.
> If we don't update the permission: > Can suddenly no longer claim issues from other branches > Woudl also be good to get more opinions about b). In view of the discussion above (including older comments at the top), this patch does not seem to be ready for inclusion yet. I am moving it to FQA for now. Will ask for additional comments on the list.
(In reply to Jonathan Druart from comment #36) > Comment on attachment 35312 [details] [review] [review] > [SIGNED OFF] Bug 8438: Followup Set the superserials permission for all > users with the claim_serials permission > > So it is preferable not to update the user permissions and maybe let a note > in the release notes to explain that if they have issues with serial > permissions to check the user permissions. Sounds like the best possible solution to me. Could you take a look if filtering out the issues from other branches would be possible instead of just hiding the claim options?
After discussing the problem with "seeing issues from other branches" with Jonathan a bit more I think we can keep it like that for now. - You will see them, but you can't claim them. If libraries think this should be more restrictive, please open a new bug report.
So back in the QA queue?
(In reply to Jonathan Druart from comment #40) > So back in the QA queue? The permission problem has not been solved? So, I would say: No. We can wait for some responses on the list first btw.
To me, there is no permission problem :)
I'd vote for: don't mingle with permissions, put a warn that now the behaviour is stricter and you can't claim issues from other branches without the proper permission for doing so.
Sadly, there was no reaction on the mailing list to the e-mail Marcel sent (thx for that!) I suggest moving this back into the queue - I personally see no cleaner way to do that, the only additions I can think of is a small warning in Release notes. To me this kind of fixes a bug - I can't search for other branches subscriptions without superserials/superlibrarian, but I can claim their late issues... that appears illogical to me.
Created attachment 43560 [details] [review] [PASSED QA] Bug 8438: Users can only claim for serials related to their branch. Only superlibrarian users and users with superserials permission can override this limitation. This patch adds a new subroutine C4::Serials::can_claim_subscription. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> The upgrading of the DB was as required: for each [PT/S] patron with the permission "claim_serials" ON, the permission "superserials" became[/was kept to] ON. Then, after having checked the DB upgrading, to test the currently adding limitation: > Users can only claim for serials related to their branch , I reset 2 PT/S patrons-users to: permission claim_serials: YES permission superserials: NO and I set the syspref "IndependendeBranches" to "Prevent". For: > Only superlibrarian users can override this limitation. the S patron-user could list AND claim: A) subscriptions of his own branch, B) subscriptions of other branch, C) subscriptions without branch. For: > Only users with superserials permission can override this limitation. the PT patron-user could list: A) subscriptions of his own branch, B) subscriptions of other branch, C) subscriptions without branch, and could claim only: A) subscriptions of his own branch, C) subscriptions without branch. NB: a subscription is selected to be claimed. Then I set the syspref "IndependendeBranches" to "Don't prevent". The behaviour was [exactly the same as in master] without the added limitation. On [S/PT] patrons, if permission claim_serials was NO, no Claims link was available on Serials' page, either under "Prevent" or under "Don't prevent". Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Added the results of Paola's testing to the commit message as test plan. Note to RM: Maybe we could add a note to the release notes, that Koha now enforces superserials with independent branches better, so people might have to adapt permissions in order to claim for other branches.
Patch pushed to master. Nice one Jonathan!