Bug 17840 - Add classes to internal and public notes in checkouts table
Summary: Add classes to internal and public notes in checkouts table
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low trivial (vote)
Assignee: Emma
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2017-01-03 21:54 UTC by Katrin Fischer
Modified: 2018-06-04 20:10 UTC (History)
6 users (show)

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


Attachments
bug 17840: adding classes to notes in checkouts Test Plan 1) Add an internal and a public note to an item. 2) Check out the item. 3) Verify that both notes are shown in between title and barcode in the second column of the details table. 4) Verify there i (1.78 KB, patch)
2017-01-17 03:26 UTC, Emma
Details | Diff | Splinter Review
bug 17840: adding classes to notes in checkouts (1.77 KB, patch)
2017-01-17 03:27 UTC, Emma
Details | Diff | Splinter Review
bug 17840: adding classes to notes in checkouts (1.95 KB, patch)
2017-01-18 10:43 UTC, Claire Gravely
Details | Diff | Splinter Review
bug 17840: adding classes to notes in checkouts (1.95 KB, patch)
2017-01-19 20:44 UTC, Emma
Details | Diff | Splinter Review
bug 17840: adding classes to notes in checkouts (2.15 KB, patch)
2017-01-19 21:20 UTC, Emma
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17840: adding classes to notes in checkouts (2.21 KB, patch)
2017-01-24 14:44 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17840: adding classes to notes in checkouts (2.27 KB, patch)
2017-02-23 16:16 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 Katrin Fischer 2017-01-03 21:54:36 UTC
In order to be able to style the internal and public notes so they stand out more, it would be good if each of them had their own class. At the moment, there seems something not quite right, as both have an empty class="".

To test:

1) Add an internal and a public note to an item.
2) Check out the item.
3) Verify that both notes are shown in between title and barcode in the second column of the details table.
4) Verify there is an empty class="" in the HTML source code for each
Comment 1 Emma 2017-01-17 03:26:12 UTC Comment hidden (obsolete)
Comment 2 Emma 2017-01-17 03:27:26 UTC Comment hidden (obsolete)
Comment 3 Claire Gravely 2017-01-18 10:43:32 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2017-01-19 12:05:54 UTC
There is a test just after, if true the "circ-hlt" class name will be used.
I guess we want the 2 new classes in any cases.

Marked as Failed QA.
Comment 5 Jonathan Druart 2017-01-19 12:06:32 UTC
Emma, you should also change the author's name of this patch.
Comment 6 Emma 2017-01-19 20:44:05 UTC Comment hidden (obsolete)
Comment 7 Emma 2017-01-19 21:20:15 UTC
Created attachment 59286 [details] [review]
bug 17840: adding classes to notes in checkouts

Test Plan
1) Add an internal and a public note to an item.
2) Check out the item.
3) Verify that both notes are shown in between title and barcode in the
second column of the details table.
4) Verify there is an empty class="" in the HTML source code for each
5) Apply patch and refresh page
6) View page source, confirm that class has been changed

I tested this by first applying the patch for bug 17919 to catch the check-in software error.
Patch working as expected :)
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Comment 8 Owen Leonard 2017-01-24 14:44:11 UTC
Created attachment 59503 [details] [review]
[SIGNED-OFF] Bug 17840: adding classes to notes in checkouts

Test Plan
1) Add an internal and a public note to an item.
2) Check out the item.
3) Verify that both notes are shown in between title and barcode in the
second column of the details table.
4) Verify there is an empty class="" in the HTML source code for each
5) Apply patch and refresh page
6) View page source, confirm that class has been changed

I tested this by first applying the patch for bug 17919 to catch the
check-in software error. Patch working as expected :)

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 9 Jonathan Druart 2017-01-26 10:32:03 UTC
But now the circ-hlt class name are removed.
No idea if we must keep it or not.
Any opinions?
Comment 10 Jonathan Druart 2017-02-02 08:52:30 UTC
Chris, do you know if this circ-hlt class is still useful?
Comment 11 Chris Cormack 2017-02-16 18:52:55 UTC
Nope, they can safely be removed
Comment 12 Jonathan Druart 2017-02-23 16:16:36 UTC
Created attachment 60619 [details] [review]
Bug 17840: adding classes to notes in checkouts

Test Plan
1) Add an internal and a public note to an item.
2) Check out the item.
3) Verify that both notes are shown in between title and barcode in the
second column of the details table.
4) Verify there is an empty class="" in the HTML source code for each
5) Apply patch and refresh page
6) View page source, confirm that class has been changed

I tested this by first applying the patch for bug 17919 to catch the
check-in software error. Patch working as expected :)

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Kyle M Hall 2017-03-03 17:56:55 UTC
Pushed to master for 17.05, thanks Emma!
Comment 14 Katrin Fischer 2017-03-12 22:56:54 UTC
This is marked enh, but as the classes being empty before can be considered a bug and the change is very small, I have decided to push this small one.

This patch has been pushed to 16.11.x and will be in 16.11.05.
Comment 15 Mason James 2017-04-03 14:40:12 UTC
Pushed to 16.05.x, for 16.05.11 release