[Bug 36027] New: search_for_data_inconsistencies.pl - make each section optional
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Bug ID: 36027 Summary: search_for_data_inconsistencies.pl - make each section optional Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Reports Assignee: koha-bugs@lists.koha-community.org Reporter: phan-tung.bui@inlibro.com QA Contact: testopia@bugs.koha-community.org Modification of the script allows us to choose which test to run instead of running all tests. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |phan-tung.bui@inlibro.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #1 from Phan Tung Bui <phan-tung.bui@inlibro.com> --- Created attachment 161852 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161852&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional Test plan : 1. Apply batch 2. Run the script 3. Enter the corresponding number to choose a method to run 4. Complete the Test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com, | |martin.renvoize@ptfs-europe | |.com, | |pedro.amorim@ptfs-europe.co | |m Status|Needs Signoff |Failed QA --- Comment #2 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Failing QA here: 1) Not tidy 2) Leftover Data::Dumper import 3) Many empty new lines 4) This enhancement turns the script into an interactive script, requiring interactive input. This change in behavior will potentially cause existing systems running this script (as a cron or otherwise) to fail. My suggestion here is to update the code to instead allow for params instead of turning it interactive, e.g.: perl misc/maintenance/search_for_data_inconsistencies.pl --items-branch --items-status And keep functionality of running all checks by default as is currently, this ensures existing default functionality plus the new enhancement, instead of changing current default functionality. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161852|0 |1 is obsolete| | --- Comment #3 from Phan Tung Bui <phan-tung.bui@inlibro.com> --- Created attachment 161953 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161953&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional 1. Code has been tidied up for better readability and maintainability. 2. The Data::Dumper import has been removed as it was unnecessary. 3. Unnecessary empty lines have been removed to improve code cleanliness. 4. The script has been updated to accept parameters instead of relying on interactive input. Now you can specify which checks to run using command-line parameters, while retaining the default behavior of running all checks if no parameters are provided. Thank you for your feedback! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #4 from Phan Tung Bui <phan-tung.bui@inlibro.com> --- Created attachment 161954 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161954&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #5 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Hi Phan, Patch isn't applying for me on current master, can you double-check please? Maybe current patches were written on top of the now obsolete patch? Thanks. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #6 from Phan Tung Bui <phan-tung.bui@inlibro.com> --- Created attachment 162162 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162162&action=edit Bug 36027 : (follow up) Fix patch isn't applying error -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply --- Comment #7 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Hi Phan, The first patch of the 3 is the first patch that does not apply, it needs to either be rebased and resubmitted or made obsolete. If you don't want to rebase and want to submit a new one, you can make it obsolete by visiting https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161953&action=edit and clicking "edit details", tick "obsolete". Me and others are available to chat on IRC and provide help if needed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161953|0 |1 is obsolete| | Attachment #161953|1 |0 is patch| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161953|0 |1 is patch| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #8 from Phan Tung Bui <phan-tung.bui@inlibro.com> --- Created attachment 162349 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162349&action=edit Bug 36027: Tidy code for QA script -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162162|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161954|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162349|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #9 from Phan Tung Bui <phan-tung.bui@inlibro.com> --- Created attachment 162351 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162351&action=edit Bug 36027: change interactive to pass param and tidy the code -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff --- Comment #10 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Putting this as "Needs Signoff" to look at again later and there's no sign-off line by anyone just yet. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |julian.maurice@biblibre.com --- Comment #11 from Julian Maurice <julian.maurice@biblibre.com> --- Passing arbitrary numbers as command line arguments is not a good UI as it's opaque (no way to guess that "3" means "check item status") and it's very hard to remember what each number does. Also it can make future changes more complicated: for instance if section 3 should be removed, what should we do with this number ? Leave it unassigned and have "gaps" in the sequence ? Re-assign it to another check and break backward compatibility ? IMO it's far better to have option flags. And as the default behavior is to run all checks, those option flags should disable a specific section when given. For instance: --skip-items-branch --skip-age-for-category. Getopt::Long can be used for that Also documentation in POD should be updated and a --help flag should be added to print usage help (see Pod::Usage) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #12 from Phan Tung Bui <phan-tung.bui@inlibro.com> --- Created attachment 162540 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162540&action=edit Bug 36027 : (follow-up) Improvement: Simplified command line options and add --help option This commit addresses the issue of opaque command line arguments by implementing option flags for better clarity and user-friendliness. Instead of passing arbitrary numbers, users can now utilize descriptive flags such as --skip-items-branch and --skip-age-for-category to disable specific sections of checks. Additionally, the documentation in POD format has been updated to reflect these changes, including the addition of a --help flag to provide usage instructions. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #13 from Phan Tung Bui <phan-tung.bui@inlibro.com> --- Updated test plan 1. Apply batch 2. Run the script and using options suggested in --help 3. Complete the test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #14 from Phan Tung Bui <phan-tung.bui@inlibro.com> --- Created attachment 162546 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162546&action=edit Bug 36027 : Tidy the code -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36039 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #15 from Phan Tung Bui <phan-tung.bui@inlibro.com> --- Created attachment 163259 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163259&action=edit Bug 36027 : (follow-up) Improvement : Add check option i add the check option so that instead of skipping certain checks, you can also choose a specific test to run -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr --- Comment #16 from Frédéric Demians <frederic@tamil.fr> --- Would it be possible to have just one patch? In my opinion, there are already too many parameters. May I suggest an implementation based on an array of inconsistencies tests? Something like that: my $inconsistencies = [ { name => 'item-branch', description => 'Items without home or holding library', sub => { }, }, { name => 'item-type', description => 'Biblios/items without item type or with invalid item type', sub => { }, }, ]; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Phan Tung Bui <phan-tung.bui@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162351|0 |1 is obsolete| | Attachment #162540|0 |1 is obsolete| | Attachment #162546|0 |1 is obsolete| | Attachment #163259|0 |1 is obsolete| | --- Comment #17 from Phan Tung Bui <phan-tung.bui@inlibro.com> --- Created attachment 163956 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163956&action=edit Bug 36027: change interactive to pass param and add check options -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 166117 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166117&action=edit Bug 36027: Change interactive to pass param and tidy the code Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163956|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This is a big improvement, signing off. For QA, I'd encourage a better commit title, a summary of what it does and a test plan to be included in the commit message. Generally, though, I like where this is headed and wouldn't block it. I'd love to see the functions moved into the Database::Audit module Pedro introduces in bug 36039 eventually and exposure of this data in there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |36039 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 [Bug 36039] Output of database_audit.pl should be accessible through UI -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #20 from Julian Maurice <julian.maurice@biblibre.com> --- If I use at least one --check-* option, all --skip-* options are ignored, which make sense given that when a --check-* option is used all other checks are disabled. But this is not documented. And it would be nice to have a warning when using --skip-* and --check-* options together. The code can be improved too. For instance: CheckItemsBranch() if $options{'check-branch'} || $options{'skip-branch'}; is not easy to understand ($options{skip-branch} == 1 means "do not skip" which is confusing) Failing QA for lack of documentation and also for what Martin said in comment 19 (more detailed commit message, including a test plan) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |julian.maurice@biblibre.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Alexandre <alexandre.noel@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alexandre.noel@inlibro.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Alexandre <alexandre.noel@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #21 from Alexandre <alexandre.noel@inlibro.com> --- Created attachment 167766 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167766&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional We can specify which sections to search for data inconsistencies using check options, or which sections to skip using skip options. TEST PLAN: Make sure there are data inconsistencies present in at least two categories to validate the results of the command. 1. Apply the patch 2. Run the command without any argument and confirm that every inconsistences are reported. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Alexandre <alexandre.noel@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #167766|0 |1 is obsolete| | --- Comment #22 from Alexandre <alexandre.noel@inlibro.com> --- Created attachment 167787 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167787&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional We can specify which sections to search for data inconsistencies using check options, or which sections to skip using skip options. TEST PLAN: Make sure there are data inconsistencies present in at least two categories to validate the results of the command. 1. Apply the patch 2. Run the command without any argument and confirm that every inconsistences are reported. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Alexandre <alexandre.noel@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com, | |philippe.blouin@inlibro.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|phan-tung.bui@inlibro.com |alexandre.noel@inlibro.com CC| |david@davidnind.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166117|0 |1 is obsolete| | --- Comment #23 from David Nind <david@davidnind.com> --- Created attachment 168953 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=168953&action=edit Bug 36027: Change interactive to pass param and tidy the code Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #167787|0 |1 is obsolete| | --- Comment #24 from David Nind <david@davidnind.com> --- Created attachment 168954 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=168954&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional We can specify which sections to search for data inconsistencies using check options, or which sections to skip using skip options. TEST PLAN: Make sure there are data inconsistencies present in at least two categories to validate the results of the command. 1. Apply the patch 2. Run the command without any argument and confirm that every inconsistences are reported. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This enhancement updates release notes| |the data inconsistencies | |script | |(misc/maintenance/search_fo | |r_data_inconsistencies.pl) | |so that you can optionally | |run specific checks, rather | |than running all the checks | |at the same time. | | | |Examples: | |- Run all checks | |(no change): | |misc/maintenance/search_for | |_data_inconsistencies.pl | |- | |Run checks for item type | |issues and patrons with an | |invalid age for their | |category: | |misc/maintenance/search_for | |_data_inconsistencies.pl | |--check-status --check-age | |- Skip specific checks, but | |run all the others: | |misc/maintenance/search_for | |_data_inconsistencies.pl | |--skip-status --skip-title | | | |Use | |misc/maintenance/search_for | |_data_inconsistencies.pl | |--help for all the check | |and skip options. Severity|normal |enhancement --- Comment #25 from David Nind <david@davidnind.com> --- Thanks Alexandre for the patch. I've signed this off, as it does what it says it will do. I have made some comments. I will leave it up to you (or the QA Team) to decide if these need addressing. I think this is an enhancement, rather than fixing something that is wrong. So I have changed the status. I have also added a draft release note. Testing notes (using koha-testing-docker (KTD)): 1. Set up data so there are inconsistencies: 1.1 There is already one inconsistency in the sample data, for record 369 (encoding errors) 1.2 I created an inconsistency for patron age and patron category type: . koha-mysql kohadev . update borrowers set categorycode = "K" where borrowernumber=49; 2. Step 3, ran check with: misc/maintenance/search_for_data_inconsistencies.pl --check-status --check-age 3. Step 4, ran check with: misc/maintenance/search_for_data_inconsistencies.pl --skip-status 4. Step 5, ran check with: misc/maintenance/search_for_data_inconsistencies.pl --check-age --skip-status Some comments: 1. The check-status option includes checking for item type issues (as described in the help description), but it also checks for invalid MARCXML. I think that the MARCXML check should have its own option (--check-marcxml Check for invalid MARCXML in bibliographic records). 1.1 I'm not sure how hard this to do (I am not a developer). 1.2 I'm not sure whether there are other checks in the script that are not included in the description and have their own options. 1.2 Instead of --check-status, --check-itemtype 2. Instead of --check-branch, use --check-library. While branch is used in the underlying code, the terminology guidelines suggest we should use library. (https://wiki.koha-community.org/wiki/Terminology#L) 3. Warning when you use --check-* and --skip-* options in the same command (step 5). I've made a wording change suggestion. The current text says to me that if you use skip and check options in the same command, the skip options are ignored. That is, it will ignore/not run the skip options: . Current warning: "Warning : skip options are ignored when check options are provided" . Suggested warning: "Warning: The --skip-* option(s) were ignored. Only the --check-* options in the command were run." 4. Description and help text. I think this needs some work with formatting and the text. I would be happy to add a follow-up patch with some suggested changes. 5. The QA script has warnings (to run the QA script when using koha-testing-docker): qa WARN misc/maintenance/search_for_data_inconsistencies.pl WARN tidiness The file is less tidy than before (bad/messy lines before: 83, now: 206) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=31143 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #168953|0 |1 is obsolete| | Attachment #168954|0 |1 is obsolete| | --- Comment #26 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 169228 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169228&action=edit Bug 36027: Change interactive to pass param and tidy the code Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #27 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 169229 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169229&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional We can specify which sections to search for data inconsistencies using check options, or which sections to skip using skip options. TEST PLAN: Make sure there are data inconsistencies present in at least two categories to validate the results of the command. 1. Apply the patch 2. Run the command without any argument and confirm that every inconsistences are reported. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com QA Contact|julian.maurice@biblibre.com |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #28 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I know that bug 36039 is blocked by this, but I'd like to ask some questions at least before pushing. 1) Quite a lot of untidyness added, please fix. WARN misc/maintenance/search_for_data_inconsistencies.pl WARN tidiness The file is less tidy than before (bad/messy lines before: 83, now: 206) Can we fix this here or on the other bug? 2) New parameters (not blocker) I wonder if the chosen pattern is good as it requires that we add 2 new parameters for every check that we add to the script. We do have something similar in cleanup_database.pl for action logs and it seems more logical to me. --log-module Specify which action log modules to trim. Repeatable. --preserve-log Specify which action logs to exclude. Repeatable. --logs DAYS purge entries from action_logs older than DAYS days. Defaults to 180 days if no days specified. So instead of a unique parameter, you can use preserve-log with different codes. I am not sure what is better, just putting the question out here. 3) Script should not start without -c or any parameters... (not blocker) This is something for a separate bug. But we once said that scripts should output help when run without parameter and require a confirmation flag. As this can run quite a time, this still seems a good idea... (leaving this here for filing later) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=37725 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169228|0 |1 is obsolete| | --- Comment #29 from Hammat wele <hammat.wele@inlibro.com> --- Created attachment 170847 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170847&action=edit Bug 36027: Change interactive to pass param and tidy the code Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169229|0 |1 is obsolete| | --- Comment #30 from Hammat wele <hammat.wele@inlibro.com> --- Created attachment 170848 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170848&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional We can specify which sections to search for data inconsistencies using check options, or which sections to skip using skip options. TEST PLAN: Make sure there are data inconsistencies present in at least two categories to validate the results of the command. 1. Apply the patch 2. Run the command without any argument and confirm that every inconsistences are reported. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #31 from Hammat wele <hammat.wele@inlibro.com> --- Created attachment 170849 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170849&action=edit Bug 36027: (follow-up) Tidy search_for_data_inconsistencies.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff CC| |hammat.wele@inlibro.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Olivier Vezina <ovezina@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170847|0 |1 is obsolete| | --- Comment #32 from Olivier Vezina <ovezina@inlibro.com> --- Created attachment 173269 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173269&action=edit Bug 36027: Change interactive to pass param and tidy the code Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Olivier Vezina <ovezina@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170848|0 |1 is obsolete| | --- Comment #33 from Olivier Vezina <ovezina@inlibro.com> --- Created attachment 173270 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173270&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional We can specify which sections to search for data inconsistencies using check options, or which sections to skip using skip options. TEST PLAN: Make sure there are data inconsistencies present in at least two categories to validate the results of the command. 1. Apply the patch 2. Run the command without any argument and confirm that every inconsistences are reported. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Olivier Vezina <ovezina@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170849|0 |1 is obsolete| | --- Comment #34 from Olivier Vezina <ovezina@inlibro.com> --- Created attachment 173271 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173271&action=edit Bug 36027: (follow-up) Tidy search_for_data_inconsistencies.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Olivier Vezina <ovezina@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ovezina@inlibro.com --- Comment #35 from Olivier Vezina <ovezina@inlibro.com> --- Fixed merge conflict for master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #36 from David Nind <david@davidnind.com> --- Everything still seems to work as expected. However, I've Failed QA as any command run generates this line in the output, which I don't think it should: Subroutine CheckRelationshipsLoop redefined at misc/maintenance/search_for_data_inconsistencies.pl line 708. Updated testing notes (from comment #25) ===================== Testing notes (using KTD): 1. Set up data so that there are inconsistencies: 1.1 There is already one inconsistency in the sample data, for record 369 (encoding errors) 1.2 I created an inconsistency for patron age and patron category type: . koha-mysql kohadev . update borrowers set categorycode = "K" where borrowernumber=49; 2. Step 3, ran check with: misc/maintenance/search_for_data_inconsistencies.pl --check-status --check-age 3. Step 4, ran check with: misc/maintenance/search_for_data_inconsistencies.pl --skip-status 4. Step 5, ran check with: misc/maintenance/search_for_data_inconsistencies.pl --check-age --skip-status Updated comments (from me, comment #25) ================ I'll file separate bugs for these comments. 1. The check-status option includes checking for item type issues (as described in the help description), but it also checks for invalid MARCXML. I think that the MARCXML check should have its own option (--check-marcxml Check for invalid MARCXML in bibliographic records). 1.1 I'm not sure how hard this to do (I am not a developer). 1.2 I'm not sure whether there are other checks in the script that are not included in the description and have their own options. 1.2 Instead of --check-status, --check-itemtype 2. Instead of --check-branch, use --check-library. While branch is used in the underlying code, the terminology guidelines suggest we should use library. (https://wiki.koha-community.org/wiki/Terminology#L) 3. Warning when you use --check-* and --skip-* options in the same command (step 5). I've made a wording change suggestion. The current text says to me that if you use skip and check options in the same command, the skip options are ignored. That is, it will ignore/not run the skip options: . Current warning: "Warning : skip options are ignored when check options are provided" . Suggested warning: "Warning: The --skip-* option(s) were ignored. Only the --check-* options in the command were run." 4. Description and help text. I think this needs some work with formatting and the text. I will add a follow-up patch with some suggested changes. Additional comments (from Katrin, comment #28) =================== Just so that these don't get lost from Katrin's comments. 1. New parameters (not blocker) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ I wonder if the chosen pattern is good as it requires that we add 2 new parameters for every check that we add to the script. We do have something similar in cleanup_database.pl for action logs and it seems more logical to me. --log-module Specify which action log modules to trim. Repeatable. --preserve-log Specify which action logs to exclude. Repeatable. --logs DAYS purge entries from action_logs older than DAYS days. Defaults to 180 days if no days specified. So instead of a unique parameter, you can use preserve-log with different codes. I am not sure what is better, just putting the question out here. 2. Script should not start without -c or any parameters... (not blocker) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This is something for a separate bug. But we once said that scripts should output help when run without parameter and require a confirmation flag. As this can run quite a time, this still seems a good idea... (leaving this here for filing later) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #37 from Olivier Vezina <ovezina@inlibro.com> --- Thank you for testing, I'll check what i can do for the marcxml i am not sure myself. I also agree that the terminology should be change to library! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #38 from Olivier Vezina <ovezina@inlibro.com> --- Created attachment 173556 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173556&action=edit Bug 36027: (follow-up) Fixed code duplication and add mandatory options An option is now mandatory for the script to lunch. 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Olivier Vezina <ovezina@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #39 from Olivier Vezina <ovezina@inlibro.com> --- It seems possible to add the MarcXML report as its own option. If this feature could be helpful, it should be possible to implement. I was not exactly sure if what i did with the script without options was the right thing to do. It now blocks you and shows you the help sheet. I'll fix it if its wrong -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|36039 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 [Bug 36039] The output of audit_database.pl should be accessible through the UI -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Eric Bégin <eric.begin@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|alexandre.noel@inlibro.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #40 from Magnus Enger <magnus@libriotech.no> --- *** Bug 31093 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=38755 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@libriotech.no Blocks| |38755 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38755 [Bug 38755] search_for_data_inconsistencies.pl: Fix list of inconsistencies -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #41 from Magnus Enger <magnus@libriotech.no> --- Would it make sense to have a --check-all option, to run all the checks without specifying each one? This would also make it possible to run all checks added in the future. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lisette@bywatersolutions.co | |m Keywords| |no-sandbox -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #42 from Hammat wele <hammat.wele@inlibro.com> --- Created attachment 177270 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177270&action=edit Bug 36027: (follow-up) Adding check-all option This patch add --check-all option, to run all the checks without specifying each one 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with 'check-all' arguments (ex: ./search_for_data_inconsistencies.pl --check-all), confirm that all the check is done. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177270|0 |1 is obsolete| | --- Comment #43 from Hammat wele <hammat.wele@inlibro.com> --- Created attachment 177283 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177283&action=edit Bug 36027: (follow-up) Adding check-all option This patch add --check-all option, to run all the checks without specifying each one 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with 'check-all' arguments (ex: ./search_for_data_inconsistencies.pl --check-all), confirm that all the check is done. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173269|0 |1 is obsolete| | --- Comment #44 from David Nind <david@davidnind.com> --- Created attachment 177438 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177438&action=edit Bug 36027: Change interactive to pass param and tidy the code Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173270|0 |1 is obsolete| | --- Comment #45 from David Nind <david@davidnind.com> --- Created attachment 177439 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177439&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional We can specify which sections to search for data inconsistencies using check options, or which sections to skip using skip options. TEST PLAN: Make sure there are data inconsistencies present in at least two categories to validate the results of the command. 1. Apply the patch 2. Run the command without any argument and confirm that every inconsistences are reported. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173271|0 |1 is obsolete| | --- Comment #46 from David Nind <david@davidnind.com> --- Created attachment 177440 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177440&action=edit Bug 36027: (follow-up) Tidy search_for_data_inconsistencies.pl Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173556|0 |1 is obsolete| | --- Comment #47 from David Nind <david@davidnind.com> --- Created attachment 177441 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177441&action=edit Bug 36027: (follow-up) Fixed code duplication and add mandatory options An option is now mandatory for the script to lunch. 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177283|0 |1 is obsolete| | --- Comment #48 from David Nind <david@davidnind.com> --- Created attachment 177442 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177442&action=edit Bug 36027: (follow-up) Adding check-all option This patch add --check-all option, to run all the checks without specifying each one 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with 'check-all' arguments (ex: ./search_for_data_inconsistencies.pl --check-all), confirm that all the check is done. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #49 from David Nind <david@davidnind.com> --- Created attachment 177443 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177443&action=edit Bug 36027: (follow-up) Update help text and error message This updates the help text and the error message when --check* and --skip* options are used in the same command. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|search_for_data_inconsisten |search_for_data_inconsisten |cies.pl - make each section |cies.pl - add options so |optional |you can choose what checks | |to run Text to go in the|This enhancement updates |This enhancement updates release notes|the data inconsistencies |the data inconsistencies |script |script |(misc/maintenance/search_fo |(misc/maintenance/search_fo |r_data_inconsistencies.pl) |r_data_inconsistencies.pl) |so that you can optionally |so that you can now choose |run specific checks, rather |what checks to run, instead |than running all the checks |of running all the checks |at the same time. |at once (which can take a | |lot of time for larger |Examples: |libraries). |- Run all checks | |(no change): |Running the |misc/maintenance/search_for |script without any options |_data_inconsistencies.pl |now displays the help text, |- |instead of running all the |Run checks for item type |checks (for consistency |issues and patrons with an |with other scripts). To run |invalid age for their |all the checks at the same |category: |time, use the --check-all |misc/maintenance/search_for |option. |_data_inconsistencies.pl | |--check-status --check-age |Examples: |- Skip specific checks, but |- Run |run all the others: |all checks (--check-all): |misc/maintenance/search_for |misc/maintenance/search_for |_data_inconsistencies.pl |_data_inconsistencies.pl |--skip-status --skip-title |--check-all | |- Run checks |Use |for item type issues and |misc/maintenance/search_for |patrons with an invalid age |_data_inconsistencies.pl |for their patron category: |--help for all the check |misc/maintenance/search_for |and skip options. |_data_inconsistencies.pl | |--check-status --check-age | |- Skip specific checks, but | |run all the others: | |misc/maintenance/search_for | |_data_inconsistencies.pl | |--skip-status --skip-title | | | |Use | |misc/maintenance/search_for | |_data_inconsistencies.pl | |--help for all the check | |and skip options. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #50 from David Nind <david@davidnind.com> --- Created attachment 177444 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177444&action=edit Bug 36027: (follow-up) Fix QA script errors -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #51 from David Nind <david@davidnind.com> --- Updated testing notes (from comment #25 and #36) ===================== Testing notes (using KTD): 1. Apply the patch. 2. Set up data so that there are inconsistencies: 2.1 There is already one inconsistency in the sample data, for record 369 (encoding errors) 2.2 I created an inconsistency for patron age and patron category type: . koha-mysql kohadev . update borrowers set categorycode = "K" where borrowernumber=49; 3. Run the script without any options: misc/maintenance/search_for_data_inconsistencies.pl ==> It should output the help for the script (a change in behavour - previously it ran all checks). 3. Run the script with one or more check options: misc/maintenance/search_for_data_inconsistencies.pl --check-status --check-age ==> It should output the inconsistencies (an invalid MARCXML record and an invalid age category). 4. Run the script with one or more --skip* options: misc/maintenance/search_for_data_inconsistencies.pl --skip-status ==> It should skip the checks specified. 6. Run the script with a check option and a skip option: misc/maintenance/search_for_data_inconsistencies.pl --check-age --skip-status ==> The skip option should be ignored (not run) and only the check option should be run. 7. Run the script with all options: misc/maintenance/search_for_data_inconsistencies.pl --check-all ==> It should output the results from running all the checks (in this case with KTD, the invalid MARCXML document and invalid age category). 8. Check the help to make sure it is improved, makes sense, and incorporates all the changes made by this bug: misc/maintenance/search_for_data_inconsistencies.pl --help Updated comments (from me, comment #25 and #36) =============================================== I'll file separate bugs for these comments when the bug passes QA: 1. The check-status option includes checking for item type issues (as described in the help description), but it also checks for invalid MARCXML. I think that the MARCXML check should have its own option (--check-marcxml Check for invalid MARCXML in bibliographic records). 1.1 I'm not sure how hard this to do (I am not a developer). 1.2 I'm not sure whether there are other checks in the script that are not included in the description and have their own options. 1.2 Instead of --check-status, --check-itemtype 2. Instead of --check-branch, use --check-library. While branch is used in the underlying code, the terminology guidelines suggest we should use library. (https://wiki.koha-community.org/wiki/Terminology#L) Follow-up - help and error message text ======================================= I've added a follow-up for: 1. Warning when you use --check-* and --skip-* options in the same command (step 6). The current text says (to me, anyway) that if you use skip and check options in the same command, the skip options are ignored. That is, it will ignore/not run the skip options: . Current warning: "Warning : skip options are ignored when check options are provided" . Suggested warning: "Warning: The --skip-* option(s) were ignored. Only the --check-* options in the command were run." 2. Description and help text - some formatting and text changes. Additional comments (from Katrin, comment #28) ============================================== Just so that these don't get lost. 1. New parameters (not blocker) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ I wonder if the chosen pattern is good as it requires that we add 2 new parameters for every check that we add to the script. We do have something similar in cleanup_database.pl for action logs and it seems more logical to me. --log-module Specify which action log modules to trim. Repeatable. --preserve-log Specify which action logs to exclude. Repeatable. --logs DAYS purge entries from action_logs older than DAYS days. Defaults to 180 days if no days specified. So instead of a unique parameter, you can use preserve-log with different codes. I am not sure what is better, just putting the question out here. (I think that would be better, for example, two options --check and --skip with a list of codes for each check available. David) 2. Script should not start without -c or any parameters... (not blocker) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This is something for a separate bug. But we once said that scripts should output help when run without parameter and require a confirmation flag. As this can run quite a time, this still seems a good idea... (leaving this here for filing later) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Signed Off |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177438|0 |1 is obsolete| | Attachment #177439|0 |1 is obsolete| | Attachment #177440|0 |1 is obsolete| | Attachment #177441|0 |1 is obsolete| | Attachment #177442|0 |1 is obsolete| | Attachment #177443|0 |1 is obsolete| | Attachment #177444|0 |1 is obsolete| | --- Comment #52 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 177849 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177849&action=edit Bug 36027: Change interactive to pass param and tidy the code Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #53 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 177850 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177850&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional We can specify which sections to search for data inconsistencies using check options, or which sections to skip using skip options. TEST PLAN: Make sure there are data inconsistencies present in at least two categories to validate the results of the command. 1. Apply the patch 2. Run the command without any argument and confirm that every inconsistences are reported. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #54 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 177851 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177851&action=edit Bug 36027: (follow-up) Tidy search_for_data_inconsistencies.pl Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #55 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 177852 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177852&action=edit Bug 36027: (follow-up) Fixed code duplication and add mandatory options An option is now mandatory for the script to lunch. 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #56 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 177853 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177853&action=edit Bug 36027: (follow-up) Adding check-all option This patch add --check-all option, to run all the checks without specifying each one 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with 'check-all' arguments (ex: ./search_for_data_inconsistencies.pl --check-all), confirm that all the check is done. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #57 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 177854 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177854&action=edit Bug 36027: (follow-up) Update help text and error message This updates the help text and the error message when --check* and --skip* options are used in the same command. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #58 from Jonathan Druart <jonathan.druart@gmail.com> --- Patches rebased using the auto-rebase script. Please retest. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|alexandre.noel@inlibro.com |hammat.wele@inlibro.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #59 from David Nind <david@davidnind.com> --- (In reply to Jonathan Druart from comment #58)
Patches rebased using the auto-rebase script. Please retest.
Have retested. Step 4 of my testing notes in comment #51 no longer seems to work: 4. Run the script with one or more --skip* options: misc/maintenance/search_for_data_inconsistencies.pl --skip-status ==> It should skip the checks specified. It just outputs the help. It was a while ago since I last tested, so not sure whether it is supposed to do that or not. Otherwise, everything else seems to work OK. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Eric Bégin <eric.begin@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177849|0 |1 is obsolete| | --- Comment #60 from Eric Bégin <eric.begin@inLibro.com> --- Created attachment 180257 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180257&action=edit Bug 36027: Change interactive to pass param and tidy the code Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Eric Bégin <eric.begin@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177850|0 |1 is obsolete| | --- Comment #61 from Eric Bégin <eric.begin@inLibro.com> --- Created attachment 180258 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180258&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional We can specify which sections to search for data inconsistencies using check options, or which sections to skip using skip options. TEST PLAN: Make sure there are data inconsistencies present in at least two categories to validate the results of the command. 1. Apply the patch 2. Run the command without any argument and confirm that every inconsistences are reported. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Eric Bégin <eric.begin@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177851|0 |1 is obsolete| | --- Comment #62 from Eric Bégin <eric.begin@inLibro.com> --- Created attachment 180259 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180259&action=edit Bug 36027: (follow-up) Tidy search_for_data_inconsistencies.pl Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Eric Bégin <eric.begin@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177852|0 |1 is obsolete| | --- Comment #63 from Eric Bégin <eric.begin@inLibro.com> --- Created attachment 180260 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180260&action=edit Bug 36027: (follow-up) Fixed code duplication and add mandatory options An option is now mandatory for the script to lunch. 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Eric Bégin <eric.begin@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177853|0 |1 is obsolete| | --- Comment #64 from Eric Bégin <eric.begin@inLibro.com> --- Created attachment 180261 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180261&action=edit Bug 36027: (follow-up) Adding check-all option This patch add --check-all option, to run all the checks without specifying each one 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with 'check-all' arguments (ex: ./search_for_data_inconsistencies.pl --check-all), confirm that all the check is done. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Eric Bégin <eric.begin@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177854|0 |1 is obsolete| | --- Comment #65 from Eric Bégin <eric.begin@inLibro.com> --- Created attachment 180262 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180262&action=edit Bug 36027: (follow-up) Update help text and error message This updates the help text and the error message when --check* and --skip* options are used in the same command. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Eric Bégin <eric.begin@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |eric.begin@inLibro.com --- Comment #66 from Eric Bégin <eric.begin@inLibro.com> --- I test David's case and you get the help info since having --skip* does nothing if you don't use the --check-all option. When you wrote the test plan, that script was doing the equivalent of --check-all when no option was set. That is not the case anymore. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #67 from David Nind <david@davidnind.com> --- It no longer seems possible to use any of the --skip-* options, as far as I can see. Set up the data in KTD so that there is an inconsistency for patron age and patron category type: 1. koha-mysql kohadev 2. update borrowers set categorycode = "K" where borrowernumber=49; There is already a record in KTD with invalid MARCXML. Example: 1. Skip the age checks, but run all the others: misc/maintenance/search_for_data_inconsistencies.pl --skip-age ==> Result: You just get the help. ==> Expected result: All checks are run, except the status checks 2. If you include the --check-all in the command, the --skip-* is ignored and all the checks are run: misc/maintenance/search_for_data_inconsistencies.pl --skip-status --check-all ==> Result: . You get a warning "Warning: The --skip-* option(s) were ignored. Only the --check-* options in the command were run." . All the checks are run -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180257|0 |1 is obsolete| | --- Comment #68 from Hammat wele <hammat.wele@inlibro.com> --- Created attachment 187299 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187299&action=edit Bug 36027: Change interactive to pass param and tidy the code Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180258|0 |1 is obsolete| | --- Comment #69 from Hammat wele <hammat.wele@inlibro.com> --- Created attachment 187300 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187300&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional We can specify which sections to search for data inconsistencies using check options, or which sections to skip using skip options. TEST PLAN: Make sure there are data inconsistencies present in at least two categories to validate the results of the command. 1. Apply the patch 2. Run the command without any argument and confirm that every inconsistences are reported. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180259|0 |1 is obsolete| | --- Comment #70 from Hammat wele <hammat.wele@inlibro.com> --- Created attachment 187301 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187301&action=edit Bug 36027: (follow-up) Tidy search_for_data_inconsistencies.pl Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180260|0 |1 is obsolete| | --- Comment #71 from Hammat wele <hammat.wele@inlibro.com> --- Created attachment 187302 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187302&action=edit Bug 36027: (follow-up) Fixed code duplication and add mandatory options An option is now mandatory for the script to lunch. 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180261|0 |1 is obsolete| | --- Comment #72 from Hammat wele <hammat.wele@inlibro.com> --- Created attachment 187303 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187303&action=edit Bug 36027: (follow-up) Adding check-all option This patch add --check-all option, to run all the checks without specifying each one 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with 'check-all' arguments (ex: ./search_for_data_inconsistencies.pl --check-all), confirm that all the check is done. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180262|0 |1 is obsolete| | --- Comment #73 from Hammat wele <hammat.wele@inlibro.com> --- Created attachment 187304 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187304&action=edit Bug 36027: (follow-up) Update help text and error message This updates the help text and the error message when --check* and --skip* options are used in the same command. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #74 from Hammat wele <hammat.wele@inlibro.com> --- Created attachment 187320 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187320&action=edit Bug 36027: (follow-up) Fix the use of --skip-* options -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply --- Comment #75 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Needs a rebase now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Chris Mathevet <chris.mathevet@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Chris Mathevet <chris.mathevet@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187299|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Chris Mathevet <chris.mathevet@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187300|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Chris Mathevet <chris.mathevet@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187301|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Chris Mathevet <chris.mathevet@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187302|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Chris Mathevet <chris.mathevet@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187303|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Chris Mathevet <chris.mathevet@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187304|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Chris Mathevet <chris.mathevet@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187320|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #76 from Chris Mathevet <chris.mathevet@inLibro.com> --- Created attachment 199043 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199043&action=edit Bug 36027: Change interactive to pass param and tidy the code Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #77 from Chris Mathevet <chris.mathevet@inLibro.com> --- Created attachment 199044 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199044&action=edit Bug 36027: search_for_data_inconsistencies.pl - make each section optional We can specify which sections to search for data inconsistencies using check options, or which sections to skip using skip options. TEST PLAN: Make sure there are data inconsistencies present in at least two categories to validate the results of the command. 1. Apply the patch 2. Run the command without any argument and confirm that every inconsistences are reported. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #78 from Chris Mathevet <chris.mathevet@inLibro.com> --- Created attachment 199045 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199045&action=edit Bug 36027: (follow-up) Tidy search_for_data_inconsistencies.pl Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #79 from Chris Mathevet <chris.mathevet@inLibro.com> --- Created attachment 199046 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199046&action=edit Bug 36027: (follow-up) Fixed code duplication and add mandatory options An option is now mandatory for the script to lunch. 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #80 from Chris Mathevet <chris.mathevet@inLibro.com> --- Created attachment 199047 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199047&action=edit Bug 36027: (follow-up) Adding check-all option This patch add --check-all option, to run all the checks without specifying each one 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with 'check-all' arguments (ex: ./search_for_data_inconsistencies.pl --check-all), confirm that all the check is done. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #81 from Chris Mathevet <chris.mathevet@inLibro.com> --- Created attachment 199048 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199048&action=edit Bug 36027: (follow-up) Update help text and error message This updates the help text and the error message when --check* and --skip* options are used in the same command. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Bégin <eric.begin@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 --- Comment #82 from Chris Mathevet <chris.mathevet@inLibro.com> --- Created attachment 199049 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199049&action=edit Bug 36027: (follow-up) Fix the use of --skip-* options Patch from commit 553a4da -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org