Description
Jonathan Druart
2015-09-24 15:08:01 UTC
Created attachment 42846 [details] [review] Bug 14889: Add Koha::BiblioFramework[s] classes Created attachment 42847 [details] [review] Bug 14889: Use Koha::BiblioFramework[s] in admin/biblio_framework.pl Test plan: Add/edit/remove biblio frameworks from the administration module (admin/biblio_framework.pl). You should get message feedback after each action. Created attachment 43775 [details] [review] [SIGNED-OFF] Bug 14889: Add Koha::BiblioFramework[s] classes Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Created attachment 43776 [details] [review] [SIGNED-OFF] Bug 14889: Use Koha::BiblioFramework[s] in admin/biblio_framework.pl Test plan: Add/edit/remove biblio frameworks from the administration module (admin/biblio_framework.pl). You should get message feedback after each action. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> No problems found. Tested add/edit/delete No koha-qa errors Created attachment 43944 [details] [review] Bug 14889: QA Follow-up - Removes unused package from BiblioFramework.pm - Makes framework table sortable - Some small changes in wording of error messages I have added a follow-up - but there are 2 things: 1) Please add some unit tests. 2) When you add a new framework with the same frameworkcode than an existing framework, I think it should display the appropriate message, but instead, it updates the existing framework description. On current master, you have a silent SQL erorr in the logs, but the existing framework is left unchanged. (not ideal either...) Otherwise this works quite well, so almost there! Created attachment 43974 [details] [review] Bug 14889: Fix error msg on duplicate Created attachment 43975 [details] [review] Bug 14889: Add tests for Koha::BiblioFramework[s] Created attachment 44493 [details] [review] Bug 14889: Add Koha::BiblioFramework[s] classes Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Created attachment 44494 [details] [review] Bug 14889: Use Koha::BiblioFramework[s] in admin/biblio_framework.pl Test plan: Add/edit/remove biblio frameworks from the administration module (admin/biblio_framework.pl). You should get message feedback after each action. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> No problems found. Tested add/edit/delete No koha-qa errors Created attachment 44495 [details] [review] Bug 14889: QA Follow-up - Removes unused package from BiblioFramework.pm - Makes framework table sortable - Some small changes in wording of error messages Created attachment 44496 [details] [review] Bug 14889: Fix error msg on duplicate Created attachment 44497 [details] [review] Bug 14889: Add tests for Koha::BiblioFramework[s] Created attachment 44985 [details] [review] [PASSED QA] Bug 14889: Add Koha::BiblioFramework[s] classes Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44986 [details] [review] [PASSED QA] Bug 14889: Use Koha::BiblioFramework[s] in admin/biblio_framework.pl Test plan: Add/edit/remove biblio frameworks from the administration module (admin/biblio_framework.pl). You should get message feedback after each action. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> No problems found. Tested add/edit/delete No koha-qa errors Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44987 [details] [review] [PASSED QA] Bug 14889: QA Follow-up - Removes unused package from BiblioFramework.pm - Makes framework table sortable - Some small changes in wording of error messages Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44988 [details] [review] [PASSED QA] Bug 14889: Fix error msg on duplicate Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44989 [details] [review] [PASSED QA] Bug 14889: Add tests for Koha::BiblioFramework[s] Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Pushed to master - Thanks! Created attachment 47857 [details] [review] Bug 14889: Fix typo in Koha/BiblioFrameworks.pm Just found a typo in the patchset, @RM could you please push the *last* patch please? Has ths happened yet? Ah, all need to be pushed still. Sorry, misunderstood! Didn't apply cleanly - I know because of the moving target that master is. Please rebase. Once it's done - leave me an @later in #koha and I'll jump right on it. Thank you so much for the time and bother of rebasing. Brendan (In reply to Brendan Gallagher from comment #24) > Didn't apply cleanly - I know because of the moving target that master is. > Please rebase. Once it's done - leave me an @later in #koha and I'll jump > right on it. Thank you so much for the time and bother of rebasing. > > Brendan Brendan, we just need the last patch, others are already pushed. Ah thanks Jonathan - I thought it was something silly like that. Ok last patch pushed to master - may 2016 release target. The unit test BiblioFrameworks.t does not have a transaction! This is corrected with a patch under report 16155. |