[Bug 36039] New: Output of database_audit.pl should be accessible through UI
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Bug ID: 36039 Summary: Output of database_audit.pl should be accessible through UI Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: In Discussion Severity: enhancement Priority: P5 - low Component: About Assignee: koha-bugs@lists.koha-community.org Reporter: pedro.amorim@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org CC: david@davidnind.com, dcook@prosentient.com.au, katrin.fischer@bsz-bw.de, martin.renvoize@ptfs-europe.com Depends on: 32693 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32693 [Bug 32693] The 'About Koha' page loads slowly -- 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=36039 --- Comment #1 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 161892 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161892&action=edit Bug 36039: PoC continuing from discussion in bug 34063. Test plan: 1) Apply patch 2) Install new dep: sudo cpanm HTML::TextToHTML 3) Restart plack and access /cgi-bin/koha/about.pl?tab=database -- 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=36039 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |34064 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34064 [Bug 34064] Compare kohastructure.sql against current database using database audit script -- 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=36039 --- Comment #2 from David Cook <dcook@prosentient.com.au> --- In hindsight, I should've put the code for "audit_database.pl" into a module. One of those things where a person is like "oh I'll just write a quick script for this..." and it becomes more useful than that heh. Also, I don't think this PoC would work in a non-git environment, since "audit_database.pl" needs to be told where to find kohastructure.sql. So I reckon we refactor this to put it into a module (Koha::Database::Auditor perhaps), and then in about.pl we could find kohastructure.sql (using "intranetdir" from koha-conf.xml probably) and pass it as a parameter to Koha::Database::Auditor. -- 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=36039 --- Comment #3 from David Cook <dcook@prosentient.com.au> --- An additional note, I'm not a fan of shelling out. It's sometimes unavoidable when integrating with a third-party app, but since we're using our own code here I think we can do better. Koha also has a lot of dependencies already. I don't know if we want to add another one just for this. HTML::TextToHTML is unmaintained, and I think we could manage without it. -- 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=36039 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- I am stoked for bug 32693 and this one though. I'm keeping it on my TODO list. I think having it in the About will also help motivate people to fix things. Sometimes when this information stays hidden on the sysadmin side of things, it's really easy to ignore. But if you have a web admin go to that tab and say "oh hey my database is our of sync..." it's more likely to get fixed sooner. -- 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=36039 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com, | |nick@bywatersolutions.com, | |tomascohen@gmail.com --- Comment #5 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Hi David, that's awesome, I broadly agree with everything you said. Yes the PoC would not work in a non-git environment as it was designed only to be a platform for the discussion we're now having. I also strongly agree that having these scripts/reports accessible through the UI will make this info more readily available to every Staff member, not just sys admins, thus likely resulting in a faster call to action to fixing/preventing bugs in individual systems due to database issues. I'm a big fan of the Koha::Database::Auditor idea, and would like to hear what others also think about this. -- 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=36039 --- Comment #6 from David Nind <david@davidnind.com> --- While I'm not familiar with the script this is based on, I think it is a good idea. I'm assuming it is a good tool to use before an upgrade, to make sure that there are no potential issues (as far as this is possible to do before an upgrade). After an upgrade, it might not be accessible if the upgrade doesn't complete. Happy to help with the messages displayed so that they are more staff friendly. I think the tab might be better placed after the system information tab. -- 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=36039 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm always happy to see things moved into modules from scripts.. it makes them more easily accessible for re-use and testable. I also agree, it's a good move to lower the requirements for further dependancies. -- 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=36039 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- I can look at refactoring audit_database.pl later. Got some other priorities at the moment. I was just thinking in terms of showing the output in the web UI... I wonder if there are security implications there. In theory, it could allow someone with staff access to see if an attack has managed to change the database schema. But I suppose our concern should be more with stopping attacks. Just some food for thought. -- 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=36039 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=35901 -- 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=36039 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- 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=36039 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161892|0 |1 is obsolete| | --- Comment #9 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 162462 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162462&action=edit Bug 36039: Move audit_database script logic to a dedicated module Test plan #1 ensure same script functionality (same as bug 34064): 0. Apply patch 1. vi ./installer/data/mysql/kohastructure.sql 2. Comment out some columns, change NULL status, or whatever you like 3. perl misc/maintenance/audit_database.pl \ --filename /kohadevbox/koha/installer/data/mysql/kohastructure.sql 4. Note that the output includes SQL commands to change the database to match the new kohastructure.sql 5a. Try using koha-foreach and note that the database name appears above the database comparison 5b. koha-foreach "perl misc/maintenance/audit_database.pl \ --filename /kohadevbox/koha/installer/data/mysql/kohastructure.sql" Test plan #2: 1. After doing test plan #1, access the new 'database audit' tab, visit: <staff_url>/cgi-bin/koha/about.pl?tab=database 2. Ensure the output is the same as test plan #1 -- 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=36039 --- Comment #10 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 162463 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162463&action=edit Bug 36039: New 'datatabase audit' tab in about page -- 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=36039 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Assignee|koha-bugs@lists.koha-commun |pedro.amorim@ptfs-europe.co |ity.org |m -- 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=36039 --- Comment #11 from David Cook <dcook@prosentient.com.au> --- Thanks for taking this on, Pedro. I've been super busy, so I really appreciate it. (I probably would've done it a bit differently. Instead of passing a "is_cli" parameter to Koha::Database::Auditor, I would just return a data structure, and then based on whether it's about.pl or audit_database.pl handle the output accordingly. As for the printing of strings within functions, that could be done using a logger object. But I don't think it's the end of the world keeping it as is.) Once again thanks for taking this on. I've still got a lot on, but I'm keeping my eye on this one, and will come back once I have some time if folk haven't already gone through it by then. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 --- Comment #12 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Hi David, fair points. I'd love to see a follow-up further improving this! Thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36027 --- Comment #13 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- search_for_data_inconsistencies.pl could also be merged into to this Koha::Database::Auditor. That cron should also be invoking methods from the module instead of having logic built-in. This new "Database" tab in about page could be extended to include these data inconsistencies on top of database diff, perhaps with async actions, or else that tab would take very long to render. Thoughts? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162462|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 166097 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166097&action=edit Bug 36039: Move audit_database script logic to a dedicated module Test plan #1 ensure same script functionality (same as bug 34064): 0. Apply patch 1. vi ./installer/data/mysql/kohastructure.sql 2. Comment out some columns, change NULL status, or whatever you like 3. perl misc/maintenance/audit_database.pl \ --filename /kohadevbox/koha/installer/data/mysql/kohastructure.sql 4. Note that the output includes SQL commands to change the database to match the new kohastructure.sql 5a. Try using koha-foreach and note that the database name appears above the database comparison 5b. koha-foreach "perl misc/maintenance/audit_database.pl \ --filename /kohadevbox/koha/installer/data/mysql/kohastructure.sql" Test plan #2: 1. After doing test plan #1, access the new 'database audit' tab, visit: <staff_url>/cgi-bin/koha/about.pl?tab=database 2. Ensure the output is the same as test plan #1 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=36039 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162463|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 166098 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166098&action=edit Bug 36039: New 'datatabase audit' tab in about page 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=36039 --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 166099 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166099&action=edit Bug 36039: (follow-up) Move view logic to script/template This patch updates the module as suggested by David to handle just the diff code and leaves the display logic up to the templates and script. 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=36039 Martin Renvoize <martin.renvoize@ptfs-europe.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=36039 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Note that Commenter also contains similar code (while only doing from the db comment perspective). Also reading structure etc. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Signed Off --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Looks very nice! Maybe creating another diff display and showing the alter commands less prominently would be an improvement to think about still Hard to require tests here. Structure changes imply commits etc. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Jesse Maseto <jesse@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166097|0 |1 is obsolete| | Attachment #166098|0 |1 is obsolete| | Attachment #166099|0 |1 is obsolete| | --- Comment #20 from Jesse Maseto <jesse@bywatersolutions.com> --- Created attachment 166155 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166155&action=edit Bug 36039: Move audit_database script logic to a dedicated module Test plan #1 ensure same script functionality (same as bug 34064): 0. Apply patch 1. vi ./installer/data/mysql/kohastructure.sql 2. Comment out some columns, change NULL status, or whatever you like 3. perl misc/maintenance/audit_database.pl \ --filename /kohadevbox/koha/installer/data/mysql/kohastructure.sql 4. Note that the output includes SQL commands to change the database to match the new kohastructure.sql 5a. Try using koha-foreach and note that the database name appears above the database comparison 5b. koha-foreach "perl misc/maintenance/audit_database.pl \ --filename /kohadevbox/koha/installer/data/mysql/kohastructure.sql" Test plan #2: 1. After doing test plan #1, access the new 'database audit' tab, visit: <staff_url>/cgi-bin/koha/about.pl?tab=database 2. Ensure the output is the same as test plan #1 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: JesseM <jesse@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 --- Comment #21 from Jesse Maseto <jesse@bywatersolutions.com> --- Created attachment 166156 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166156&action=edit Bug 36039: New 'datatabase audit' tab in about page Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: JesseM <jesse@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 --- Comment #22 from Jesse Maseto <jesse@bywatersolutions.com> --- Created attachment 166157 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166157&action=edit Bug 36039: (follow-up) Move view logic to script/template This patch updates the module as suggested by David to handle just the diff code and leaves the display logic up to the templates and script. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: JesseM <jesse@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |36027 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 [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=36039 Bug 36039 depends on bug 32693, which changed state. Bug 32693 Summary: The 'About Koha' page loads slowly https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32693 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com QA Contact|m.de.rooy@rijksmuseum.nl |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=36039 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=36039 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166155|0 |1 is obsolete| | Attachment #166156|0 |1 is obsolete| | Attachment #166157|0 |1 is obsolete| | --- Comment #23 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 169225 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169225&action=edit Bug 36039: Move audit_database script logic to a dedicated module Test plan #1 ensure same script functionality (same as bug 34064): 0. Apply patch 1. vi ./installer/data/mysql/kohastructure.sql 2. Comment out some columns, change NULL status, or whatever you like 3. perl misc/maintenance/audit_database.pl \ --filename /kohadevbox/koha/installer/data/mysql/kohastructure.sql 4. Note that the output includes SQL commands to change the database to match the new kohastructure.sql 5a. Try using koha-foreach and note that the database name appears above the database comparison 5b. koha-foreach "perl misc/maintenance/audit_database.pl \ --filename /kohadevbox/koha/installer/data/mysql/kohastructure.sql" Test plan #2: 1. After doing test plan #1, access the new 'database audit' tab, visit: <staff_url>/cgi-bin/koha/about.pl?tab=database 2. Ensure the output is the same as test plan #1 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: JesseM <jesse@bywatersolutions.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=36039 --- Comment #24 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 169226 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169226&action=edit Bug 36039: New 'datatabase audit' tab in about page Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: JesseM <jesse@bywatersolutions.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=36039 --- Comment #25 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 169227 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169227&action=edit Bug 36039: (follow-up) Move view logic to script/template This patch updates the module as suggested by David to handle just the diff code and leaves the display logic up to the templates and script. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: JesseM <jesse@bywatersolutions.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=36039 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This enhancement makes the release notes| |misc/database_audit.pl | |script (added in Koha | |23.11) available in the | |staff interface - About | |Koha > Database audit tab. | |The script compares the | |instance's database against | |kohastructure.sql and | |identifies any differences | |that need fixing. This is | |useful for identifying | |database issues that should | |be addressed before running | |a maintenance or release | |update. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Bug 36039 depends on bug 34064, which changed state. Bug 34064 Summary: Compare kohastructure.sql against current database using database audit script https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34064 What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |Needs documenting Resolution|FIXED |--- -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Output of database_audit.pl |Output of audit_database.pl |should be accessible |should be accessible |through UI |through UI Text to go in the|This enhancement makes the |This enhancement makes the release notes|misc/database_audit.pl |misc/maintenance/audit_data |script (added in Koha |base.pl script (added in |23.11) available in the |Koha 23.11) available in |staff interface - About |the staff interface - About |Koha > Database audit tab. |Koha > Database audit tab. |The script compares the |The script compares the |instance's database against |instance's database against |kohastructure.sql and |kohastructure.sql and |identifies any differences |identifies any differences |that need fixing. This is |that need fixing. This is |useful for identifying |useful for identifying |database issues that should |database issues that should |be addressed before running |be addressed before running |a maintenance or release |a maintenance or release |update. |update. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Output of audit_database.pl |The output of |should be accessible |audit_database.pl should be |through UI |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=36039 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |BLOCKED --- Comment #26 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Blocked by bug 36027 ... but that should be quick to resolve. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169225|0 |1 is obsolete| | --- Comment #27 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 174403 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174403&action=edit Bug 36039: Move audit_database script logic to a dedicated module Test plan #1 ensure same script functionality (same as bug 34064): 0. Apply patch 1. vi ./installer/data/mysql/kohastructure.sql 2. Comment out some columns, change NULL status, or whatever you like 3. perl misc/maintenance/audit_database.pl \ --filename /kohadevbox/koha/installer/data/mysql/kohastructure.sql 4. Note that the output includes SQL commands to change the database to match the new kohastructure.sql 5a. Try using koha-foreach and note that the database name appears above the database comparison 5b. koha-foreach "perl misc/maintenance/audit_database.pl \ --filename /kohadevbox/koha/installer/data/mysql/kohastructure.sql" Test plan #2: 1. After doing test plan #1, access the new 'database audit' tab, visit: <staff_url>/cgi-bin/koha/about.pl?tab=database 2. Ensure the output is the same as test plan #1 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: JesseM <jesse@bywatersolutions.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=36039 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169226|0 |1 is obsolete| | --- Comment #28 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 174404 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174404&action=edit Bug 36039: New 'datatabase audit' tab in about page Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: JesseM <jesse@bywatersolutions.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=36039 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169227|0 |1 is obsolete| | --- Comment #29 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 174405 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174405&action=edit Bug 36039: (follow-up) Move view logic to script/template This patch updates the module as suggested by David to handle just the diff code and leaves the display logic up to the templates and script. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: JesseM <jesse@bywatersolutions.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=36039 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Passed QA Depends on|36027 | CC| |pedro.amorim@ptfs-europe.co | |m --- Comment #30 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- I'm removing the dependency on bug 36027 here. And putting this back to PQA. Test plan still works and patches apply after a minor rebase on a conflict caused by bug 37260. This bug and bug 36027 don't relate to each other (audit_database.pl != search_for_data_inconsistencies.pl), and I believe the dependency was possibly added with plans to move bug 36027 logic into Koha::Database::Auditor. But can be it's own follow-up bug, of that bug. It should not block this. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36027 [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=36039 --- Comment #31 from David Cook <dcook@prosentient.com.au> --- (In reply to Pedro Amorim from comment #30)
I'm removing the dependency on bug 36027 here. And putting this back to PQA. Test plan still works and patches apply after a minor rebase on a conflict caused by bug 37260.
This bug and bug 36027 don't relate to each other (audit_database.pl != search_for_data_inconsistencies.pl), and I believe the dependency was possibly added with plans to move bug 36027 logic into Koha::Database::Auditor. But can be it's own follow-up bug, of that bug. It should not block this.
Sounds reasonable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed Status|Passed QA |Failed QA --- Comment #32 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am hesitant to push this as is, as this will expose some useful, but also dangerous information to a lot more eyes. I'd like to work out some better display for the errors. This is unlikely to conflict, so I think we can take a little extra time to fix it up before push. 1) This includes some untranslatable strings that should be handled in the template: + print "Parsing schema for database '$database_name'\n" if $self->{is_cli}; + print "Parsing schema for file $self->{filename} \n" if $self->{is_cli}; We can move this to a separate bug if that helps. 2) Make the message easier to understand for people without code knowledge. + <h3>WARNING!!!</h3> + <p> + These commands are only suggestions! They are not a replacement for updatedatabase.pl!<br /> + Review the database, updatedatabase.pl, and kohastructure.sql before making any changes!<br /> + </p> We are referring to updatedatabase.pl here, but in package installations you will never encounter this script even as an update. Should we make the note more general maybe to make it also easier to understand? Suggestion: <p> These commands are only suggestions. There are not a replacement for the database update scripts run during installations and updates </p> <p> Review the database, the atomic update files and the table definitions in kohastructure.sql before making any changes.<p> I think better to create a visual warn with formatting than with many !. I'd also reduce to Warning! but maybe make it bold instead. <p>Following SQL statements might help in fixing your database. Review before use.</p> 3) Success message -- Convert schema '' to '':; -- No differences found; This doesn't look nicely formatted and also might be a bit confusing to some. Could we show some nice "All is well" type of note instead or at least remove the first line? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 --- Comment #33 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 175933 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=175933&action=edit Bug 36039: (QA follow-up): Address i18n for print messages -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 --- Comment #34 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 175934 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=175934&action=edit Bug 36039: (QA follow-up): Improve audit messages Improve messages clarity. Show success message if no schema diff was found from audit. Code refactor to ensure the title+message shown in UI is the same as the one shown in CLI and are all translatable. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 --- Comment #35 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Thanks for the follow-ups! Fixed before push: FAIL koha-tmpl/intranet-tmpl/prog/en/modules/about.tt FAIL filters missing_filter at line 806 ( <h3><strong>[% db_audit.title %]</strong></h3>) missing_filter at line 807 ( <p><pre>[% db_audit.message %]</pre><p>) I see you took me literally on the "All is well" :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |25.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 --- Comment #36 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 25.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|25.05.00 |25.05.00,24.11.03 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 --- Comment #37 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 24.11.x for 24.11.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #38 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 24.05.x unless requested -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |40370 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40370 [Bug 40370] about.pl should NOT say "Run the following SQL to fix the database" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 --- Comment #39 from David Cook <dcook@prosentient.com.au> --- Looks like I dropped the ball here a bit. The about.pl page should not say "Run the following SQL to fix the database:". It looks like it's confused people who have gone on to ignore the warning of "Warning! These commands are only suggestions." I've raised bug 40370 to fix that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 --- Comment #40 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Needs documenting -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36039 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42469 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42469 [Bug 42469] Database audit - should be an option to run, instead of running automatically -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org