Bug 27586 - Import patrons script has a confirm switch that doesn't do anything
Summary: Import patrons script has a confirm switch that doesn't do anything
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on: 12598
Blocks:
  Show dependency treegraph
 
Reported: 2021-02-01 18:15 UTC by Kyle M Hall
Modified: 2021-02-20 23:44 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
This fixes the misc/import_patrons.pl script so that patrons are not imported unless the --confirm option is used. Currently, if the script is run without "--confirm" option it reports that it is "Running in dry-run mode, provide --confirm to apply the changes", however it imports the patrons anyway.
Version(s) released in:
21.05.00,20.11.03,20.05.09,19.11.15


Attachments
Bug 27586: Import patrons script has a confirm switch that doesn't nothing! (2.26 KB, patch)
2021-02-01 18:25 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27586: Import patrons script has a confirm switch that doesn't nothing! (2.29 KB, patch)
2021-02-04 19:55 UTC, David Nind
Details | Diff | Splinter Review
Bug 27586: Rollback should be before return statement (912 bytes, patch)
2021-02-05 11:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27586: Import patrons script has a confirm switch that doesn't nothing! (2.35 KB, patch)
2021-02-07 11:01 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27586: Rollback should be before return statement (953 bytes, patch)
2021-02-07 11:01 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2021-02-01 18:15:09 UTC
If the script misc/import_patrons.pl is run without "--confirm" it reports that is "running in dry-run mode" but it is not!
Comment 1 Kyle M Hall 2021-02-01 18:25:59 UTC
Created attachment 116179 [details] [review]
Bug 27586: Import patrons script has a confirm switch that doesn't nothing!

If the script misc/import_patrons.pl is run without "--confirm" it reports
that it is "running in dry-run mode" but it is not!

Test Plan:
1) Import a CSV of patrons using import_patrons.pm *without* the --confirm parameter
2) Note that it does actually import the patrons
3) Apply this patch
4) Note that this time the patrons are not actually imported
5) Import again with --confirm
6) Note that the patrons were actually imported this time!
Comment 2 David Nind 2021-02-04 19:55:55 UTC
Created attachment 116338 [details] [review]
Bug 27586: Import patrons script has a confirm switch that doesn't nothing!

If the script misc/import_patrons.pl is run without "--confirm" it reports
that it is "running in dry-run mode" but it is not!

Test Plan:
1) Import a CSV of patrons using import_patrons.pm *without* the --confirm parameter
2) Note that it does actually import the patrons
3) Apply this patch
4) Note that this time the patrons are not actually imported
5) Import again with --confirm
6) Note that the patrons were actually imported this time!

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2021-02-04 20:00:12 UTC
Testing notes (koha-testing-docker):
- created a CSV file with sample data using the Starter CSV file from Tools > Patrons and circulation > Import patrons 
- commands to run  the import: 
  . misc/import_patrons.pl --file /kohadevbox/koha/patron-sample-import.csv --matchpoint cardnumber -v
  . misc/import_patrons.pl --file /kohadevbox/koha/patron-sample-import.csv --matchpoint cardnumber -v --confirm
Comment 4 Kyle M Hall 2021-02-05 11:15:29 UTC
Created attachment 116390 [details] [review]
Bug 27586: Rollback should be before return statement
Comment 5 Katrin Fischer 2021-02-07 11:01:04 UTC
Created attachment 116452 [details] [review]
Bug 27586: Import patrons script has a confirm switch that doesn't nothing!

If the script misc/import_patrons.pl is run without "--confirm" it reports
that it is "running in dry-run mode" but it is not!

Test Plan:
1) Import a CSV of patrons using import_patrons.pm *without* the --confirm parameter
2) Note that it does actually import the patrons
3) Apply this patch
4) Note that this time the patrons are not actually imported
5) Import again with --confirm
6) Note that the patrons were actually imported this time!

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2021-02-07 11:01:08 UTC
Created attachment 116453 [details] [review]
Bug 27586: Rollback should be before return statement

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Jonathan Druart 2021-02-08 13:57:58 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 8 Fridolin Somers 2021-02-12 12:51:35 UTC
Pushed to 20.11.x for 20.11.03
Comment 9 Victor Grousset/tuxayo 2021-02-20 23:44:38 UTC
Backported: Pushed to 19.11.x branch for 19.11.15