|
Description
Jonathan Druart
2014-07-16 11:50:04 UTC
Created attachment 29746 [details] [review] Bug 12583: DelItem prototype - Remove $dbh Since C4::Context->dbh shares the DB handler, it's useless to pass it to routines. Test plan: Try to remove an item from the Koha interface. Verify that unit tests pass. Created attachment 29747 [details] [review] Bug 12583: DelItem prototype - prefer hashref To delete an item, only the itemnumber should be mandatory. The DelItem routine can retrieve the biblionumber from the itemnumber. Test plan: Verify that t/db_dependent/Items/DelItem.t passes Created attachment 29748 [details] [review] Bug 12583: DelItem prototype - Add unit tests DelItem should return 1 if the item has been deleted, otherwise 0. Test plan: Verify that t/db_dependent/Items/DelItem.t returns green Created attachment 30053 [details] [review] Bug 12583: DelItem prototype - Remove $dbh Since C4::Context->dbh shares the DB handler, it's useless to pass it to routines. Test plan: Try to remove an item from the Koha interface. Verify that unit tests pass. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 30054 [details] [review] Bug 12583: DelItem prototype - prefer hashref To delete an item, only the itemnumber should be mandatory. The DelItem routine can retrieve the biblionumber from the itemnumber. Test plan: Verify that t/db_dependent/Items/DelItem.t passes Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 30055 [details] [review] Bug 12583: DelItem prototype - Add unit tests DelItem should return 1 if the item has been deleted, otherwise 0. Test plan: Verify that t/db_dependent/Items/DelItem.t returns green Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 30110 [details] [review] [PASSED QA] Bug 12583: DelItem prototype - Remove $dbh Since C4::Context->dbh shares the DB handler, it's useless to pass it to routines. Test plan: Try to remove an item from the Koha interface. Verify that unit tests pass. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 30111 [details] [review] [PASSED QA] Bug 12583: DelItem prototype - prefer hashref To delete an item, only the itemnumber should be mandatory. The DelItem routine can retrieve the biblionumber from the itemnumber. Test plan: Verify that t/db_dependent/Items/DelItem.t passes Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 30112 [details] [review] [PASSED QA] Bug 12583: DelItem prototype - Add unit tests DelItem should return 1 if the item has been deleted, otherwise 0. Test plan: Verify that t/db_dependent/Items/DelItem.t returns green Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Looks great! We should do this for DelItemCheck as well. Patches pushed to master. Thanks Jonathan! P.S. I renamed the unit tests file. I'll propose a discussion on naming schemas and arrangement. See also Bug 10869, in which I first removed the useless parameter in DelOrder as did Jonathan, but changed my mind after a comment by M de Rooy. "Just a *dumb* question: Why remove the biblio parameter and look it up in the database if you already had it in the first place?" Mathieu Saby Sorry, I'm confused. Your patch was about biblionumber beeing passed to DelItem, mine was about biblionumber beeing passed to DelOrder... Mathieu |