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.