Bug 36067 - Inconsistencies in --commit, --confirm for command line scripts
Summary: Inconsistencies in --commit, --confirm for command line scripts
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: manvisharma1006
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2024-02-12 09:35 UTC by Martin Renvoize (ashimema)
Modified: 2026-01-30 16:41 UTC (History)
6 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: Sponsored
Comma delimited list of Sponsors: Catalyst
Crowdfunding goal: 0
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This updates several scripts, cron jobs, and files so that they use --confirm instead of --commit, for consistency with other scripts. Scripts, cron jobs, package commands, and files updated are: - koha-elasticsearch - misc/cronjobs/crontab.example - misc/cronjobs/delete_items.pl - misc/cronjobs/longoverdue.pl - misc/cronjobs/patron_emailer.pl - misc/link_bibs_to_authorities.pl - misc/search_tools/rebuild_elasticsearch.pl **Important note for existing installations**: You will need to update your crontab files if any entries include --commit as an option.
Version(s) released in:
Circulation function:


Attachments
Bug 36067: changed 'commit' to 'confirm' across files (8.02 KB, patch)
2026-01-21 02:32 UTC, manvisharma1006
Details | Diff | Splinter Review
Bug 36067: Use '--confirm' across commandline scripts (8.19 KB, patch)
2026-01-21 02:48 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 36067: Use '--confirm' across commandline scripts (8.18 KB, patch)
2026-01-22 18:46 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2024-02-12 09:35:33 UTC
Our scripts have a mix of --commit and --confirm to 'actually do the job'.  I think we should try to be consistent here.. with the most backward compatible option being to make them synonyms in each script.
Comment 1 manvisharma1006 2026-01-21 02:32:01 UTC
Created attachment 191738 [details] [review]
Bug 36067: changed 'commit' to 'confirm' across files
Comment 2 Aleisha Amohia 2026-01-21 02:48:23 UTC
Created attachment 191740 [details] [review]
Bug 36067: Use '--confirm' across commandline scripts

This patch changes commandline scripts using --commit to use --confirm instead, to be more consistent.
Comment 3 David Nind 2026-01-21 21:14:53 UTC
Thanks for the patch!

The author is currently showing as "John Doe <you@example.com>".

If you can let me know the correct details, I'll update the patch author.

I think these details are also used for what is shown on More > About Koha > Timeline for new Koha developers (and looks good on your CV!).

David

Testing notes (using koha-testing-docker):
1. I didn't test the actual scripts to see if they still work - looks like a straight-forward string change to me.
2. Before the patch: git grep "\--commit"
3. Several occurrences.
4. After the patch: only one occurrence left in misc/migration_tools/bulkmarcimport.pl (others are historical release notes)
5. To get `koha-elasticsearch -h` to show the updated help information, do a reset_all after applying the patch.

Does misc/migration_tools/bulkmarcimport.pl need updating as well?
Comment 4 manvisharma1006 2026-01-22 03:43:03 UTC
It would be great if you could put it under Manvi <manvisharma1006@gmail.com> David! And you're welcome!
Comment 5 David Nind 2026-01-22 18:46:29 UTC
Created attachment 191880 [details] [review]
Bug 36067: Use '--confirm' across commandline scripts

This patch changes commandline scripts using --commit to use --confirm instead, to be more consistent.
Comment 6 David Nind 2026-01-22 18:48:20 UTC
(In reply to manvisharma1006 from comment #4)
> It would be great if you could put it under Manvi
> <manvisharma1006@gmail.com> David! And you're welcome!

Thanks Manvi!

Now signed off.

David
Comment 7 Andrew Fuerste-Henry 2026-01-30 16:41:22 UTC
The test plan for this bug needs to include actually running all the impacted scripts with both --commmit and --confirm to ensure they both work after the patch is applied.

With this patch applied, delete_items.pl works with --commit but not with --confirm. 

kohadev-koha@kohadevbox:koha(main)$ perl misc/cronjobs/delete_items.pl --where "withdrawn=1" --confirm -v
Unknown option: confirm
Where statement: withdrawn=1
Test run only! No data will be deleted.
Would have deleted item 756 (1 items remain on record)

Looking at the code, you've added --confirm to the list of options shown to the user when using --help or --manual, but not actually implemented it in the GetOptions section (https://github.com/Koha-Community/Koha/blob/c3169205eced4a862be39120ea7db82b116e1435/misc/cronjobs/delete_items.pl#L34)