http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14544
--- Comment #172 from Jonathan Druart <jonathan.dru...@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. _______________________________________________ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/