Bug 9009 - Add separate user js and css for SCO module
Summary: Add separate user js and css for SCO module
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Self checkout (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-05 16:57 UTC by Kyle M Hall
Modified: 2013-05-23 06:24 UTC (History)
3 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 9009 - Add separate user js and css of SCO module (4.70 KB, patch)
2012-11-05 17:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9009 - Add separate user js and css of SCO module (6.02 KB, patch)
2012-11-05 17:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9009 - Add separate user js and css of SCO module (7.06 KB, patch)
2012-11-05 18:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9009 - Add separate user js and css of SCO module (7.04 KB, patch)
2012-11-05 18:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9009 - Add separate user js and css for SCO module (7.04 KB, patch)
2012-11-05 18:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9009 - Add separate user js and css for SCO module (7.04 KB, patch)
2012-12-04 13:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9009 - Add separate user js and css for SCO module (7.13 KB, patch)
2012-12-04 20:41 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9009 - Add separate user js and css for SCO module (7.63 KB, patch)
2012-12-04 21:45 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 9009 - Add separate user js and css for SCO module (7.57 KB, patch)
2012-12-27 10:19 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 Kyle M Hall 2012-11-05 16:57:03 UTC
Right now, the system preferences OPACUserCSS and opacuserjs affect the SCO
system. Some libraries would like to add css/js that specifically for the SCO module, but it would negatively impact the OPAC.
Comment 1 Kyle M Hall 2012-11-05 17:22:29 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2012-11-05 17:26:34 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2012-11-05 18:15:46 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2012-11-05 18:18:55 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2012-11-05 18:20:27 UTC
Test plan:
1) Apply patch
2) Run updatedatabase.pl
3) Add something arbitrary to the new sysprefs SCOUserCSS and SCOUserJS ( such as TestCSS and TestJS ).
4) Load the SCO module in a browser, and view the HTML, verify the CSS and JS values in the system preferences have been included.
Comment 6 Kyle M Hall 2012-11-05 18:22:27 UTC Comment hidden (obsolete)
Comment 7 Marc Véron 2012-12-03 21:37:47 UTC
Small but useful enhancement!

However, patch does not apply, sorry:

Applying: Bug 9009 - Add separate user js and css for SCO module
error: patch failed: installer/data/mysql/sysprefs.sql:386
error: installer/data/mysql/sysprefs.sql: patch does not apply
error: patch failed: installer/data/mysql/updatedatabase.pl:6020
error: installer/data/mysql/updatedatabase.pl: patch does not apply
error: patch failed: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:454
error: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref: patch does not apply
Comment 8 Kyle M Hall 2012-12-04 13:22:01 UTC Comment hidden (obsolete)
Comment 9 Marc Véron 2012-12-04 20:41:15 UTC Comment hidden (obsolete)
Comment 10 Marc Véron 2012-12-04 20:43:52 UTC
Addendum:
I tested with an alert('hello') as JavaScript and some background-color for the CSS. Worked as expected.
Comment 11 Owen Leonard 2012-12-04 21:31:35 UTC
I think it is a problem that this patch doesn't include a commit message describing what it does, and doesn't include a test plan (even though the test plan is in a comment).
Comment 12 Marc Véron 2012-12-04 21:45:38 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2012-12-05 12:46:51 UTC
I always include my test plan in a comment so we don't have them mucking up the git log. I also didn't include a commit message because I thought the title was self explanatory.

(In reply to comment #11)
> I think it is a problem that this patch doesn't include a commit message
> describing what it does, and doesn't include a test plan (even though the
> test plan is in a comment).
Comment 14 Jared Camins-Esakov 2012-12-05 13:32:07 UTC
(In reply to comment #13)
> I always include my test plan in a comment so we don't have them mucking up
> the git log. I also didn't include a commit message because I thought the
> title was self explanatory.

If the test plan on the bug is clearly labelled "test plan:" or "to test:" I will probably use it. However, including the test plan in the commit message does increase the chances of the patch getting pushed to Master quickly. Marc's commit message is absolutely perfect, because it tells me in one second exactly what the patch does (adds two sysprefs), how to test it, and what tests Marc performed.
Comment 15 Kyle M Hall 2012-12-05 14:22:10 UTC
> If the test plan on the bug is clearly labelled "test plan:" or "to test:" I
> will probably use it. However, including the test plan in the commit message
> does increase the chances of the patch getting pushed to Master quickly.
> Marc's commit message is absolutely perfect, because it tells me in one
> second exactly what the patch does (adds two sysprefs), how to test it, and
> what tests Marc performed.

That's good to know. I'll follow that procedure from now on.
Comment 16 Jonathan Druart 2012-12-27 10:19:29 UTC
QA Comments:

The patch is clean and it works as expected (tested some js and css).

Marked as Passed QA.
Comment 17 Jonathan Druart 2012-12-27 10:19:39 UTC
Created attachment 14288 [details] [review]
Bug 9009 - Add separate user js and css for SCO module

Add system preferences SCOUserCSS and SCOUserJS to define separate CSS and JavaScript for the Self Checkout Module.

Test plan:
1) Apply patch
2) Run updatedatabase.pl
3) Add something arbitrary to the new sysprefs SCOUserCSS and SCOUserJS ( such as TestCSS and TestJS ).
4) Load the SCO module in a browser, and view the HTML, verify the CSS and JS values in the system preferences have been included.

Signed-off-by: Marc Veron <veron@veron.ch>

I tested with an alert('hello') as JavaScript and some background-color for the CSS. Worked as expected.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 18 Jared Camins-Esakov 2012-12-27 15:06:21 UTC
This patch has been pushed to master.