This is a bug to create a process for automatically creating order lines from a MARC record. Users can nominate a directory for a particular vendor and then set a cronjob to periodically check that directory for new files. The cronjob will then stage the file, import biblios/items as required and then create order lines
Koha has already EDI functionnality, that can do what you describe. See https://koha-community.org/manual/23.05/en/html/acquisitions.html#edi-process
This enhancement would be for non-edi vendors and libraries who receive marc files.
Created attachment 154705 [details] [review] Bug 34355: DB schema and atomic update file
Created attachment 154706 [details] [review] Bug 34355: Update menus
Created attachment 154707 [details] [review] Bug 34355: Add account creation
Created attachment 154708 [details] [review] Bug 34355: Add cronjob, MarcOrder object and refactored addorderiso2709 script
Created attachment 154709 [details] [review] Bug 34355: Make item addition dependent on MarcItemFieldsToOrder mappings and introduce MarcFieldsToOrder to cronjob
Created attachment 154710 [details] [review] Bug 34355: Fix form rendering issue This patch also fixes the referencing of import_record_id_selected
I feel like we will need a nice and concise test plan here as there already was some confusion about the functionality!
There is a nice, detailed (ish) test plan that I've written but I'm trying to write unit tests before submitting everything and marking as needing signoff. I'm also weighing up Nick's point from Kohacon about splitting this into multiple bugs as there's some refactoring of the addorderiso2709.pl script at the heart of the enhancement
(In reply to Matt Blenkinsop from comment #10) > There is a nice, detailed (ish) test plan that I've written but I'm trying > to write unit tests before submitting everything and marking as needing > signoff. > > I'm also weighing up Nick's point from Kohacon about splitting this into > multiple bugs as there's some refactoring of the addorderiso2709.pl script > at the heart of the enhancement +1 :)
Created attachment 157092 [details] [review] Bug 34355: DB schema and atomic update file
Created attachment 157093 [details] [review] Bug 34355: Update menus
Created attachment 157094 [details] [review] Bug 34355: Add account creation
Created attachment 157095 [details] [review] Bug 34355: Add cronjob, MarcOrder object and refactored addorderiso2709 script
Created attachment 157096 [details] [review] Bug 34355: Make item addition dependent on MarcItemFieldsToOrder mappings and introduce MarcFieldsToOrder to cronjob
Created attachment 157097 [details] [review] Bug 34355: Fix form rendering issue This patch also fixes the referencing of import_record_id_selected
Created attachment 157098 [details] [review] Bug 34355: Add match_field and match_value to allow multiple accounts in the same file directory
Created attachment 157099 [details] [review] Bug 34355: Add a plugin hook - before_orderline_create
Created attachment 157100 [details] [review] Bug 34355: Rebase changes made in bug 33170
We need a test plan please.
(In reply to Katrin Fischer from comment #21) > We need a test plan please. ... and in general please more commit messages ;)
Created attachment 160858 [details] [review] Bug 34355: DB schema and atomic update file
Created attachment 160859 [details] [review] Bug 34355: Update menus
Created attachment 160860 [details] [review] Bug 34355: Add account creation
Created attachment 160861 [details] [review] Bug 34355: Add cronjob, MarcOrder object and refactored addorderiso2709 script
Created attachment 160862 [details] [review] Bug 34355: Make item addition dependent on MarcItemFieldsToOrder mappings and introduce MarcFieldsToOrder to cronjob
Created attachment 160863 [details] [review] Bug 34355: Fix form rendering issue This patch also fixes the referencing of import_record_id_selected
Created attachment 160864 [details] [review] Bug 34355: Add match_field and match_value to allow multiple accounts in the same file directory
Created attachment 160865 [details] [review] Bug 34355: Add a plugin hook - before_orderline_create
Created attachment 160866 [details] [review] Bug 34355: Rebase changes made in bug 33170
Created attachment 160867 [details] [review] Bug 34355: DB schema and atomic update file
Created attachment 160868 [details] [review] Bug 34355: Update menus
Created attachment 160869 [details] [review] Bug 34355: Add account creation
Created attachment 160870 [details] [review] Bug 34355: Add cronjob, MarcOrder object and refactored addorderiso2709 script
Created attachment 160871 [details] [review] Bug 34355: Make item addition dependent on MarcItemFieldsToOrder mappings and introduce MarcFieldsToOrder to cronjob
Created attachment 160872 [details] [review] Bug 34355: Fix form rendering issue This patch also fixes the referencing of import_record_id_selected
Created attachment 160873 [details] [review] Bug 34355: Add match_field and match_value to allow multiple accounts in the same file directory
Created attachment 160874 [details] [review] Bug 34355: Add a plugin hook - before_orderline_create
Created attachment 160875 [details] [review] Bug 34355: Rebase changes made in bug 33170
Test Plan 1) Create an sFTP server (1 set up for each unique vendor) and share the credentials with the vendor who will be sending the test data 2)apply patch 3) navigate to Administration -> Acquisition parameters -> MARC order accounts 4) click New Account 5)Enter the following details a) Vendor: b) Budget:This budget will be used as the fallback value if the MARC records do not contain a mapped value for a budget code. c)Description: d)Download directory:The download directory specifies the directory in your koha installation that should be searched for new files. e)Match field:(Optional): If you have files from multiple vendors in the same file directory, the match field is the field in the marc record that will be checked to see if the file should be processed by this account. f)Match value: (Optional): This is the value that will be checked against the match field to see if the file matches this account. If it does it will be processed by this account, if not it will be skipped. 6) enter File import settings 7) enter Record matching settings 8) select if it should check for embedded item record data 9) click submit 10) drop test file in sFTP server (run manually or have a cron run to pick up the file) 11) file should load in basket for said vendor and create orders.
Is the setup of the mentioned SFTP server something done in Koha or something done outside? Are you expected to set it up on the Koha server itself? Maybe this works similar like picking up files for EDI?
Created attachment 169896 [details] [review] Bug 34355: DB schemas and atomicupdate
Created attachment 169897 [details] [review] Bug 34355: Update menus
Created attachment 169898 [details] [review] Bug 34355: Add account creation
Created attachment 169899 [details] [review] Bug 34355: Add cronjob and required methods This patch adds a cronjob to take a marc file from the Koha file directory system and import it before creating order lines Test plan: This test plan involves running a cronjob, not sure how easy this is in the sandboxes 1) In system preferences, click Search and then select the Acquisitions option from the left hand menu 2) Enable MarcOrderingAutomation 3) Paste the following into MarcFieldsToOrder price: 975$p quantity: 975$q budget_code: 975$h 4) Paste the following into MarcItemFieldsToOrder homebranch: 949$a holdingbranch: 949$b itype: 949$y nonpublic_note: 949$x public_note: 949$z loc: 949$c ccode: 949$8 notforloan: 949$7 uri: 949$u copyno: 949$t price: 949$g replacementprice: 949$v itemcallnumber: 949$o quantity: 949$k budget_code: 949$l Now save the sysprefs 5) In the administration homepage, under Acquisition parameters there should now be a link called "MARC order accounts" 6) Click on this and click the New account button 7) Choose a vendor and a budget 8) Enter a description 9) Enter the following in download directory - "/var/lib/koha/kohadev/tmp/koha_kohadev_upload" 10) Select any record matching settings you want (not important for the test plan, this uses the same code as the marc file staging that already exists in Koha) 11) Check Yes under Check for embedded item record data 12) Click submit 13) We now need to upload a MARC file into the directory that we put in the account. The easiest way to do this is through the marc staging tool 14) Navigate to Cataloging > Stage MARC records for import 15) Upload the file attached to this bug using the Choose file button 16) Click Upload file. 17) Leave the page without completing the new form that appears, we don't want to stage this file. The file we uploaded is now in the directory ready for the cronjob to process as if it had been transferred via SFTP. 18) In the CLI run, perl misc/cronjobs/marc_ordering_process.pl -v -c 19) The logging should show that the file has been processed for that account 20) Navigate to Acquisitions 21) Search for the Vendor that you put in the MARC ordering account 22) There should be a basket created on the vendor with the name of the file you uploaded (there will also be a reference at the start e.g. ea2323966faef2c59a6056dbd1f7d658_filename.marcxml. This is because we used the marc staging tool to upload the file and that adds a reference to filenames. This will not appear when using files from an SFTP location). 23) Click in the basket 24) There should be three orderlines, complete with accounting details that have been mapped from the file. One line should have a fund of Fund 1_2, quantity of 4 and a replacement price of 6.75, one should have Secondary fund, 2, 6.40 and one should have Main fund,2, 6.30
Created attachment 169900 [details] [review] Bug 34355: Make item addition dependent on MarcItemFieldsToOrder mappings and introduce MarcFieldsToOrder to cronjob
Created attachment 169901 [details] [review] Bug 34355: Fix form rendering issue This patch also fixes the referencing of import_record_id_selected
Created attachment 169902 [details] [review] Bug 34355: Add match_field and match_value to allow multiple accounts in the same file directory
Created attachment 169903 [details] [review] Bug 34355: Add a plugin hook - before_orderline_create
Created attachment 169904 [details] [review] Bug 34355: Add unit tests
Background: This bug is intended for non-EDI vendors who wish to receive MARC files and automatically create order lines from these using the mappings in MarcFieldsToOrder and MarcItemFieldsToOrder. A vendor can supply MARC files to an SFTP location in the Koha installation and a cronjob can pick up those files and process them into order lines in a new basket for each file. A file has been attached to test with, but you can use your own files. You just need to ensure that there is a mapping in the file consistent with what you put into the system preferences as per the test plan below. Test plan: This test plan involves running a cronjob, not sure how easy this is in the sandboxes 1) In system preferences, click Search and then select the Acquisitions option from the left hand menu 2) Enable MarcOrderingAutomation 3) Paste the following into MarcFieldsToOrder price: 975$p quantity: 975$q budget_code: 975$h 4) Paste the following into MarcItemFieldsToOrder homebranch: 949$a holdingbranch: 949$b itype: 949$y nonpublic_note: 949$x public_note: 949$z loc: 949$c ccode: 949$8 notforloan: 949$7 uri: 949$u copyno: 949$t price: 949$g replacementprice: 949$v itemcallnumber: 949$o quantity: 949$k budget_code: 949$l Now save the sysprefs 5) In the administration homepage, under Acquisition parameters there should now be a link called "MARC order accounts" 6) Click on this and click the New account button 7) Choose a vendor and a budget 8) Enter a description 9) Enter the following in download directory - "/var/lib/koha/kohadev/tmp/koha_kohadev_upload" 10) Select any record matching settings you want (not important for the test plan, this uses the same code as the marc file staging that already exists in Koha) 11) Check Yes under Check for embedded item record data 12) Click submit 13) We now need to upload a MARC file into the directory that we put in the account. The easiest way to do this is through the marc staging tool 14) Navigate to Cataloging > Stage MARC records for import 15) Upload the file attached to this bug using the Choose file button 16) Click Upload file. 17) Leave the page without completing the new form that appears, we don't want to stage this file. The file we uploaded is now in the directory ready for the cronjob to process as if it had been transferred via SFTP. 18) In the CLI run, perl misc/cronjobs/marc_ordering_process.pl -v -c 19) The logging should show that the file has been processed for that account 20) Navigate to Acquisitions 21) Search for the Vendor that you put in the MARC ordering account 22) There should be a basket created on the vendor with the name of the file you uploaded (there will also be a reference at the start e.g. ea2323966faef2c59a6056dbd1f7d658_filename.marcxml. This is because we used the marc staging tool to upload the file and that adds a reference to filenames. This will not appear when using files from an SFTP location). 23) Click in the basket 24) There should be three orderlines, complete with accounting details that have been mapped from the file. One line should have a fund of Fund 1_2, quantity of 4 and a replacement price of 6.75, one should have Secondary fund, 2, 6.40 and one should have Main fund,2, 6.30
Created attachment 170395 [details] Test file
Created attachment 170401 [details] [review] Bug 34355: DB schemas and atomicupdate Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 170402 [details] [review] Bug 34355: Update menus Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 170403 [details] [review] Bug 34355: Add account creation Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 170404 [details] [review] Bug 34355: Add cronjob and required methods This patch adds a cronjob to take a marc file from the Koha file directory system and import it before creating order lines Test plan: This test plan involves running a cronjob, not sure how easy this is in the sandboxes 1) In system preferences, click Search and then select the Acquisitions option from the left hand menu 2) Enable MarcOrderingAutomation 3) Paste the following into MarcFieldsToOrder price: 975$p quantity: 975$q budget_code: 975$h 4) Paste the following into MarcItemFieldsToOrder homebranch: 949$a holdingbranch: 949$b itype: 949$y nonpublic_note: 949$x public_note: 949$z loc: 949$c ccode: 949$8 notforloan: 949$7 uri: 949$u copyno: 949$t price: 949$g replacementprice: 949$v itemcallnumber: 949$o quantity: 949$k budget_code: 949$l Now save the sysprefs 5) In the administration homepage, under Acquisition parameters there should now be a link called "MARC order accounts" 6) Click on this and click the New account button 7) Choose a vendor and a budget 8) Enter a description 9) Enter the following in download directory - "/var/lib/koha/kohadev/tmp/koha_kohadev_upload" 10) Select any record matching settings you want (not important for the test plan, this uses the same code as the marc file staging that already exists in Koha) 11) Check Yes under Check for embedded item record data 12) Click submit 13) We now need to upload a MARC file into the directory that we put in the account. The easiest way to do this is through the marc staging tool 14) Navigate to Cataloging > Stage MARC records for import 15) Upload the file attached to this bug using the Choose file button 16) Click Upload file. 17) Leave the page without completing the new form that appears, we don't want to stage this file. The file we uploaded is now in the directory ready for the cronjob to process as if it had been transferred via SFTP. 18) In the CLI run, perl misc/cronjobs/marc_ordering_process.pl -v -c 19) The logging should show that the file has been processed for that account 20) Navigate to Acquisitions 21) Search for the Vendor that you put in the MARC ordering account 22) There should be a basket created on the vendor with the name of the file you uploaded (there will also be a reference at the start e.g. ea2323966faef2c59a6056dbd1f7d658_filename.marcxml. This is because we used the marc staging tool to upload the file and that adds a reference to filenames. This will not appear when using files from an SFTP location). 23) Click in the basket 24) There should be three orderlines, complete with accounting details that have been mapped from the file. One line should have a fund of Fund 1_2, quantity of 4 and a replacement price of 6.75, one should have Secondary fund, 2, 6.40 and one should have Main fund,2, 6.30 Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 170405 [details] [review] Bug 34355: Make item addition dependent on MarcItemFieldsToOrder mappings and introduce MarcFieldsToOrder to cronjob Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 170406 [details] [review] Bug 34355: Fix form rendering issue This patch also fixes the referencing of import_record_id_selected Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 170407 [details] [review] Bug 34355: Add match_field and match_value to allow multiple accounts in the same file directory Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 170408 [details] [review] Bug 34355: Add a plugin hook - before_orderline_create Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 170409 [details] [review] Bug 34355: Add unit tests Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Doubled pref in patches: pref: SerialsDefaultReplyTo looks like rebase issues A user can CRUD Marc Order Accounts with only catalogue permissions - I think it needs to be a new subpermission of acquisitions A full search of vendors is used on the marc order accounts page - this should be a select2 and not load all into template (we have partners with thousands of vendors) create_items is not set for basket - I think 'ordering' is required? maybe this setting is just ignored for this process? you mention _stage_file is a clone from the background job. it should be moved into a place that can be shared with the background job to dry the code match_file_to_account - POD should indicate only the first record is checked on the account page it should specify the format, dollar sign for subfields, i.e. 245$a Commit titles below don't match the patches, can you explain? Bug 34355: Make item addition dependent on MarcItemFieldsToOrder mappings and introduce MarcFieldsToOrder to cronjob Bug 34355: Add a plugin hook - before_orderline_create koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_order_accounts.tt The selected option choices are split by the conditionals, include the full option selected or not You remove the use of tools-overlay-action.inc / tools-nomatch-action / tools-item-action - I think we should keep this to not repeat - the selection variables lok unsused so can be updated to work with the data here (e.g. git grep overlay_action_replace ) Warning when processing test file: Use of uninitialized value in concatenation (.) or string at /usr/share/perl5/MARC/File/XML.pm line 399, <__ANONIO__> chunk 5. Maybe a data problem with the file? The cronjob deletes the file - I think we want an option to rename/move the file, rather than full deletion - if something is wrong, we want the file for reference It looks like a few fields were not included in _create_item_fields_from_syspref: coded_location_qualifier, barcode, and enumchron - special field quantity
Created attachment 173054 [details] [review] Bug 34355: DB schemas and atomicupdate Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 173055 [details] [review] Bug 34355: Update menus Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 173056 [details] [review] Bug 34355: Add account creation Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 173057 [details] [review] Bug 34355: Add cronjob and required methods This patch adds a cronjob to take a marc file from the Koha file directory system and import it before creating order lines Test plan: This test plan involves running a cronjob, not sure how easy this is in the sandboxes 1) In system preferences, click Search and then select the Acquisitions option from the left hand menu 2) Enable MarcOrderingAutomation 3) Paste the following into MarcFieldsToOrder price: 975$p quantity: 975$q budget_code: 975$h 4) Paste the following into MarcItemFieldsToOrder homebranch: 949$a holdingbranch: 949$b itype: 949$y nonpublic_note: 949$x public_note: 949$z loc: 949$c ccode: 949$8 notforloan: 949$7 uri: 949$u copyno: 949$t price: 949$g replacementprice: 949$v itemcallnumber: 949$o quantity: 949$k budget_code: 949$l Now save the sysprefs 5) In the administration homepage, under Acquisition parameters there should now be a link called "MARC order accounts" 6) Click on this and click the New account button 7) Choose a vendor and a budget 8) Enter a description 9) Enter the following in download directory - "/var/lib/koha/kohadev/tmp/koha_kohadev_upload" 10) Select any record matching settings you want (not important for the test plan, this uses the same code as the marc file staging that already exists in Koha) 11) Check Yes under Check for embedded item record data 12) Click submit 13) We now need to upload a MARC file into the directory that we put in the account. The easiest way to do this is through the marc staging tool 14) Navigate to Cataloging > Stage MARC records for import 15) Upload the file attached to this bug using the Choose file button 16) Click Upload file. 17) Leave the page without completing the new form that appears, we don't want to stage this file. The file we uploaded is now in the directory ready for the cronjob to process as if it had been transferred via SFTP. 18) In the CLI run, perl misc/cronjobs/marc_ordering_process.pl -v -c 19) The logging should show that the file has been processed for that account 20) Navigate to Acquisitions 21) Search for the Vendor that you put in the MARC ordering account 22) There should be a basket created on the vendor with the name of the file you uploaded (there will also be a reference at the start e.g. ea2323966faef2c59a6056dbd1f7d658_filename.marcxml. This is because we used the marc staging tool to upload the file and that adds a reference to filenames. This will not appear when using files from an SFTP location). 23) Click in the basket 24) There should be three orderlines, complete with accounting details that have been mapped from the file. One line should have a fund of Fund 1_2, quantity of 4 and a replacement price of 6.75, one should have Secondary fund, 2, 6.40 and one should have Main fund,2, 6.30 Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 173058 [details] [review] Bug 34355: Fix form rendering issue This patch also fixes the referencing of import_record_id_selected Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 173059 [details] [review] Bug 34355: Add match_field and match_value to allow multiple accounts in the same file directory Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 173060 [details] [review] Bug 34355: Add a plugin hook - before_orderline_create Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 173061 [details] [review] Bug 34355: Add unit tests Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 173062 [details] [review] Bug 34355: (QA follow-up) Add permission marc_order_manage to govern control of account creation
Created attachment 173063 [details] [review] Bug 34355: (QA follow-up) Migrate vendor dropdown to a kohaSelect
Created attachment 173064 [details] [review] Bug 34355: (QA follow-up) Add a delete flag to the cronjob This patch adds a 'delete' flag to the cronjob to select whether a file should be deleted or archived once processed
Created attachment 173065 [details] [review] Bug 34355: (QA follow-up) Fix template errors This patch addresses the following issues: - Fixes a duplication of the SerialsDefaultReplyTo syspref - Re-introduces include files for overlay rules
Created attachment 173066 [details] [review] Bug 34355: (QA follow-up) Make record staging code DRY
(In reply to Nick Clemens (kidclamp) from comment #63) > Doubled pref in patches: > pref: SerialsDefaultReplyTo > looks like rebase issues Corrected > A user can CRUD Marc Order Accounts with only catalogue permissions - I > think it needs to be a new subpermission of acquisitions Added a new permission marc_order_manage > A full search of vendors is used on the marc order accounts page - this > should be a select2 and not load all into template (we have partners with > thousands of vendors) Migrated to kohaSelect > create_items is not set for basket - I think 'ordering' is required? maybe > this setting is just ignored for this process? I think we should set it - I've amended it to be set > you mention _stage_file is a clone from the background job. it should be > moved into a place that can be shared with the background job to dry the code Done - moved to Koha::ImportBatch > match_file_to_account - POD should indicate only the first record is checked > on the account page it should specify the format, dollar sign for subfields, > i.e. 245$a Done > Commit titles below don't match the patches, can you explain? > Bug 34355: Make item addition dependent on MarcItemFieldsToOrder mappings > and introduce MarcFieldsToOrder to cronjob > Bug 34355: Add a plugin hook - before_orderline_create I think this is the result of rebases - the plugin hook had been lost and is now restored. The other commit I have squashed as it is no longer necessary as a separate commit as the history has been broken by other squashes/rebases > koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_order_accounts.tt > The selected option choices are split by the conditionals, include the full > option selected or not > You remove the use of tools-overlay-action.inc / tools-nomatch-action / > tools-item-action - I think we should keep this to not repeat - the > selection variables lok unsused so can be updated to work with > the data here (e.g. git grep overlay_action_replace ) Amended the templates to reflect this > Warning when processing test file: > Use of uninitialized value in concatenation (.) or string at > /usr/share/perl5/MARC/File/XML.pm line 399, <__ANONIO__> chunk 5. > Maybe a data problem with the file? This comes from the MARC library we import from CPAN and happens with the file when running a normal staged import > The cronjob deletes the file - I think we want an option to rename/move the > file, rather than full deletion - if something is wrong, we want the file > for reference I've added a delete flag - if this isn't set the cronjob now moves processed files to an 'archived' folder > It looks like a few fields were not included in > _create_item_fields_from_syspref: > coded_location_qualifier, barcode, and enumchron - special field quantity Updated
Created attachment 173709 [details] [review] Bug 34355: (QA follow-up) Permissions fixes This patch addresses some permissions errors and updates the breadcrumbs to reflect this
Created attachment 173805 [details] [review] Bug 34355: (QA follow-up) Restore MarcFieldsToOrder prices
Created attachment 173806 [details] [review] Bug 34355: (QA follow-up) Restore MarcFieldsToOrder prices
Created attachment 173809 [details] [review] Bug 34355: (QA follow-up) Restore MarcFieldsToOrder prices
Created attachment 173824 [details] [review] Bug 34355: (QA follow-up) Restore MarcFieldsToOrder prices and budgets
Created attachment 173825 [details] [review] Bug 34355: (QA follow-up) Respect AcqCreateItem syspref
Created attachment 173993 [details] [review] Bug 34355: DB schemas and atomicupdate Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 173994 [details] [review] Bug 34355: Update menus Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 173995 [details] [review] Bug 34355: Add account creation Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 173996 [details] [review] Bug 34355: Add cronjob and required methods This patch adds a cronjob to take a marc file from the Koha file directory system and import it before creating order lines Test plan: This test plan involves running a cronjob, not sure how easy this is in the sandboxes 1) In system preferences, click Search and then select the Acquisitions option from the left hand menu 2) Enable MarcOrderingAutomation 3) Paste the following into MarcFieldsToOrder price: 975$p quantity: 975$q budget_code: 975$h 4) Paste the following into MarcItemFieldsToOrder homebranch: 949$a holdingbranch: 949$b itype: 949$y nonpublic_note: 949$x public_note: 949$z loc: 949$c ccode: 949$8 notforloan: 949$7 uri: 949$u copyno: 949$t price: 949$g replacementprice: 949$v itemcallnumber: 949$o quantity: 949$k budget_code: 949$l Now save the sysprefs 5) In the administration homepage, under Acquisition parameters there should now be a link called "MARC order accounts" 6) Click on this and click the New account button 7) Choose a vendor and a budget 8) Enter a description 9) Enter the following in download directory - "/var/lib/koha/kohadev/tmp/koha_kohadev_upload" 10) Select any record matching settings you want (not important for the test plan, this uses the same code as the marc file staging that already exists in Koha) 11) Check Yes under Check for embedded item record data 12) Click submit 13) We now need to upload a MARC file into the directory that we put in the account. The easiest way to do this is through the marc staging tool 14) Navigate to Cataloging > Stage MARC records for import 15) Upload the file attached to this bug using the Choose file button 16) Click Upload file. 17) Leave the page without completing the new form that appears, we don't want to stage this file. The file we uploaded is now in the directory ready for the cronjob to process as if it had been transferred via SFTP. 18) In the CLI run, perl misc/cronjobs/marc_ordering_process.pl -v -c 19) The logging should show that the file has been processed for that account 20) Navigate to Acquisitions 21) Search for the Vendor that you put in the MARC ordering account 22) There should be a basket created on the vendor with the name of the file you uploaded (there will also be a reference at the start e.g. ea2323966faef2c59a6056dbd1f7d658_filename.marcxml. This is because we used the marc staging tool to upload the file and that adds a reference to filenames. This will not appear when using files from an SFTP location). 23) Click in the basket 24) There should be three orderlines, complete with accounting details that have been mapped from the file. One line should have a fund of Fund 1_2, quantity of 4 and a replacement price of 6.75, one should have Secondary fund, 2, 6.40 and one should have Main fund,2, 6.30 Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 173997 [details] [review] Bug 34355: Fix form rendering issue This patch also fixes the referencing of import_record_id_selected Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 173998 [details] [review] Bug 34355: Add match_field and match_value to allow multiple accounts in the same file directory Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 173999 [details] [review] Bug 34355: Add a plugin hook - before_orderline_create Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174000 [details] [review] Bug 34355: Add unit tests Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174001 [details] [review] Bug 34355: (QA follow-up) Add permission marc_order_manage to govern control of account creation Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174002 [details] [review] Bug 34355: (QA follow-up) Migrate vendor dropdown to a kohaSelect Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174003 [details] [review] Bug 34355: (QA follow-up) Add a delete flag to the cronjob This patch adds a 'delete' flag to the cronjob to select whether a file should be deleted or archived once processed Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174004 [details] [review] Bug 34355: (QA follow-up) Fix template errors This patch addresses the following issues: - Fixes a duplication of the SerialsDefaultReplyTo syspref - Re-introduces include files for overlay rules Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174005 [details] [review] Bug 34355: (QA follow-up) Make record staging code DRY Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174006 [details] [review] Bug 34355: (QA follow-up) Permissions fixes This patch addresses some permissions errors and updates the breadcrumbs to reflect this Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174007 [details] [review] Bug 34355: (QA follow-up) Restore MarcFieldsToOrder prices and budgets Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174008 [details] [review] Bug 34355: (QA follow-up) Respect AcqCreateItem syspref Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174009 [details] [review] Bug 34355: (QA follow-up) Remove debugging comment from archiving command Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174010 [details] [review] Bug 34355: (QA follow-up) Tidy Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
I think we can do more in the future to improve readability, but this one moves more ordering code into modules to make it easier for testing and gets my vote. I tested all combinations I could think of, and orders are generated correctly from Marc. PQA
Please don't combine database update and schema update. We have a conflict now in the first patch: Bug 34355: DB schemas and atomicupdate
I am trying to remove the schema files from the patches, so they apply...
Created attachment 174362 [details] [review] Bug 34355: DB schemas and atomicupdate Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174363 [details] [review] Bug 34355: DBIC files Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174364 [details] [review] Bug 34355: Update menus Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174365 [details] [review] Bug 34355: Add account creation Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174366 [details] [review] Bug 34355: Add cronjob and required methods This patch adds a cronjob to take a marc file from the Koha file directory system and import it before creating order lines Test plan: This test plan involves running a cronjob, not sure how easy this is in the sandboxes 1) In system preferences, click Search and then select the Acquisitions option from the left hand menu 2) Enable MarcOrderingAutomation 3) Paste the following into MarcFieldsToOrder price: 975$p quantity: 975$q budget_code: 975$h 4) Paste the following into MarcItemFieldsToOrder homebranch: 949$a holdingbranch: 949$b itype: 949$y nonpublic_note: 949$x public_note: 949$z loc: 949$c ccode: 949$8 notforloan: 949$7 uri: 949$u copyno: 949$t price: 949$g replacementprice: 949$v itemcallnumber: 949$o quantity: 949$k budget_code: 949$l Now save the sysprefs 5) In the administration homepage, under Acquisition parameters there should now be a link called "MARC order accounts" 6) Click on this and click the New account button 7) Choose a vendor and a budget 8) Enter a description 9) Enter the following in download directory - "/var/lib/koha/kohadev/tmp/koha_kohadev_upload" 10) Select any record matching settings you want (not important for the test plan, this uses the same code as the marc file staging that already exists in Koha) 11) Check Yes under Check for embedded item record data 12) Click submit 13) We now need to upload a MARC file into the directory that we put in the account. The easiest way to do this is through the marc staging tool 14) Navigate to Cataloging > Stage MARC records for import 15) Upload the file attached to this bug using the Choose file button 16) Click Upload file. 17) Leave the page without completing the new form that appears, we don't want to stage this file. The file we uploaded is now in the directory ready for the cronjob to process as if it had been transferred via SFTP. 18) In the CLI run, perl misc/cronjobs/marc_ordering_process.pl -v -c 19) The logging should show that the file has been processed for that account 20) Navigate to Acquisitions 21) Search for the Vendor that you put in the MARC ordering account 22) There should be a basket created on the vendor with the name of the file you uploaded (there will also be a reference at the start e.g. ea2323966faef2c59a6056dbd1f7d658_filename.marcxml. This is because we used the marc staging tool to upload the file and that adds a reference to filenames. This will not appear when using files from an SFTP location). 23) Click in the basket 24) There should be three orderlines, complete with accounting details that have been mapped from the file. One line should have a fund of Fund 1_2, quantity of 4 and a replacement price of 6.75, one should have Secondary fund, 2, 6.40 and one should have Main fund,2, 6.30 Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174367 [details] [review] Bug 34355: Fix form rendering issue This patch also fixes the referencing of import_record_id_selected Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174368 [details] [review] Bug 34355: Add match_field and match_value to allow multiple accounts in the same file directory Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174369 [details] [review] Bug 34355: Add a plugin hook - before_orderline_create Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174370 [details] [review] Bug 34355: Add unit tests Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174371 [details] [review] Bug 34355: (QA follow-up) Add permission marc_order_manage to govern control of account creation Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174372 [details] [review] Bug 34355: (QA follow-up) Migrate vendor dropdown to a kohaSelect Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174373 [details] [review] Bug 34355: (QA follow-up) Add a delete flag to the cronjob This patch adds a 'delete' flag to the cronjob to select whether a file should be deleted or archived once processed Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174374 [details] [review] Bug 34355: (QA follow-up) Fix template errors This patch addresses the following issues: - Fixes a duplication of the SerialsDefaultReplyTo syspref - Re-introduces include files for overlay rules Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174375 [details] [review] Bug 34355: (QA follow-up) Make record staging code DRY Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174376 [details] [review] Bug 34355: (QA follow-up) Permissions fixes This patch addresses some permissions errors and updates the breadcrumbs to reflect this Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174377 [details] [review] Bug 34355: (QA follow-up) Restore MarcFieldsToOrder prices and budgets Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174378 [details] [review] Bug 34355: (QA follow-up) Respect AcqCreateItem syspref Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174379 [details] [review] Bug 34355: (QA follow-up) Remove debugging comment from archiving command Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 174380 [details] [review] Bug 34355: (QA follow-up) Tidy Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Rebased, although the conflict must be coming from something you've applied locally and not yet pushed as it applies cleanly for me - if there is still conflict then let me know when you've pushed all your local patches
Created attachment 174382 [details] [review] Bug 34355: DBIC schema update
Created attachment 174383 [details] [review] Bug 34355: (QA follow-up) Terminology and capitalization
Pushed for 24.11! Well done everyone, thank you!
Might need some follow-ups for style (Bootstrap 5).
Should we treat this as "new feature"?
There are so many things to say here, but I don't have time right now for a full QA review... Just let say the main points that shall have been raised by QA: 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 Everybody wants their stuffs pushed, but we really need more eyes and spend more time on QA IMO. I have been fighting for years to clean the code, but what's the point if we don't enforce the quality and prevent new legacy code to be pushed?