http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14544 --- Comment #172 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #171)
QA Comment: Great job !!! Lot of work involved.
Thanks a lot for Qaing this! Very useful feedback :)
Still have some points for consideration:
OPAC -- View contents of a list -- Send shelf - Crash Can't call method "can_be_viewed" on an undefined value at /home/koha/testclone/opac/opac-sendshelf.pl line 55. *BLOCKER*
Fixed! The shelfnumber param was not passed. I have also add a test in the pl file to avoid this crash when viewing an non-existing shelf (now display 'not authorized', which is the current behavior).
OPAC - View contents of a list - Sort by callnumber - Bang DBIx::Class::ResultSet::next(): Unknown column 'itemcallnumber' in 'order clause' at /home/koha/testclone/opac/opac-shelves.pl line 236 *BLOCKER*
Fixed! But... This is a bad one. In opac-shelves.pl and shelves.pl, the join was only done on biblio. But itemcallnumber is in the items table. I had to add the biblio<=>biblioitems<=>items relationship to the Schema and a PK on virtualshelfcontents table (DBIx::Class::Storage::DBIHacks::_adjust_select_args_for_complex_prefetch(): Unable to perform complex limited prefetch off Virtualshelfcontent without declared primary key). Fixed in a separate patch. Note that bug 14819 is stuck in the NSO status but also adds the relationships.
OPAC - Lists- Open a shared list where you do not have permission to remove items of someone else. Try to remove such an item. No success, but no warning too! IIRC There was a warning in the past about some items not deleted.. Now try to remove a item you added yourself to that list while having permission to remove your own entries. This does not work ! No items are deleted, no message. I am inclined to see this last point as a *BLOCKER* too.
Fixed! The "op" was "view" instead of "remove_biblios" on the submit button. Note that the individual links work (and intranet is ok).
OPAC - View list - Delete your own item while the perms are ADD or DDD. Should not be possible. But the owner is able to now. This is change of behavior and not consistent with the wording of those permissions "Do not allow anyone to remove his own contributed entries" (including your own!). I would view it as a sort of Readonly list where you cannot accidentally delete your own items. (Note however that adding is still possible.) I am inclined to see this last point as a *BLOCKER* too (but I may not be completely neutral here :) + # FIXME + # This does not make sense, but it's has been backported from DelFromShelf. + # Why shouldn't we allow to remove his own contribution if allow_delete_other is on?
Well, not fixed. I have asked on #koha and Nicole agreed (http://irc.koha-community.org/koha/2015-08-18#i_1716925). So I am not sure what to do here.
Code Exceptions.pm module comes in now with a dependency Exception::Class. The description => "poeut" still needs some attention apparently :) Not sure what it means btw.. Although this looks good to me on itself, this dev should probably better have gone on its own report. Note there is another report too (13995). It kind of creeps in now.
poeut is a typo for pouet which does not mean anything :) The exceptions implementation can be modified easily later (the question was: should we put all exceptions in 1 file, all exceptions of a given module in 1 file or 1 exception per file).
OPAC - Share a list - Enter email address - Return to your lists: This list does not exist ?? op=view && category=1. Adjust the URL?
Fixed! Yes, the op is 'list', not 'view".
OPAC - I shared a list with AAD perms. The other user could add a few entries. After that removed the share. Now come back as owner of the list and try to remove one item of yourself and one of the other user. You will get the message: The item has been removed from the list. This is not completely true: You could only remove one item; the other is not permitted.
This is a specific case and I would prefer to manage it in a separate bug report. I am not sure this work on master :)
OPAC - Create a new list - Starts with permissions D D D - Would it not be better to default to D A D ? Note that if I create a list with save to new list, it does default to DAD. So this is a minor inconsistency..
Yes, and it's a regression. Fixed on both interface (I have c/p OPAC code for the intranet).
OPAC - View a list: For a list with ADD or DDD perms, the button Remove could be hidden or disabled? Might be the case now too btw.
Not easily, and there is no regression here: it's how it works currently. The problem is that the queries done in remove_biblios should be executed for each biblio in the list. Maybe not a big deal, to investigate...
OPAC Detail - List(s) this item appears in: [nothing filled while expected] I click from view a (private) list on one entry. So go to opac detail. Should I expect that list to be mentioned now? It is not.
Fixed! In the template shelves->count should be check, not only the shelves variable (which is always defined).
Staff - View public lists [Related to another report] With the delete public lists permission I can delete the public list of another user. OK The staff user probably has a good reason to do so. Just noting that it would perhaps be more friendly to switch it to Private instead of a rigourous Delete?
Todo later yes.
Code in opac-addbybiblionumber.pl: HandleSelectedShelf: Empty TODO in an else branch. Noting for the record.
Yes, I preferred to add the TODO marked than nothing, but no regression here.
Code t/db_dependent/Utils/Datatables_Virtualshelves.t and C4/Utils/DataTables/VirtualShelves.pm This is only justified/used by svc/virtualshelves/search Isn't this the time to move this svc script to Koha::Virtualshelves too ? Or at least on another report ?
Not ready yet. The rewrite should use the REST api.
Database design: Do we still need (or use) virtualshelfcontents.flags ? Could a dbrev remove it now?
To investigate, but yes, I am not sure to have found any usefulness to this column.
An observation (which could be considered as outside the scope): Add an item to the list with a barcode (in staff) is somewhat strange. You cannot add items, only biblios. This is current situation already. Just for completeness :)
Changing the wording could maybe fix the ambiguity. -- You are receiving this mail because: You are watching all bug changes.