|
Description
Paul Poulain
2012-02-03 16:58:14 UTC
*** Bug 7580 has been marked as a duplicate of this bug. *** *** Bug 9155 has been marked as a duplicate of this bug. *** Created attachment 17871 [details] [review] Add new features and tests to the budget duplication My colleague Maxime did a patch adding this feature to the budget duplication form. The patch also adds an option to transfer the ordered acquisitions into the new budget. Unit tests were also added. Patch sponsored by the CCSR ( http://www.ccsr.qc.ca ) Looks good on first glance and includes unit tests - contains a few strings so bumping to 3.14. Patch tested with a sandbox, by Cédric Vita <cedric.vita@dracenie.com> Created attachment 18341 [details] [review] Add new features and tests to the budget duplication : * Allow to enter a new name for the duplicate * Add option to transfer the not received orders in the budget copy * Transfer the duplication code in Budgets.pm to allow easier testing. In doing this I removed the code block " # deal with any children" because I just don't understand it and it had a typo in the table name ('aqcudgets'), so I figured it was useless code * Update Budgets.t with the right field names, test the data more and tests the duplication code. Sadly I didn't write tests for the order transfers because it just depends on too much stuff :/ http://bugs.koha-community.org/show_bug.cgi?id=7498 Signed-off-by: C QA comment: All look good. I have just a little remark: after click on 'duplicate', the description field is prefilled with the original description's budget. But if the user does not change it, nothing append (no error and no creation). It would be good if a message appears (JS on submitting or after reloading the page ). Marked as Failed QA. (In reply to comment #8) > QA comment: > > All look good. > I have just a little remark: after click on 'duplicate', the description > field is prefilled with the original description's budget. But if the user > does not change it, nothing append (no error and no creation). It would be > good if a message appears (JS on submitting or after reloading the page ). > > Marked as Failed QA. I don't understand this comment. Leaving the description to the original name works fine for me and it's a wanted feature. Could you explain more or test again ? Oops sorry Maxime, I had a unique key on my aqbudgetperiod.description field, but it does not exist on the current master! So all works as described. Unit tests are presents and a big bloc of code is moved in a pm file. Just 3 tabulation characters in the .t file, my signoff patch will fix that. Marked as Passed QA. Created attachment 18816 [details] [review] Add new features and tests to the budget duplication : * Allow to enter a new name for the duplicate * Add option to transfer the not received orders in the budget copy * Transfer the duplication code in Budgets.pm to allow easier testing. In doing this I removed the code block " # deal with any children" because I just don't understand it and it had a typo in the table name ('aqcudgets'), so I figured it was useless code * Update Budgets.t with the right field names, test the data more and tests the duplication code. Sadly I didn't write tests for the order transfers because it just depends on too much stuff :/ http://bugs.koha-community.org/show_bug.cgi?id=7498 Signed-off-by: C Created attachment 18817 [details] [review] Add new features and tests to the budget duplication : * Allow to enter a new name for the duplicate * Add option to transfer the not received orders in the budget copy * Transfer the duplication code in Budgets.pm to allow easier testing. In doing this I removed the code block " # deal with any children" because I just don't understand it and it had a typo in the table name ('aqcudgets'), so I figured it was useless code * Update Budgets.t with the right field names, test the data more and tests the duplication code. Sadly I didn't write tests for the order transfers because it just depends on too much stuff :/ http://bugs.koha-community.org/show_bug.cgi?id=7498 Signed-off-by: Cedric Vita <cedric.vita@dracenie.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Test cases do not pass: $ prove -v t/db_dependent/Budgets.t ... not ok 24 - 2 budgets for that budget period : the original and his child # Failed test '2 budgets for that budget period : the original and his child' # at t/db_dependent/Budgets.t line 119. # got: '1' # expected: '2' ok 25 ok 26 ok 27 not ok 28 # Failed test at t/db_dependent/Budgets.t line 123. # got: 'P�riodiques' # expected: 'Serials' ok 29 - DelBudget returned 1 # Looks like you failed 2 tests of 29. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/29 subtests Created attachment 29614 [details] [review] Bug 7498 - Cloning a budget, enable change of description Patch sponsored by the CCSR ( http://www.ccsr.qc.ca ) This patch allows to enter a new name for the duplicate budget and adds an option to transfer the not received orders in the budget copy. Took the previous patch, adapted it to the current master branch and made the tests work. Hi Charles, from the description of this, it seems to be a partial duplicate to bug 12164. 12164 adds options to move unreceived funds into the next year, but does not offer to rename the budget at time of duplication. What do you recommend, Katrin? Moving into next year is not the same as duplicating, but I see where they could collide. We were unaware of the "duplicate" situation, and the features (simply cloning, and renaming) still seem to have value to us (and certainly the sponsor). Hi Blou, I'd suggest you take a look at Jonathan's feature and talk to him. I think there is an overlap and it would be good to identify, where your feature does more than his patches do and then add those things separately. From the description - I haven't tested your patches - the only missing piece might be giving the new budget a different name in the same step as the duplicating. (In reply to Blou from comment #16) You should test the new feature (see bug 12164). It certainly answers to your needs. Created attachment 30562 [details] [review] This file builds on the work done for Bug 12164 This new patch is not a stand alone feature. It is now an extension of the work done for Bug 12168, giving the user the possibility to change the cloned budget's name. I've applied the patch against master 3.17.00.013. I pass the patch to "Signed Off" status. Created attachment 30785 [details] [review] 30562: This file builds on the work done for Bug 12164 The budget can be renamed at time of duplication. Created attachment 30827 [details] [review] [PASSED QA] Bug 7498 - Cloning a budget, enable change of description Patch sponsored by the CCSR ( http://www.ccsr.qc.ca ) This new patch builds on the work of Bug 12164 by allowing the user to enter a new name (budget_period_description) for the cloned budget. A test was added to t/db_dependent/Budgets.t. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works as described, passes tests and QA script. Nice feature! Patch pushed to master. Thanks Charles! |