http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8836
Katrin Fischer <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #70 from Katrin Fischer <[email protected]> --- Hi Kyle, starting with a code review of the new patches. 1. CODE REVIEW 1.1 QA script Problem (normal) We got a problem with perl critic: FAIL C4/Circulation.pm FAIL critic # Variables::ProhibitConditionalDeclarations: Got 1 violation(s). 1.2 Code Note: Be careful with GetReserveStatus, it's not always working correctly. 'waiting' works, but other status are problematic, see bug 10697. + transferbook( $colBranchcode, $item->{barcode}, + my $ignore_reserves = 1 ) + unless ( GetReserveStatus( $item->{itemnumber} ) eq "Waiting" ); GetReserveStatus works for 'Waiting' but is problematic for any other status (see bug 10697) 1.3 Database structure Question: Why is the constraint in kohastructure.sql added by a separate statement? +-- Constraints for table `collections` +-- +ALTER TABLE `collections` + ADD CONSTRAINT `collections_ibfk_1` FOREIGN KEY (`colBranchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE 2. FEATURE TEST As this brings the Rotating collections feature back to life, I think we also need to test the feature here. I know that the code has been written a while ago, so I will add notes on what I think NEED to be fixed in order for this to go back in at the bottom, while also noting other things I see while testing this. 2.1 Documentation 2.1.1 Question: Is there a good documentation about the feature somewhere? Feeling a bit like flying blind here :) 2.1.2 Problem (normal, separate bug) We have no help pages for the Rotating collections 2.2 Navigation 2.2.1 Problem (normal) There is no entry for "Rotating collections" on the side navigation when you are on a tool's page. 2.2.2 Problem (normal) The main page of the Rotating collection page is missing the side navigation altogether. 2.2.3 Hm. In several places we have a button to return to the Rotating collections start page, maybe this could be replaced by the side navigation link? 2.3 Adding a rotating collection 2.3.1 Problem (minor) The 'new collection' form should have 'Title' marked as required. 2.3.2 Problem (normal) The error messages stem from RotatingCollections.pm and are not translatable (see bug 11595). Please fix! 2.3.3 Problem (trivial) The table shows the branch code instead of the description name, easy to fix today, with TT! 2.3.3 Hm. It feels a bit weird, that I have to use a button to get to a page where I can edit the collections. Why not have this functionality accessible from the table on the start page? (Adding "Edit" and "Delete" links to the table and a "New collection" button at the top? This way it would fit in a bit better with Koha's usual way of doing things. 2.4 Rotating collections home 2.4.1 Problem (trivial) Several capitalization 'problems' on rotatingCollections.pl. 2.4.1 Problem (trivial) The table shows the branch code instead of the description name, easy to fix today, with TT! 2.5 Adding/removing items 2.5.1 Problem (normal) The error messages stem from RotatingCollections.pm and are not translatable (see also bug 11595). Please fix! 2.5.2 Problem (trivial) Error messages should use the usual styling to stand out a bit more. Example: remove a barcode that is not in the collection. 2.5.3 Problem (trivial) I am not sure this error makes sense to the user: Reason: No Itemnumber Given I think it would be better to say: Barcode doesn't exist (or similar) 2.5.4 Enhancement idea Removing an item from the collection currently requires that you enter the barcode. It would be nice to also have a 'Delete' link in the table. 2.6 Transfers Collection goes to Troy. 2.6.1 OK Checking in an item that is "waiting" in Centerville The rotating collection message shows, but also the hold message. I confirm the hold message. The item is still 'waiting' for the patron. No transfer is generated. 2.6.2 OK Check-out the waiting item in Centerville to the borrower. 2.6.3 OK Check-in item in Centerville. There is another hold in Springfield for another patron. Confirm transfer. Transfer is generated. 2.6.4 OK Check-in of the item in Springfield. Confirm hold. Transfer is completed. 2.6.5 OK Check-out to the patron the item is waiting for. 2.6.6 OK? Check-in of the item in _Centerville_ Patron returns the item at another branch. I still see the dialog, that the item shoudl go to "Troy", but no transfer message is generated, is this the way it's supposed to work? Note: not sure, but should the rotating collections message show up, when there is also a hold/other transfer request? SUMMARY Please check everything noted as a 'Problem' specifically the perl critic and the translation issues. Some of the other things are easy to fix too :) Can you also summarize for me, what the suspected behaviour is (2.6.6)? -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] 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/
