Summary: | Cannot update patron categories | ||
---|---|---|---|
Product: | Koha | Reporter: | Katrin Fischer <katrin.fischer> |
Component: | System Administration | Assignee: | Pedro Amorim <pedro.amorim> |
Status: | CLOSED FIXED | QA Contact: | Katrin Fischer <katrin.fischer> |
Severity: | blocker | ||
Priority: | P5 - low | CC: | david, fridolin.somers, gmcharlt, jonathan.druart |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
23.11.00
|
|
Circulation function: | |||
Bug Depends on: | 18203 | ||
Bug Blocks: | 35556 | ||
Attachments: |
Bug 35263: Add selenium tests
Bug 35263: Default 'can_place_ill_in_opac' to 1 Bug 35263: Add selenium tests Bug 35263: Default 'can_place_ill_in_opac' to 1 Bug 35263: Add selenium tests Bug 35263: Default 'can_place_ill_in_opac' to 1 Bug 35263: QA follow-up: Update code Bug 35263: Add selenium tests Bug 35263: Default 'can_place_ill_in_opac' to 1 Bug 35263: (QA follow-up) Update code |
Description
Katrin Fischer
2023-11-05 12:23:13 UTC
Created attachment 158495 [details] [review] Bug 35263: Add selenium tests Test plan, on k-t-d selenium (ktd --selenium up): 1) Apply just this tests patch 2) Run: prove t/db_dependent/selenium/administration_tasks.t 3) Notice it fails. Apply the other patch. Restart plack 4) Repeat step 2. Notice it passes. Created attachment 158496 [details] [review] Bug 35263: Default 'can_place_ill_in_opac' to 1 If ILLModule sys pref is 'off', this input param is undef. The database does not allow it to be nullable causing an error. Check if its defined (i.e. the option was submitted) and use that, if not default to 1 Test plan, k-t-d: 1) Edit an existing patron category, visit: /cgi-bin/koha/admin/categories.pl?op=add_form&categorycode=B 2) Click 'Save' at the bottom of the form 3) Notice a yellow box is shown with an error message 4) Apply patch. Repeat steps 1-3. Notice it now updates successfully. Created attachment 158543 [details] [review] Bug 35263: Add selenium tests Test plan, on k-t-d selenium (ktd --selenium up): 1) Apply just this tests patch 2) Run: prove t/db_dependent/selenium/administration_tasks.t 3) Notice it fails. Apply the other patch. Restart plack 4) Repeat step 2. Notice it passes. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 158544 [details] [review] Bug 35263: Default 'can_place_ill_in_opac' to 1 If ILLModule sys pref is 'off', this input param is undef. The database does not allow it to be nullable causing an error. Check if its defined (i.e. the option was submitted) and use that, if not default to 1 Test plan, k-t-d: 1) Edit an existing patron category, visit: /cgi-bin/koha/admin/categories.pl?op=add_form&categorycode=B 2) Click 'Save' at the bottom of the form 3) Notice a yellow box is shown with an error message 4) Apply patch. Repeat steps 1-3. Notice it now updates successfully. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> I was going to add my sign-off, so Katrin's could count as QA. However, the tests fail for me after the patch is applied - are there other system preferences that need enabling/disabling (tested using KTD)?: kohadev-koha@kohadevbox:koha(bz35263)$ prove t/db_dependent/selenium/administration_tasks.t t/db_dependent/selenium/administration_tasks.t .. Selenium server did not return proper status at /usr/share/perl5/Selenium/Remote/Driver.pm line 552. # Looks like your test exited with 22 before it could output anything. t/db_dependent/selenium/administration_tasks.t .. Dubious, test returned 22 (wstat 5632, 0x1600) Failed 3/3 subtests Test Summary Report ------------------- t/db_dependent/selenium/administration_tasks.t (Wstat: 5632 Tests: 0 Failed: 0) Non-zero exit status: 22 Parse errors: Bad plan. You planned 3 tests but ran 0. Files=1, Tests=0, 1 wallclock secs ( 0.00 usr 0.01 sys + 0.82 cusr 0.14 csys = 0.97 CPU) Result: FAIL I get the same result whether ILLModule is enabled or not. The patches do fix the error when editing a patron category though... Hm, I just double checked, tests are passing nicely on a new database for me: OK admin/categories.pl OK t/db_dependent/selenium/administration_tasks.t Processing additional checks OK! Running tests (1) * Proving /kohadevbox/koha/t/db_dependent/selenium/administration_tasks.t OK! kohadev-koha@kohadevbox:koha(71-35263-update-atron-category)$ prove t/db_dependent/selenium/administration_tasks.t t/db_dependent/selenium/administration_tasks.t .. ok All tests successful. Files=1, Tests=3, 18 wallclock secs ( 0.01 usr 0.01 sys + 1.03 cusr 0.15 csys = 1.20 CPU) Result: PASS (In reply to David Nind from comment #5) > I was going to add my sign-off, so Katrin's could count as QA. > > However, the tests fail for me after the patch is applied - are there other > system preferences that need enabling/disabling (tested using KTD)?: > I didn't read the test plan properly - needed to start KTD with --selenium (ktd --selenium up). Created attachment 158566 [details] [review] Bug 35263: Add selenium tests Test plan, on k-t-d selenium (ktd --selenium up): 1) Apply just this tests patch 2) Run: prove t/db_dependent/selenium/administration_tasks.t 3) Notice it fails. Apply the other patch. Restart plack 4) Repeat step 2. Notice it passes. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: David Nind <david@davidnind.com> Created attachment 158567 [details] [review] Bug 35263: Default 'can_place_ill_in_opac' to 1 If ILLModule sys pref is 'off', this input param is undef. The database does not allow it to be nullable causing an error. Check if its defined (i.e. the option was submitted) and use that, if not default to 1 Test plan, k-t-d: 1) Edit an existing patron category, visit: /cgi-bin/koha/admin/categories.pl?op=add_form&categorycode=B 2) Click 'Save' at the bottom of the form 3) Notice a yellow box is shown with an error message 4) Apply patch. Repeat steps 1-3. Notice it now updates successfully. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: David Nind <david@davidnind.com> Will be better written this way IMO: my $can_place_ill_in_opac = defined $input->param('can_place_ill_in_opac') ? $input->param('can_place_ill_in_opac') : 1; => my $can_place_ill_in_opac = $input->param('can_place_ill_in_opac') // 1; Created attachment 158588 [details] [review] Bug 35263: QA follow-up: Update code Thank you Jonathan. I first tried with '||' but it would fallback to 1 for either undef or 0 (we want the first case, but not the second). But using '//' instead, it only falls back to 1 if undef, and not when 0. Today I learned between '||' and '//'! Thanks! From perldoc.perl.org (relating to '//'): 'In fact, it's exactly the same as ||, except that it tests the left hand side's definedness instead of its truth.' Processing additional checks * Commit title does not contain 'follow-up' correctly spelt - a0b13696d3 * Commit title does not contain '(QA follow-up)' correctly spelt - a0b13696d3 Fixing. Created attachment 158633 [details] [review] Bug 35263: Add selenium tests Test plan, on k-t-d selenium (ktd --selenium up): 1) Apply just this tests patch 2) Run: prove t/db_dependent/selenium/administration_tasks.t 3) Notice it fails. Apply the other patch. Restart plack 4) Repeat step 2. Notice it passes. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: David Nind <david@davidnind.com> Created attachment 158634 [details] [review] Bug 35263: Default 'can_place_ill_in_opac' to 1 If ILLModule sys pref is 'off', this input param is undef. The database does not allow it to be nullable causing an error. Check if its defined (i.e. the option was submitted) and use that, if not default to 1 Test plan, k-t-d: 1) Edit an existing patron category, visit: /cgi-bin/koha/admin/categories.pl?op=add_form&categorycode=B 2) Click 'Save' at the bottom of the form 3) Notice a yellow box is shown with an error message 4) Apply patch. Repeat steps 1-3. Notice it now updates successfully. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: David Nind <david@davidnind.com> Created attachment 158635 [details] [review] Bug 35263: (QA follow-up) Update code Thank you Jonathan. I first tried with '||' but it would fallback to 1 for either undef or 0 (we want the first case, but not the second). But using '//' instead, it only falls back to 1 if undef, and not when 0. Today I learned between '||' and '//'! Thanks! From perldoc.perl.org (relating to '//'): 'In fact, it's exactly the same as ||, except that it tests the left hand side's definedness instead of its truth.' Thanks a lot for the commit message, I also learned the difference today :) Pushed to master for 23.11. Nice work everyone, thanks! Depends on Bug 18203 not in 23.05.x |