There are some improvements needed to the code in bug 34355
Comments from bug 34355 1. Missing tests! 2. Useless lines + my @messages; +use Net::FTP; 3. Plugin hook added without being advertised (wiki, kitchenzink?) + 'before_orderline_create', 4. Code for acq not in Koha::Acquisition? 5. controller poorly written and not following standard admin/marc_order_accounts.pl * useless use statements * 51 my $crypt = Koha::Encryption->new; wtf? * values for '$op' not following "standard" * no error handling, no user messages/alerts
Created attachment 175018 [details] [review] Bug 38547: Move code to Koha/Acquisitions and add more tests Test plan: 1) prove t/db_dependent/Koha/Acquisition/MarcOrder.t
Created attachment 175019 [details] [review] Bug 38547: Bring controller in line with modern standards Test plan: 1) In system preferences, click Search and then select the Acquisitions option from the left hand menu 2) Enable MarcOrderingAutomation 3) In the administration homepage, under Acquisition parameters there should now be a link called "MARC order accounts" 4) Click on this and click the New account button 5) Create a new account and then experiment with editing and deleting the account. 6) These actions should all work and there should be bootstrap messages displayed for each action
Hi Matt! Could you please rebase this? Thanks, --Laura E.
Created attachment 181676 [details] [review] Bug 38547: Move code to Koha/Acquisitions and add more tests Test plan: 1) prove t/db_dependent/Koha/Acquisition/MarcOrder.t
Created attachment 181677 [details] [review] Bug 38547: Bring controller in line with modern standards Test plan: 1) In system preferences, click Search and then select the Acquisitions option from the left hand menu 2) Enable MarcOrderingAutomation 3) In the administration homepage, under Acquisition parameters there should now be a link called "MARC order accounts" 4) Click on this and click the New account button 5) Create a new account and then experiment with editing and deleting the account. 6) These actions should all work and there should be bootstrap messages displayed for each action
# Test Execution 1. Applied 181676. 2. Ran `prove t/db_dependent/Koha/Acquisition/MarcOrder.t`. 3. See [Test Results] below. 4. Applied both 181676 and 181677. 5. Ran `prove t/db_dependent/Koha/Acquisition/MarcOrder.t`. 6. Same output. # Test Results prove t/db_dependent/Koha/Acquisition/MarcOrder.t t/db_dependent/Koha/Acquisition/MarcOrder.t .. 1/10 # Looks like you planned 5 tests but ran 1. # Failed test 'add_biblio_from_import_record()' # at t/db_dependent/Koha/Acquisition/MarcOrder.t line 276. Undefined subroutine &Koha::MarcOrder::add_biblio_from_import_record called at t/db_dependent/Koha/Acquisition/MarcOrder.t line 247. # Looks like your test exited with 255 just after 3. t/db_dependent/Koha/Acquisition/MarcOrder.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 8/10 subtests Test Summary Report ------------------- t/db_dependent/Koha/Acquisition/MarcOrder.t (Wstat: 65280 (exited 255) Tests: 3 Failed: 1) Failed test: 3 Non-zero exit status: 255 Parse errors: Bad plan. You planned 10 tests but ran 3. Files=1, Tests=3, 1 wallclock secs ( 0.01 usr 0.00 sys + 0.87 cusr 0.07 csys = 0.95 CPU) Result: FAIL
Created attachment 182435 [details] [review] Bug 38547: Move code to Koha/Acquisitions and add more tests Test plan: 1) prove t/db_dependent/Koha/Acquisition/MarcOrder.t
Created attachment 182436 [details] [review] Bug 38547: Bring controller in line with modern standards Test plan: 1) In system preferences, click Search and then select the Acquisitions option from the left hand menu 2) Enable MarcOrderingAutomation 3) In the administration homepage, under Acquisition parameters there should now be a link called "MARC order accounts" 4) Click on this and click the New account button 5) Create a new account and then experiment with editing and deleting the account. 6) These actions should all work and there should be bootstrap messages displayed for each action
# Test Results Unit tests pass. Webpage of new feature fails to load. # Test Execution 1. Apply 182435. 2. Run `prove t/db_dependent/Koha/Acquisition/MarcOrder.t`. => success. 3. Apply 182436. 4. Run `prove t/db_dependent/Koha/Acquisition/MarcOrder.t`. => success. 5. In "Koha administration", search for "MarcOrderingAutomation" and set to "Enable." 6. Go to "Acquisitions." 7. Under "Administration," click on "MARC order accounts." => Page fails to load with Error 500 Internal Server Error. # Additional Notes - URL that crashed: /cgi-bin/koha/admin/marc_order_accounts.pl - Apache2 error log: [Wed May 14 16:00:02.166822 2025] [cgid:error] [pid 310819] [client 127.0.0.1:60780] End of script output before headers: marc_order_accounts.pl, referer: http://master-intranet/cgi-bin/koha/acqui/acqui-home.pl
Hi, you might need to run a restart_all after applying the patches - the web page is working normally for me