Bug 14889 - Move the framework related code to Koha::BiblioFramework[s]
Summary: Move the framework related code to Koha::BiblioFramework[s]
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 15449 15801 16807
  Show dependency treegraph
 
Reported: 2015-09-24 15:08 UTC by Jonathan Druart
Modified: 2017-12-07 22:15 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 14889: Add Koha::BiblioFramework[s] classes (3.00 KB, patch)
2015-09-24 15:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14889: Use Koha::BiblioFramework[s] in admin/biblio_framework.pl (25.97 KB, patch)
2015-09-24 15:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14889: Add Koha::BiblioFramework[s] classes (3.08 KB, patch)
2015-10-22 18:09 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14889: Use Koha::BiblioFramework[s] in admin/biblio_framework.pl (26.10 KB, patch)
2015-10-22 18:09 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 14889: QA Follow-up (2.24 KB, patch)
2015-10-25 09:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14889: Fix error msg on duplicate (2.22 KB, patch)
2015-10-26 08:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14889: Add tests for Koha::BiblioFramework[s] (2.36 KB, patch)
2015-10-26 08:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14889: Add Koha::BiblioFramework[s] classes (3.07 KB, patch)
2015-11-05 12:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14889: Use Koha::BiblioFramework[s] in admin/biblio_framework.pl (26.09 KB, patch)
2015-11-05 12:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14889: QA Follow-up (2.24 KB, patch)
2015-11-05 12:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14889: Fix error msg on duplicate (2.22 KB, patch)
2015-11-05 12:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14889: Add tests for Koha::BiblioFramework[s] (2.36 KB, patch)
2015-11-05 12:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 14889: Add Koha::BiblioFramework[s] classes (3.13 KB, patch)
2015-11-19 10:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14889: Use Koha::BiblioFramework[s] in admin/biblio_framework.pl (26.16 KB, patch)
2015-11-19 10:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14889: QA Follow-up (2.31 KB, patch)
2015-11-19 10:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14889: Fix error msg on duplicate (2.29 KB, patch)
2015-11-19 10:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14889: Add tests for Koha::BiblioFramework[s] (2.42 KB, patch)
2015-11-19 10:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14889: Fix typo in Koha/BiblioFrameworks.pm (621 bytes, patch)
2016-02-10 15:59 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2015-09-24 15:08:01 UTC

    
Comment 1 Jonathan Druart 2015-09-24 15:10:44 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2015-09-24 15:10:47 UTC Comment hidden (obsolete)
Comment 3 Bernardo Gonzalez Kriegel 2015-10-22 18:09:31 UTC Comment hidden (obsolete)
Comment 4 Bernardo Gonzalez Kriegel 2015-10-22 18:09:40 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2015-10-25 09:36:44 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2015-10-25 09:38:58 UTC
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!
Comment 7 Jonathan Druart 2015-10-26 08:57:16 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2015-10-26 08:57:19 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2015-11-05 12:17:56 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2015-11-05 12:17:59 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2015-11-05 12:18:04 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2015-11-05 12:18:07 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2015-11-05 12:18:10 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2015-11-19 10:43:10 UTC
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>
Comment 15 Katrin Fischer 2015-11-19 10:43:14 UTC
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>
Comment 16 Katrin Fischer 2015-11-19 10:43:17 UTC
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>
Comment 17 Katrin Fischer 2015-11-19 10:43:20 UTC
Created attachment 44988 [details] [review]
[PASSED QA] Bug 14889: Fix error msg on duplicate

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 18 Katrin Fischer 2015-11-19 10:43:24 UTC
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>
Comment 19 Brendan Gallagher 2015-12-31 19:10:00 UTC
Pushed to master - Thanks!
Comment 20 Jonathan Druart 2016-02-10 15:59:59 UTC
Created attachment 47857 [details] [review]
Bug 14889: Fix typo in Koha/BiblioFrameworks.pm
Comment 21 Jonathan Druart 2016-02-10 16:00:53 UTC
Just found a typo in the patchset, @RM could you please push the *last* patch please?
Comment 22 Katrin Fischer 2016-02-28 21:01:18 UTC
Has ths happened yet?
Comment 23 Katrin Fischer 2016-02-28 21:01:43 UTC
Ah, all need to be pushed still. Sorry, misunderstood!
Comment 24 Brendan Gallagher 2016-03-02 22:52:39 UTC
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
Comment 25 Jonathan Druart 2016-03-03 08:31:41 UTC
(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.
Comment 26 Brendan Gallagher 2016-03-03 20:37:34 UTC
Ah thanks Jonathan - I thought it was something silly like that.  Ok last patch pushed to master - may 2016 release target.
Comment 27 Marcel de Rooy 2016-04-21 07:21:28 UTC
The unit test BiblioFrameworks.t does not have a transaction!
This is corrected with a patch under report 16155.