Description
Bernardo Gonzalez Kriegel
2014-07-29 20:14:22 UTC
Created attachment 30306 [details] [review] Bug 12675: Remove CGI::scrolling_list from labeledMARCdetail.pl This patch removes only instance in this file. Also updates getframework POD on C4/Koha.pm, adds new GetFrameworksLoop() func on same file from suggested code, but with ordered result. To test: 1. Apply the patch 2. Enable viewLabeledMARC syspref 3. On staff, search for a record, goto detail view 4. Clic on Labeled MARC 5. Framework pulldown was replaced, check changing framework. A bug was fixed, because selecting any fw and then Default tries to load values from 'Default' fw code, which does not exists. This still needs a unit test, trying to figure out how to write it :) Created attachment 30408 [details] [review] Bug 12675: Remove CGI::scrolling_list from labeledMARCdetail.pl This patch removes only instance in this file. Also updates getframework POD on C4/Koha.pm, adds new GetFrameworksLoop() func on same file from suggested code, but with ordered result. To test: 1. Apply the patch 2. Enable viewLabeledMARC syspref 3. On staff, search for a record, goto detail view 4. Clic on Labeled MARC 5. Framework pulldown was replaced, check changing framework. A bug was fixed, because selecting any fw and then Default tries to load values from 'Default' fw code, which does not exists. Created attachment 30409 [details] [review] Bug 12675: Add unit test to GetFrameworksLoop to test: 1. Apply the patch 2. Run the test, prove t/db_dependent/Koha.t 3. Check that it pass Created attachment 30413 [details] [review] Bug 12675: Add unit test to GetFrameworksLoop to test: 1. Apply the patch 2. Run the test, prove t/db_dependent/Koha.t 3. Check that it pass Ready to sign Created attachment 30436 [details] [review] [SIGNED-OFF] Bug 12675: Remove CGI::scrolling_list from labeledMARCdetail.pl This patch removes only instance in this file. Also updates getframework POD on C4/Koha.pm, adds new GetFrameworksLoop() func on same file from suggested code, but with ordered result. To test: 1. Apply the patch 2. Enable viewLabeledMARC syspref 3. On staff, search for a record, goto detail view 4. Clic on Labeled MARC 5. Framework pulldown was replaced, check changing framework. A bug was fixed, because selecting any fw and then Default tries to load values from 'Default' fw code, which does not exists. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 30437 [details] [review] [SIGNED-OFF] Bug 12675: Add unit test to GetFrameworksLoop to test: 1. Apply the patch 2. Run the test, prove t/db_dependent/Koha.t 3. Check that it pass Signed-off-by: Owen Leonard <oleonard@myacpl.org> Hm Bernardo, could you take a look at the test file? It works, but I wonder if $dbh->rollback(); should not be outside of the last subtest. Also, the new subtests are the only ones not printing anything when doing perl t/db_dependent/Koha.t - but I can see there are comments in the file. Please switch back to Signed off if the changes are not too big. Created attachment 30505 [details] [review] [SIGNED-OFF] Bug 12675: Add unit test to GetFrameworksLoop to test: 1. Apply the patch 2. Run the test, prove t/db_dependent/Koha.t 3. Check that it pass Signed-off-by: Owen Leonard <oleonard@myacpl.org> (In reply to Katrin Fischer from comment #9) > Hm Bernardo, could you take a look at the test file? It works, but I wonder > if $dbh->rollback(); should not be outside of the last subtest. Also, the > new subtests are the only ones not printing anything when doing > perl t/db_dependent/Koha.t - but I can see there are comments in the file. > > Please switch back to Signed off if the changes are not too big. Katrin, I moved the roolback() to the end of file outside subtest. It's the only change. Now, messages. Using perl t/db_dependent/Koha.t I got # Subtest: GetFrameworksLoop() tests 1..6 ok 1 - No frameworks ok 2 - All frameworks ok 3 - None selected ok 4 - One selected ok 5 - Ordered result ok 6 - Full check, sorted by description with selected val (Bug 12675) ok 8 - GetFrameworksLoop() tests Do they not shown to you? You are right, I think I miread it expecting it to be below the last line seeing it as the heading. Sorry! Created attachment 30507 [details] [review] [PASSED QA] Bug 12675: Remove CGI::scrolling_list from labeledMARCdetail.pl This patch removes only instance in this file. Also updates getframework POD on C4/Koha.pm, adds new GetFrameworksLoop() func on same file from suggested code, but with ordered result. To test: 1. Apply the patch 2. Enable viewLabeledMARC syspref 3. On staff, search for a record, goto detail view 4. Clic on Labeled MARC 5. Framework pulldown was replaced, check changing framework. A bug was fixed, because selecting any fw and then Default tries to load values from 'Default' fw code, which does not exists. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works as described, passes tests and QA script. Created attachment 30508 [details] [review] [PASSED QA] Bug 12675: Add unit test to GetFrameworksLoop to test: 1. Apply the patch 2. Run the test, prove t/db_dependent/Koha.t 3. Check that it pass Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Patches pushed to master. New function, proper unit tests! Thanks Bernardo! |