In both the OPAC and the staff client a "allowremovingitems" variable is used in the template to determine whether the logged in user should be able to delete items from a list. In both the OPAC and staff client this variable is true when it should be false. Fortunately the user can't actually remove something from the list. Instead the page returns an error message. However, the "remove" option should not be visible in that case. To confirm this bug, log in to the OPAC or staff client and open a public list which was created by another user. The "remove from list" option will be available but submitting the form will return an error like "You could not delete any selected items from this shelf."
Just from recall: Isn't it so that the variable is set to true (and the remove button is displayed) as long as it is possible that this user might be able to delete at least some items of this list? The permissions now differentiate deleting items you added yourself and items of someone else. It would not be very effective to do this check on item level every time you display a list? BICBW..
(In reply to M. de Rooy from comment #1) > Isn't it so that the variable is set to true > as long as it is possible that this user might > be able to delete at least some items of this list? Why would it be possible that I might be able to delete some items from a list which I didn't create and which wasn't created with permission for others to add items?
(In reply to Owen Leonard from comment #2) > Why would it be possible that I might be able to delete some items from a > list which I didn't create and which wasn't created with permission for > others to add items? Just thinking: Permissions can be changed at any time. So if I just disallowed adding items by others on my list, there could still be items added earlier on?
Created attachment 60228 [details] [review] Bug 10679: Hide delete option for lists if user does not have permission The reason the delete button was still showing is because, by default, we allow anyone to remove THEIR OWN contributions from a list. So, the delete button will always show. This patch makes sure that the delete button will only show if permission for anyone else to add or delete their own entries is granted. To test: 1) Make a public list setting all permissions to 'do not allow' EXCEPT 'remove own contributed entries' (set this to Allow). This should be done by default, but just make sure 2) Log in as a different user and go to that list. notice the 'remove selected' button shows, but attempting to delete any records fails. 3) Apply patch and refresh the page 4) Notice that the delete button is now gone. 5) Log back in as the list owner and set the first two permissions to 'Allow' (add entries, remove their own contributed entries) 6) Log back in as a different user and go to the list. The delete button should now be showing and work as expected. Sponsored-by: Catalyst IT
(In reply to Aleisha Amohia from comment #4) > The reason the delete button was still showing is because, by default, > we allow anyone to remove THEIR OWN contributions from a list. So, the > delete button will always show. This patch makes sure that the delete > button will only show if permission for anyone else to add or delete > their own entries is granted. The permission to add is not relevant here. The only way to make sure if you can delete is check permissions and optionally check if loggedinuser 'owns' entries on that list (or perhaps on the page we show). > To test: > 1) Make a public list setting all permissions to 'do not allow' EXCEPT > 'remove own contributed entries' (set this to Allow). This should be > done by default, but just make sure > 2) Log in as a different user and go to that list. notice the 'remove > selected' button shows, but attempting to delete any records fails. This assumption is not correct. If I added an entry to a public list and the owner later sets the list to 'D-A-D', I should still be able to delete my entry. But you removed the button !
Created attachment 60364 [details] [review] Bug 10679: Hide delete option for lists if user does not have permission and has not added any items to that list Taking from Comment 5, if a list once had permissions for people to add and remove entries, but then later the owner changes it so that others can no longer add entries, users should be able to delete the entries they added previously. This patch checks that the logged in user is the same account as the borrower who added the entry to the list. If they are, the button to remove should show. Attempting to delete any records they did not add will fail, but attempting to delete the records they did add will succeed. If they did not add any records to a list, the button to remove will not show. To test: 1) Make TWO public lists. Set all permissions to 'Allow' on one list and all permissions to 'Do not allow' on the other list. 2) Add one or two items to both lists. 3) Log in as a different user. Add one or two items to the list that you have permission to add to. 4) Log back in as the original user and set all permissions on the 'Allow' list to 'Do not allow' 5) Log back in as the different user. Go to the list you added entries to 6) The button to remove should show and you should be able to delete your entries only. 7) There should be no button to remove on the other list that you have added nothing to. Sponsored-by: Catalyst IT
We're not there yet. But we will! Will submit a more detailed comment later.
Instead of looping through the search results of Koha::Virtualshelfcontents->search({ shelfnumber => $self->shelfnumber }) please search for the column borrowernumber too. The algorithm should be something as: if list->count==0 return false if owner return true if delete_own AND delete_other return true (in that case you could delete all) if delete_own AND there are 'own' entries return true (so search here!) if delete_other AND there are 'other' entries (count - countown > 0) then return true Maybe you understand now why I did not write that code :) We should perhaps consider: show the button Remove selected only if you are the owner or allowed to delete all entries. And show a link on the individual entries if this is not the case but you can delete an individual one. Because if you see the button, it could well be that you are only allowed to delete one record out of 20. Which one? So you would need a routine can_biblio_be_removed (singular) Or simplify the permissions: Add/Delete on owner level (or staff member, moderator?) and Add/Delete for other users.
Please look at my proposal on bug 18228 to make things a bit easier.