Bug 18667 - Show a diff view of SQL reports when converting
Summary: Show a diff view of SQL reports when converting
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords: Academy
Depends on: 17898
Blocks:
  Show dependency treegraph
 
Reported: 2017-05-24 17:48 UTC by Owen Leonard
Modified: 2018-06-04 20:10 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 18667 - Show a diff view of SQL reports when converting (4.87 KB, patch)
2017-05-24 18:04 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 18667 [Revised] Show a diff view of SQL reports when converting (4.90 KB, patch)
2017-05-25 12:37 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 18667 [Revised] Show a diff view of SQL reports when converting (4.96 KB, patch)
2017-07-06 13:51 UTC, Jesse Maseto
Details | Diff | Splinter Review
Bug 18667: Show a diff view of SQL reports when converting (5.03 KB, patch)
2017-07-13 19:39 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2017-05-24 17:48:44 UTC
I think it would be helpful to use the same diff JavaScript library to compare SQL versions.
Comment 1 Owen Leonard 2017-05-24 18:04:04 UTC
Created attachment 63708 [details] [review]
Bug 18667 - Show a diff view of SQL reports when converting

This patch reformats the SQL conversion before-and-after view so that it
shows a diff between the two, just as the Manage MARC Import interface
does.

This patch also makes a correction to the JavaScript which is triggered
by the closing of the modal. In Bootstrap 3 "hidden.bs.modal" is the
event name instead of "hidden."

To test, apply the patch and find an SQL report in your reports library
which requires updating.

- Click the "Update SQL" button.
- In the modal window, the "existing" and "updated" SQL should be shown
  side-by-side with colored highlighting of the differece.
- Confirm that the "Update" button still works correctly.
- Also confirm that if you open the preview of another report, the
  "Close" button hides the modal and updates the contents of the modal
  markup. Use your browser's DOM inspector to confirm that <div
  class="modal-body"> contains only <div id="loading"> and its contents.
Comment 2 Josef Moravec 2017-05-25 06:02:42 UTC
Comment on attachment 63708 [details] [review]
Bug 18667 - Show a diff view of SQL reports when converting

Review of attachment 63708 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/reports/convert_report.tt
@@ +7,4 @@
>              [% IF msg == 'no_report' %]
>                  There is no valid report for this id.
>              [% ELSIF msg == 'can_be_updated' %]
> +                <div id="col1" class="" style="width:45%;float:left;">

It would be better to use class instead of style attribute

@@ +11,5 @@
> +                    <h3>Existing SQL</h3>
> +                    <span class="show_sql">[% current_sql | html_entity %]</span>
> +                </div>
> +
> +                <div id="col2" class="" style="width:45%; float:left;">

And here too please
Comment 3 Josef Moravec 2017-05-25 06:03:23 UTC
But otherwise it's working as expected
Comment 4 Owen Leonard 2017-05-25 12:37:46 UTC
Created attachment 63712 [details] [review]
Bug 18667 [Revised] Show a diff view of SQL reports when converting

This patch reformats the SQL conversion before-and-after view so that it
shows a diff between the two, just as the Manage MARC Import interface
does.

This patch also makes a correction to the JavaScript which is triggered
by the closing of the modal. In Bootstrap 3 "hidden.bs.modal" is the
event name instead of "hidden."

To test, apply the patch and find an SQL report in your reports library
which requires updating.

- Click the "Update SQL" button.
- In the modal window, the "existing" and "updated" SQL should be shown
  side-by-side with colored highlighting of the differece.
- Confirm that the "Update" button still works correctly.
- Also confirm that if you open the preview of another report, the
  "Close" button hides the modal and updates the contents of the modal
  markup. Use your browser's DOM inspector to confirm that <div
  class="modal-body"> contains only <div id="loading"> and its contents.

Revision moves inline style attributes to CSS block.
Comment 5 Jesse Maseto 2017-07-06 13:51:23 UTC
Created attachment 64849 [details] [review]
Bug 18667 [Revised] Show a diff view of SQL reports when converting

This patch reformats the SQL conversion before-and-after view so that it
shows a diff between the two, just as the Manage MARC Import interface
does.

This patch also makes a correction to the JavaScript which is triggered
by the closing of the modal. In Bootstrap 3 "hidden.bs.modal" is the
event name instead of "hidden."

To test, apply the patch and find an SQL report in your reports library
which requires updating.

- Click the "Update SQL" button.
- In the modal window, the "existing" and "updated" SQL should be shown
  side-by-side with colored highlighting of the differece.
- Confirm that the "Update" button still works correctly.
- Also confirm that if you open the preview of another report, the
  "Close" button hides the modal and updates the contents of the modal
  markup. Use your browser's DOM inspector to confirm that <div
  class="modal-body"> contains only <div id="loading"> and its contents.

Revision moves inline style attributes to CSS block.

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Comment 6 Jonathan Druart 2017-07-13 19:39:35 UTC
Created attachment 65023 [details] [review]
Bug 18667: Show a diff view of SQL reports when converting

This patch reformats the SQL conversion before-and-after view so that it
shows a diff between the two, just as the Manage MARC Import interface
does.

This patch also makes a correction to the JavaScript which is triggered
by the closing of the modal. In Bootstrap 3 "hidden.bs.modal" is the
event name instead of "hidden."

To test, apply the patch and find an SQL report in your reports library
which requires updating.

- Click the "Update SQL" button.
- In the modal window, the "existing" and "updated" SQL should be shown
  side-by-side with colored highlighting of the differece.
- Confirm that the "Update" button still works correctly.
- Also confirm that if you open the preview of another report, the
  "Close" button hides the modal and updates the contents of the modal
  markup. Use your browser's DOM inspector to confirm that <div
  class="modal-body"> contains only <div id="loading"> and its contents.

Revision moves inline style attributes to CSS block.

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Jonathan Druart 2017-07-13 21:42:22 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 8 Fridolin Somers 2017-07-18 12:30:40 UTC
Enhancement not pushed to 17.05.x