https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43616 --- Comment #7 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 206729 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206729&action=edit Bug 43616: Add patron_imports.pl cron job Polls every active patron_import_accounts row's configured Koha::File::Transport for new .csv files and imports each one through the existing, unmodified Koha::Patrons::Import engine, passing through every option configurable on the account: matchpoint (resolved from its stored, possibly patron_attribute_-prefixed form), overwrite cardnumber/passwords, preserved fields, per-column default values, membership expiry renewal behaviour, welcome emails, and patron list creation from each run's imported patrons - the same options tools/import_borrowers.pl exposes interactively, since Koha::Patrons::Import already implements all of them. Structurally adapted from misc/cronjobs/article_request_scans.pl, minus the zip/mapping-file layer since patron import files are plain CSV imported directly. Processed and errored files are archived under processed/ and error/ subdirectories of the transport's configured directory, and each outcome is recorded in patron_import_log for per-(account, filename) idempotency. A few edge cases are handled explicitly: - A file that produces errors but not a single imported or overwritten patron (e.g. a CSV with a bad header row) is treated as a failure - it is logged with status 'error' and archived to error/ instead of being indistinguishable from a legitimately empty file. Partial successes (some rows imported, some errored) still archive to processed/, with the error count appended to the log details. - Any remote-server-supplied filename containing a path separator is rejected before it can be used to build a local download path or an archive destination, closing a path traversal hole against a malicious or misconfigured remote server. - Errors while storing the patron_import_log row (most likely a Koha::Exceptions::Object::DuplicateID from two overlapping cron runs racing on the same new file) are caught and logged as a warning rather than killing the whole run and abandoning every remaining file and account. Genuinely overlapping runs can still both import the same new file before either records having processed it, so the schedule interval should stay comfortably longer than a single run's worst-case duration - only the crash on that race is fixed here. - Patron list creation (the account's create_patron_list option) requires a real logged-in user to attribute ownership to. This cronjob runs unattended and never has one, so when create_patron_list is set, a warning is logged explaining why and no list is created - see the cronjob's own POD for this documented limitation. - A stored matchpoint can go stale if the patron attribute type it refers to is later deleted, or ExtendedPatronAttributes is switched off - Koha::Patrons::Import would then silently fail to match any existing row, mass-creating duplicate patrons on every unattended run. Each account's resolved matchpoint is validated up front ('cardnumber', 'userid', or a still-existing, unique-id-enabled attribute type) and the account is skipped with a logged error otherwise. Test plan: 1. Apply the whole patchset, update the database, restart_all. 2. As in the "Schedule patron imports" test plan, create a Local file transport and a scheduled account using it, this time also setting a default branchcode, preserving "surname" on overwrite, enabling "Replace patron passwords", "Renew existing patrons from the current date", "Send email to new patrons", and "Create patron list". 3. Prepare a CSV with a header row and a data row missing the branchcode column (to exercise the default value), matching an existing patron's cardnumber (to exercise overwrite/preserve/renew/ password-replace), and drop it into the transport's directory. 4. Dry run: `misc/cronjobs/patron_imports.pl -v` - confirm it reports "Test run only" and lists the file as "Processing: <filename>" without moving or importing anything. 5. Confirm: `misc/cronjobs/patron_imports.pl -c -v` - confirm: - the existing patron is overwritten, their surname is unchanged (preserved), their password is replaced, and their expiry date is now today - the row missing branchcode picked up the configured default - a WELCOME notice was queued for any newly created patron - a warning is logged that no patron list was created (no real user context is available when running from cron), and no patron list exists for this run - the file is moved into processed/, and a patron_import_log row exists with status 'processed' 6. Run the same command again - confirm the file is skipped ("Already processed: <filename>"). 7. Drop a CSV with a garbled header row and run with -c -v again - confirm it archives to error/ with log status 'error'. 8. Configure an account whose matchpoint is a patron_attribute_ code, then delete that patron attribute type (or disable ExtendedPatronAttributes) - confirm the account is skipped with a logged error and no files are processed for it, rather than mass-creating new patrons. 9. Confirm -a <account_id> restricts processing to that one account when more than one is configured. 10. `prove t/db_dependent/Koha/PatronImportAccounts.t t/db_dependent/Koha/PatronImportLogs.t` all pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> -- You are receiving this mail because: You are watching all bug changes.