Bug 38547 - Improve the code for the MARC ordering process
Summary: Improve the code for the MARC ordering process
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Matt Blenkinsop
QA Contact: Testopia
URL:
Keywords:
Depends on: 34355
Blocks:
  Show dependency treegraph
 
Reported: 2024-11-28 15:04 UTC by Matt Blenkinsop
Modified: 2024-11-28 15:17 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 38547: Move code to Koha/Acquisitions and add more tests (23.79 KB, patch)
2024-11-28 15:16 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38547: Bring controller in line with modern standards (11.39 KB, patch)
2024-11-28 15:17 UTC, Matt Blenkinsop
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Blenkinsop 2024-11-28 15:04:55 UTC
There are some improvements needed to the code in bug 34355
Comment 1 Matt Blenkinsop 2024-11-28 15:05:51 UTC
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
Comment 2 Matt Blenkinsop 2024-11-28 15:16:58 UTC
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
Comment 3 Matt Blenkinsop 2024-11-28 15:17:00 UTC
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