Summary: | Add a way to automatically convert SQL reports | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Reports | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | abl, josef.moravec, kyle, mtj, paul.poulain, tomascohen, veron |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: |
Bug 17196 moved the column marcxml out of the biblioitems table and into a separate one.
That will break any SQL reports using marcxml, but in order to make it easy to fix them, a new column with a warning has been added to the Saved reports page (/reports/guided_reports.pl?phase=Use saved). There is also an update link that will help to modify the SQL query.
|
|
Version(s) released in: | Circulation function: | ||
Bug Depends on: | 17196 | ||
Bug Blocks: | 39082, 18642, 18667, 19495, 19638 | ||
Attachments: |
Bug 17898: Automagically convert SQL reports
Bug 17898: Followup - udpate templates for bootstrap 3 Bug 17898: Automagically convert SQL reports Bug 17898: Followup - udpate templates for bootstrap 3 Bug 17898: Automagically convert SQL reports Bug 17898: Followup - udpate templates for bootstrap 3 |
Description
Jonathan Druart
2017-01-13 14:52:17 UTC
Created attachment 58955 [details] [review] Bug 17898: Automagically convert SQL reports Bug 17196 move the marcxml out of the biblioitems table. That will break SQL reports using it. It would be handy to propose an automagically way to convert the SQL reports. We do not want to update the reports automatically without user inputs, it will be too hasardous. However we can lead the user to convert them. In this patchset I suggest to warn the user if a report is subject to be updated. TODO: Add a way to mark this job done (using a pref?) to remove the check and not to display false positives. Test plan: - Create some SQL reports (see https://wiki.koha-community.org/wiki/SQL_Reports_Library) - Go on the report list page (/reports/guided_reports.pl?phase=Use saved) - For the reports using biblioitems.marcxml you will see a new column warning you that it is obsolete - Click on update link => that will open a modal with the converted SQL query - Click on the update button => you will be informed that the query has been updated If all the reports are updated, the new column "Update" will no longer be displayed. Note that this is not polish, but not needed to as it won't be something used on a daily basis. If someone wants, feel free :) Created attachment 58961 [details] [review] Bug 17898: Followup - udpate templates for bootstrap 3 As of bug 16239 is pushed, the templates should use bootstrap 3. Also, I tried to update one of my custom reports, but it looks like only first occurence of marcxml was changed... Not sure it is possible automatically, but that would be awesome to also add a "WHERE format='marcxml' and marcflavour=$syspref->{marcflavour}" in the query. That would be needed in the long term anyway. wonderful job btw ! I've been trying to make up my mind before commenting. My conclusion is that this is a really hard task to achieve, and thus the current behaviour (i.e. propose a translated SQL, no magic stuff) is the way to go. I'd put it in a modal, and probably add a legend stating that they should contact someone who knows SQL to check before overwriting the old report (or the list). This is an example that won't work with this simplistic algorithm, but we could go forward without fixing it anyway, as reports are now broken for everyone anyway: $sql = q| SELECT biblionumber, marcxml, biblioitems.issn AS issn FROM biblioitems INNER JOIN biblio USING (biblionumber) WHERE biblionumber = 14|; $expected_converted_sql = q| SELECT biblionumber, metadata, biblioitems.issn AS issn FROM biblioitems INNER JOIN biblio USING (biblionumber) INNER JOIN biblio_metadata USING (biblionumber) WHERE biblionumber = 14|; so the algorithm works basically only if the query uses marcxml exclusively. (In reply to Tomás Cohen Arazi from comment #7) > I've been trying to make up my mind before commenting. My conclusion is that > this is a really hard task to achieve, and thus the current behaviour (i.e. > propose a translated SQL, no magic stuff) is the way to go. I'd put it in a > modal, and probably add a legend stating that they should contact someone > who knows SQL to check before overwriting the old report (or the list). > > This is an example that won't work with this simplistic algorithm, but we > could go forward without fixing it anyway, as reports are now broken for > everyone anyway: > > $sql = q| > SELECT biblionumber, > marcxml, > biblioitems.issn AS issn > FROM biblioitems > INNER JOIN biblio USING (biblionumber) > WHERE biblionumber = 14|; > > $expected_converted_sql = q| > SELECT biblionumber, > metadata, > biblioitems.issn AS issn > FROM biblioitems > INNER JOIN biblio USING (biblionumber) > INNER JOIN biblio_metadata USING (biblionumber) > WHERE biblionumber = 14|; > > so the algorithm works basically only if the query uses marcxml exclusively. I'm sorry for the first part of my comment, it is already a modal, and has an 'Update' button. I just messed up with the rebase. I'd still add a note. Created attachment 63582 [details] [review] Bug 17898: Automagically convert SQL reports Bug 17196 move the marcxml out of the biblioitems table. That will break SQL reports using it. It would be handy to propose an automagically way to convert the SQL reports. We do not want to update the reports automatically without user inputs, it will be too hasardous. However we can lead the user to convert them. In this patchset I suggest to warn the user if a report is subject to be updated. TODO: Add a way to mark this job done (using a pref?) to remove the check and not to display false positives. Test plan: - Create some SQL reports (see https://wiki.koha-community.org/wiki/SQL_Reports_Library) - Go on the report list page (/reports/guided_reports.pl?phase=Use saved) - For the reports using biblioitems.marcxml you will see a new column warning you that it is obsolete - Click on update link => that will open a modal with the converted SQL query - Click on the update button => you will be informed that the query has been updated If all the reports are updated, the new column "Update" will no longer be displayed. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 63583 [details] [review] Bug 17898: Followup - udpate templates for bootstrap 3 Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Looks like this is in master now? (In reply to Katrin Fischer from comment #11) > Looks like this is in master now? Yes, and leads to Bug 18642 Created attachment 63613 [details] [review] Bug 17898: Automagically convert SQL reports Bug 17196 move the marcxml out of the biblioitems table. That will break SQL reports using it. It would be handy to propose an automagically way to convert the SQL reports. We do not want to update the reports automatically without user inputs, it will be too hasardous. However we can lead the user to convert them. In this patchset I suggest to warn the user if a report is subject to be updated. TODO: Add a way to mark this job done (using a pref?) to remove the check and not to display false positives. Test plan: - Create some SQL reports (see https://wiki.koha-community.org/wiki/SQL_Reports_Library) - Go on the report list page (/reports/guided_reports.pl?phase=Use saved) - For the reports using biblioitems.marcxml you will see a new column warning you that it is obsolete - Click on update link => that will open a modal with the converted SQL query - Click on the update button => you will be informed that the query has been updated If all the reports are updated, the new column "Update" will no longer be displayed. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 63614 [details] [review] Bug 17898: Followup - udpate templates for bootstrap 3 Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Pushed to master for 17.05, thanks Jonathan, Josef! Depends on bug 17196 which is not part of 16.11.x. Hm, was too early to close - still open bug reports linked to this. |