Summary: | Improve the code for the MARC ordering process | ||
---|---|---|---|
Product: | Koha | Reporter: | Matt Blenkinsop <matt.blenkinsop> |
Component: | Acquisitions | Assignee: | Matt Blenkinsop <matt.blenkinsop> |
Status: | Needs Signoff --- | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | jonathan.druart, nick |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 34355 | ||
Bug Blocks: | |||
Attachments: |
Bug 38547: Move code to Koha/Acquisitions and add more tests
Bug 38547: Bring controller in line with modern standards |
Description
Matt Blenkinsop
2024-11-28 15:04:55 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 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 |