Bug 17698 - Make patron notes show up on staff dashboard
Summary: Make patron notes show up on staff dashboard
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Aleisha Amohia
QA Contact: Jonathan Druart
URL:
Whiteboard:
Keywords: release-notes-needed
Depends on: 14224
Blocks: 19772
  Show dependency treegraph
 
Reported: 2016-11-30 02:28 UTC by Aleisha Amohia
Modified: 2018-08-03 13:35 UTC (History)
14 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
[NOT READY FOR TESTING] Bug 17698: Make patron notes show up on staff dashboard (11.38 KB, patch)
2016-12-01 00:55 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (23.46 KB, patch)
2017-01-10 21:29 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (24.58 KB, patch)
2017-01-10 21:35 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (23.32 KB, patch)
2017-01-10 21:50 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (23.98 KB, patch)
2017-01-24 03:07 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (24.57 KB, patch)
2017-01-29 21:52 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (24.59 KB, patch)
2017-02-02 03:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17698: Make patron notes show up on staff dashboard (24.66 KB, patch)
2017-02-02 10:00 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (24.59 KB, patch)
2017-02-02 12:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (24.96 KB, patch)
2017-02-02 21:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (24.96 KB, patch)
2017-02-02 21:42 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (24.97 KB, patch)
2017-02-06 22:03 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (24.45 KB, patch)
2017-04-04 21:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (24.47 KB, patch)
2017-04-18 22:38 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (24.64 KB, patch)
2017-04-19 04:39 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Changing to Koha Objects style, adding circ sidebar (16.56 KB, patch)
2017-06-26 23:35 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Reverting back to original resultset usage (1.03 KB, patch)
2017-06-27 21:56 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Koha Objects, access borrower through patron sub (1.78 KB, patch)
2017-07-06 01:30 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (25.29 KB, patch)
2018-02-01 00:26 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Changing to Koha Objects style, adding circ sidebar (16.21 KB, patch)
2018-02-01 21:48 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Koha Objects, access borrower through patron sub (3.26 KB, patch)
2018-02-01 21:56 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (25.33 KB, patch)
2018-03-14 15:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Changing to Koha Objects style, adding circ sidebar (16.28 KB, patch)
2018-03-14 15:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Koha Objects, access borrower through patron sub (3.26 KB, patch)
2018-03-14 15:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (25.39 KB, patch)
2018-03-16 09:57 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Changing to Koha Objects style, adding circ sidebar (16.34 KB, patch)
2018-03-16 09:57 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Koha Objects, access borrower through patron sub (3.32 KB, patch)
2018-03-16 09:57 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (25.53 KB, patch)
2018-04-09 03:23 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Changing to Koha Objects style, adding circ sidebar (16.34 KB, patch)
2018-04-09 03:23 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Koha Objects, access borrower through patron sub (3.32 KB, patch)
2018-04-09 03:23 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Fixing some small issues (12.67 KB, patch)
2018-04-12 04:26 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: (QA follow-up) Changes to use Asset TT plugin (bug 20538) (1.48 KB, patch)
2018-04-15 12:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (25.71 KB, patch)
2018-06-05 23:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Changing to Koha Objects style, adding circ sidebar (16.34 KB, patch)
2018-06-05 23:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Koha Objects, access borrower through patron sub (3.32 KB, patch)
2018-06-05 23:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Fixing some small issues (12.67 KB, patch)
2018-06-05 23:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: (QA follow-up) Changes to use Asset TT plugin (bug 20538) (1.48 KB, patch)
2018-06-05 23:05 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Rebase and schema update (4.05 KB, patch)
2018-06-05 23:20 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: (QA follow-up) Fix minors design issues (12.71 KB, patch)
2018-06-12 20:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17698: [FOLLOW-UP] Fixing things in Comment 57 (8.08 KB, patch)
2018-06-12 23:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17698: Make patron notes show up on staff dashboard (25.65 KB, patch)
2018-06-13 15:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17698: (follow-up) Changing to Koha Objects style, adding circ sidebar (16.49 KB, patch)
2018-06-13 15:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17698: (follow-up) Koha Objects, access borrower through patron sub (3.41 KB, patch)
2018-06-13 15:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17698: (follow-up) Fixing some small issues (11.72 KB, patch)
2018-06-13 15:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17698: (QA follow-up) Changes to use Asset TT plugin (bug 20538) (1.56 KB, patch)
2018-06-13 15:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17698: (follow-up) Rebase and schema update (4.15 KB, patch)
2018-06-13 15:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17698: (QA follow-up) Fix minors design issues (11.88 KB, patch)
2018-06-13 15:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17698: (follow-up) Fixing things in Comment 57 (8.17 KB, patch)
2018-06-13 15:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17698: (QA follow-up) few more things (3.47 KB, patch)
2018-06-13 15:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17698: Do not send pending_checkout_notes from all circ scripts (16.20 KB, patch)
2018-06-13 15:24 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 Aleisha Amohia 2016-11-30 02:28:59 UTC
Bug 14224 adds issue notes, where users can add notes to issues on the OPAC side and a notice is generated and sent to the library. Other than the notice, librarians cannot see the issue notes until the item is checked in. This bug will show issue notes on the staff side (similar to how purchase suggestions and patron detail modifications show up on mainpage.pl), where librarians can view issue notes and mark them as 'seen'.
Comment 1 Aleisha Amohia 2016-12-01 00:55:36 UTC Comment hidden (obsolete)
Comment 2 Aleisha Amohia 2017-01-10 21:29:48 UTC Comment hidden (obsolete)
Comment 3 Aleisha Amohia 2017-01-10 21:35:12 UTC Comment hidden (obsolete)
Comment 4 Aleisha Amohia 2017-01-10 21:50:37 UTC Comment hidden (obsolete)
Comment 5 Aleisha Amohia 2017-01-24 03:07:18 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2017-01-24 14:27:04 UTC
My only comment about required fixes is that "Issue notes" should be "Checkout notes."

Suggestions:

 - In the table of notes I think it makes more sense to have:

   Title | Note | Date | Set by | Status | Actions

   I think it makes more sense to be able to scan the list by
   title than by note content. And I think it makes sense for
   the status to be next to the actions.

- A nice improvement would be to have the batch action buttons at the bottom be disabled until one or more checkboxes are checked. See Audio alerts for an example.

Marking as Failed QA just for the language fix.
Comment 7 Aleisha Amohia 2017-01-29 21:52:20 UTC
Created attachment 59638 [details] [review]
Bug 17698: Make patron notes show up on staff dashboard

This patch adds a user permission for managing issue notes, and a 'noteseen'
column to the issues table.

To test:
1) Apply Bug 14224 first, update database, rebuild schema.
2) Apply this patch, update database, rebuild schema.
3) Restart koha-common and memcached
4) Turn on AllowIssueNotes syspref if haven't already
5) Issue two items to two different users (one item each)
6) Log into the OPAC as one of the users and add an issue note to their
issue
7) Log out and log back into the OPAC as the other user
8) Disable Javascript
9) Refresh opac-user.pl
10) Leave an issue note on their issue
11) Enable javascript and log into the Staff Client as a superlibrarian
user
12) Go to your user's account and edit their permissions to have
everything ticked EXCEPT circulate->manage issue notes.
13) Go to main intranet page. There should be no message saying
'checkout notes pending'.
14) Go to circulation home page. There should be no link to Checkout notes.
15) Go back to user's permissions and tick circulate->manage issue
notes.
16) Go back to main intranet page. There should now be a message at the
bottom saying 'Checkout notes pending: 2'
17) Go to circulation home page. There should be a link to Checkout notes
with a 2 next to it. Click this link
18) Attempt to mark an issue note as seen. This should update the status
of the issue note to 'seen' and disable to 'mark as seen' button while
enabling the 'mark as not seen' button.
19) Test both buttons with both issues.
20) Test select all and clear all buttons
21) Confirm that buttons at the bottom are only enabled if a checkbox is
checked
22) Try selecting both issues and using the buttons at the bottom to
mark multiple issue notes at once.
23) Confirm the barcode link to the item works as expected.
24) Confirm the cardnumber link to the user works as expected.
25) Confirm all table details show correctly.

Sponsored-by: Catalyst IT
Comment 8 Josef Moravec 2017-01-30 10:22:14 UTC
Koha QA tools complains: 
 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/checkout_notes/checkout-notes.tt
   FAIL	  forbidden patterns
		forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 104)

There are some references to non-existing page issue notes see git grep "issue-notes.pl"

And bad terminology: "Back to issue notes", line 104 of checkout-notes.tt
Comment 9 Aleisha Amohia 2017-02-02 03:09:21 UTC Comment hidden (obsolete)
Comment 10 Josef Moravec 2017-02-02 10:00:25 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2017-02-02 12:10:45 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2017-02-02 12:14:31 UTC
Aleisha,
I have rebased your patch on top of the ones I have submitted on bug 14224.
You should take a look to apply some changes, for instance:
1/ Return a to_json
2/ Check the existence of the DB columns before inserting them
3/ Use "AFTER" in the ADD column sql query (to enforce fields order)
4/ I do not think checkout_notes should be a new module. I'd say it should go to circ
5/ Make sure the strings are translatable
Comment 13 Jonathan Druart 2017-02-02 12:26:37 UTC
Something important: In svc/issue_notes you are not testing that the logged in user is allowed to call this script.
Comment 14 Aleisha Amohia 2017-02-02 21:40:41 UTC Comment hidden (obsolete)
Comment 15 Aleisha Amohia 2017-02-02 21:42:48 UTC Comment hidden (obsolete)
Comment 16 Aleisha Amohia 2017-02-02 21:43:41 UTC
(In reply to Jonathan Druart from comment #12)
> Aleisha,
> I have rebased your patch on top of the ones I have submitted on bug 14224.
> You should take a look to apply some changes, for instance:
> 1/ Return a to_json
> 2/ Check the existence of the DB columns before inserting them
> 3/ Use "AFTER" in the ADD column sql query (to enforce fields order)
> 4/ I do not think checkout_notes should be a new module. I'd say it should
> go to circ
> 5/ Make sure the strings are translatable

(In reply to Jonathan Druart from comment #13)
> Something important: In svc/issue_notes you are not testing that the logged
> in user is allowed to call this script.

I have applied all of these changes and changed all instances (I think) of 'issue note' to 'checkout note'. This is ready to sign off
Comment 17 Josef Moravec 2017-02-03 07:10:29 UTC
Please update the POD in svc/checkout_notes

29 	=head1 NAME
30 	
31 	svc/issue_notes - Web service for managing patron notes set on issues
Comment 18 Aleisha Amohia 2017-02-06 22:03:54 UTC Comment hidden (obsolete)
Comment 19 Aleisha Amohia 2017-04-04 21:09:26 UTC Comment hidden (obsolete)
Comment 20 Marc Véron 2017-04-17 08:55:20 UTC
SQL error in bug-17698_add-noteseen-column-to-issues.sql

$dbh->do(q|ALTER TABLE issues ADD noteseen int(1) default NULL AFTER notedate|);
and 
$dbh->do(q|ALTER TABLE old_issues ADD noteseen int(1) default NULL AFTER notedate|);

ADD schould be ADD COLUMN
Comment 21 Aleisha Amohia 2017-04-18 22:38:11 UTC Comment hidden (obsolete)
Comment 22 Marc Véron 2017-04-19 04:39:06 UTC Comment hidden (obsolete)
Comment 23 Marcel de Rooy 2017-06-20 13:44:41 UTC
my @notes = $schema->resultset('Issue')->search({ 'me.note' => { '!=', undef } }, { prefetch => [ 'borrower', { item => 'biblionumber' } ] });

Any chance to put this statement in Koha Object[s] style too? Thanks.
Comment 24 Aleisha Amohia 2017-06-26 23:35:14 UTC Comment hidden (obsolete)
Comment 25 Marcel de Rooy 2017-06-27 15:08:17 UTC Comment hidden (obsolete)
Comment 26 Aleisha Amohia 2017-06-27 21:56:34 UTC Comment hidden (obsolete)
Comment 27 Marcel de Rooy 2017-06-28 10:05:06 UTC
Using patron instead of borrower should do it (see 18862)
Comment 28 Aleisha Amohia 2017-07-06 01:30:04 UTC Comment hidden (obsolete)
Comment 29 Simon Pouchol 2017-10-26 14:16:29 UTC
Hey,
I tried testing this patch, seems like it doesn't apply anymore :
Applying: Bug 17698: Make patron notes show up on staff dashboard
fatal: sha1 information is lacking or useless (Koha/Schema/Result/Issue.pm).
error: could not build fake ancestor
Patch failed at 0001 Bug 17698: Make patron notes show up on staff dashboard
Comment 30 Aleisha Amohia 2018-02-01 00:26:02 UTC Comment hidden (obsolete)
Comment 31 Aleisha Amohia 2018-02-01 21:48:04 UTC Comment hidden (obsolete)
Comment 32 Aleisha Amohia 2018-02-01 21:56:18 UTC Comment hidden (obsolete)
Comment 33 Aleisha Amohia 2018-02-01 21:58:34 UTC
Patches rebased, please test!
Comment 34 Victor Grousset/tuxayo 2018-03-12 15:13:48 UTC
~/d/koha ❯❯❯ git bz apply 17698
Bug 17698 - Make patron notes show up on staff dashboard

71100 - Bug 17698: Make patron notes show up on staff dashboard
71126 - Bug 17698: [FOLLOW-UP] Changing to Koha Objects style, adding circ sidebar
71127 - Bug 17698: [FOLLOW-UP] Koha Objects, access borrower through patron sub

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 17698: Make patron notes show up on staff dashboard
Using index info to reconstruct a base tree...
M	installer/data/mysql/kohastructure.sql
M	installer/data/mysql/userpermissions.sql
M	koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc
M	koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation-home.tt
M	koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt
M	mainpage.pl
Falling back to patching base and 3-way merge...
Auto-merging mainpage.pl
CONFLICT (content): Merge conflict in mainpage.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation-home.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc
Auto-merging installer/data/mysql/userpermissions.sql
Auto-merging installer/data/mysql/kohastructure.sql
error: Failed to merge in the changes.
Patch failed at 0001 Bug 17698: Make patron notes show up on staff dashboard
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-17698-Make-patron-notes-show-up-on-staff-dashb-H6e7WQ.patch
Comment 35 Owen Leonard 2018-03-14 15:00:25 UTC
Created attachment 72875 [details] [review]
Bug 17698: Make patron notes show up on staff dashboard

This patch adds a user permission for managing issue notes, and a 'noteseen'
column to the issues table.

To test:
1) Apply Bug 14224 first
2) Apply this patch, update database, rebuild schema.
3) Restart koha-common and memcached
4) Turn on AllowCheckoutNotes syspref if haven't already
5) Issue two items to two different users (one item each)
6) Log into the OPAC as one of the users and add an issue note to their
issue
7) Log out and log back into the OPAC as the other user
8) Disable Javascript
9) Refresh opac-user.pl
10) Leave a checkout note on their issue
11) Enable javascript and log into the Staff Client as a superlibrarian
user
12) Go to your user's account and edit their permissions to have
everything ticked EXCEPT circulate->manage checkout notes.
13) Go to main intranet page. There should be no message saying
'checkout notes pending'.
14) Go to circulation home page. There should be no link to Checkout notes.
15) Go back to user's permissions and tick circulate->manage checkout notes.
16) Go back to main intranet page. There should now be a message at the
bottom saying 'Checkout notes pending: 2'
17) Go to circulation home page. There should be a link to Checkout notes
with a 2 next to it. Click this link
18) Attempt to mark an checkout note as seen. This should update the status
of the checkout note to 'seen' and disable to 'mark as seen' button while
enabling the 'mark as not seen' button.
19) Test both buttons with both issues.
20) Test select all and clear all buttons
21) Confirm that buttons at the bottom are only enabled if a checkbox is
checked
22) Try selecting both issues and using the buttons at the bottom to
mark multiple issue notes at once.
23) Confirm the barcode link to the item works as expected.
24) Confirm the cardnumber link to the user works as expected.
25) Confirm all table details show correctly.

Sponsored-by: Catalyst IT

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 36 Owen Leonard 2018-03-14 15:00:32 UTC
Created attachment 72876 [details] [review]
Bug 17698: [FOLLOW-UP] Changing to Koha Objects style, adding circ sidebar

Marcel, can you please have a look at this patch. I tried to implement
the change
my @notes = $schema->resultset('Issue')->search({ 'me.note' => { '!=', undef } }, { prefetch => [ 'borrower', { item => 'biblionumber' } ] });
to
my @notes = Koha::Checkouts->search({ 'me.note' => { '!=', undef } }, { prefetch => [ 'borrower', { item => 'biblionumber' } ] });
but am having problems on the template side. I can access the item and
biblio information about the issue, but not the borrower information,
even though the query is definitely pulling it correctly. Any
suggestions or ideas as to why this breaks?

This patch also adds the implementation of the circSidebar.
Comment 37 Owen Leonard 2018-03-14 15:00:38 UTC
Created attachment 72877 [details] [review]
Bug 17698: [FOLLOW-UP] Koha Objects, access borrower through patron sub

See Comment 27
This is ready to be tested.
Comment 38 Owen Leonard 2018-03-14 15:01:39 UTC
I rebased the patches again. Fixed a minor conflict in mainpage.pl -- correctly, I hope!
Comment 39 Brendan Gallagher 2018-03-16 09:57:23 UTC
Created attachment 73004 [details] [review]
Bug 17698: Make patron notes show up on staff dashboard

This patch adds a user permission for managing issue notes, and a 'noteseen'
column to the issues table.

To test:
1) Apply Bug 14224 first
2) Apply this patch, update database, rebuild schema.
3) Restart koha-common and memcached
4) Turn on AllowCheckoutNotes syspref if haven't already
5) Issue two items to two different users (one item each)
6) Log into the OPAC as one of the users and add an issue note to their
issue
7) Log out and log back into the OPAC as the other user
8) Disable Javascript
9) Refresh opac-user.pl
10) Leave a checkout note on their issue
11) Enable javascript and log into the Staff Client as a superlibrarian
user
12) Go to your user's account and edit their permissions to have
everything ticked EXCEPT circulate->manage checkout notes.
13) Go to main intranet page. There should be no message saying
'checkout notes pending'.
14) Go to circulation home page. There should be no link to Checkout notes.
15) Go back to user's permissions and tick circulate->manage checkout notes.
16) Go back to main intranet page. There should now be a message at the
bottom saying 'Checkout notes pending: 2'
17) Go to circulation home page. There should be a link to Checkout notes
with a 2 next to it. Click this link
18) Attempt to mark an checkout note as seen. This should update the status
of the checkout note to 'seen' and disable to 'mark as seen' button while
enabling the 'mark as not seen' button.
19) Test both buttons with both issues.
20) Test select all and clear all buttons
21) Confirm that buttons at the bottom are only enabled if a checkbox is
checked
22) Try selecting both issues and using the buttons at the bottom to
mark multiple issue notes at once.
23) Confirm the barcode link to the item works as expected.
24) Confirm the cardnumber link to the user works as expected.
25) Confirm all table details show correctly.

Sponsored-by: Catalyst IT

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 40 Brendan Gallagher 2018-03-16 09:57:36 UTC
Created attachment 73005 [details] [review]
Bug 17698: [FOLLOW-UP] Changing to Koha Objects style, adding circ sidebar

Marcel, can you please have a look at this patch. I tried to implement
the change
my @notes = $schema->resultset('Issue')->search({ 'me.note' => { '!=', undef } }, { prefetch => [ 'borrower', { item => 'biblionumber' } ] });
to
my @notes = Koha::Checkouts->search({ 'me.note' => { '!=', undef } }, { prefetch => [ 'borrower', { item => 'biblionumber' } ] });
but am having problems on the template side. I can access the item and
biblio information about the issue, but not the borrower information,
even though the query is definitely pulling it correctly. Any
suggestions or ideas as to why this breaks?

This patch also adds the implementation of the circSidebar.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 41 Brendan Gallagher 2018-03-16 09:57:42 UTC
Created attachment 73006 [details] [review]
Bug 17698: [FOLLOW-UP] Koha Objects, access borrower through patron sub

See Comment 27
This is ready to be tested.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 42 Katrin Fischer 2018-04-01 13:52:06 UTC
There are some mostly small issues:

1) Small rebase conflict in SQL files for permissions

2) Capitalization
+<title>Home &rsaquo; Circulation &rsaquo; Checkout Notes</title>

2) Translatability
$("#status_" + $issue_id).text("Not seen");
$("#status_" + $issue_id).text("Seen");

3) Move JavaScript to the bottom - we are currently migrating all pages to do that. See: Bug 17858 - Omnibus: Move staff client JavaScript to the footer 

4) Missing [% VERSION %] variable for JavaScript libraries:
+<script type="text/javascript" src="[% interface %]/lib/jquery/plugins/jquery.checkboxes.min.js"></script>
See: Bug 12904 - Force browser to load new JavaScript and CSS files after upgrade

5) Typo:
+   ( 1, 'manage_checkout_nots', 'Mark checkout notes as seen/not seen'),

6) There are also some issues noted by the QA script:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/circ/checkout-notes.tt
   FAIL	  forbidden patterns
		forbidden pattern: Include [% KOHA_VERSION %] in css paths (bug 12904) - May be false positive (line 7)
		forbidden pattern: Include [% KOHA_VERSION %] in js paths (bug 12904) - May be false positive (line 76)
   FAIL	  valid_template
		parse error - /home/vagrant/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/circ/checkout-notes.tt line 134: unexpected token ([)
  [% note.patron.title [% END %]

 FAIL	circ/on-site_checkouts.pl
   FAIL	  valid
		Global symbol "$pending_onsite_notes" requires explicit package name 
		circ/on-site_checkouts.pl had compilation errors.


I've tried to test, but I can't see the link on the circulation home page, possibly to a problem (typo?) with the permissions. Please fix and test. Thank you!
Comment 43 Aleisha Amohia 2018-04-09 03:23:16 UTC
Created attachment 73839 [details] [review]
Bug 17698: Make patron notes show up on staff dashboard

This patch adds a user permission for managing issue notes, and a 'noteseen'
column to the issues table.

To test:
1) Apply Bug 14224 first
2) Apply this patch, update database, rebuild schema.
3) Restart koha-common and memcached
4) Turn on AllowCheckoutNotes syspref if haven't already
5) Issue two items to two different users (one item each)
6) Log into the OPAC as one of the users and add an issue note to their
issue
7) Log out and log back into the OPAC as the other user
8) Disable Javascript
9) Refresh opac-user.pl
10) Leave a checkout note on their issue
11) Enable javascript and log into the Staff Client as a superlibrarian
user
12) Go to your user's account and edit their permissions to have
everything ticked EXCEPT circulate->manage checkout notes.
13) Go to main intranet page. There should be no message saying
'checkout notes pending'.
14) Go to circulation home page. There should be no link to Checkout notes.
15) Go back to user's permissions and tick circulate->manage checkout notes.
16) Go back to main intranet page. There should now be a message at the
bottom saying 'Checkout notes pending: 2'
17) Go to circulation home page. There should be a link to Checkout notes
with a 2 next to it. Click this link
18) Attempt to mark an checkout note as seen. This should update the status
of the checkout note to 'seen' and disable to 'mark as seen' button while
enabling the 'mark as not seen' button.
19) Test both buttons with both issues.
20) Test select all and clear all buttons
21) Confirm that buttons at the bottom are only enabled if a checkbox is
checked
22) Try selecting both issues and using the buttons at the bottom to
mark multiple issue notes at once.
23) Confirm the barcode link to the item works as expected.
24) Confirm the cardnumber link to the user works as expected.
25) Confirm all table details show correctly.

Sponsored-by: Catalyst IT

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 44 Aleisha Amohia 2018-04-09 03:23:24 UTC
Created attachment 73840 [details] [review]
Bug 17698: [FOLLOW-UP] Changing to Koha Objects style, adding circ sidebar

Marcel, can you please have a look at this patch. I tried to implement
the change
my @notes = $schema->resultset('Issue')->search({ 'me.note' => { '!=', undef } }, { prefetch => [ 'borrower', { item => 'biblionumber' } ] });
to
my @notes = Koha::Checkouts->search({ 'me.note' => { '!=', undef } }, { prefetch => [ 'borrower', { item => 'biblionumber' } ] });
but am having problems on the template side. I can access the item and
biblio information about the issue, but not the borrower information,
even though the query is definitely pulling it correctly. Any
suggestions or ideas as to why this breaks?

This patch also adds the implementation of the circSidebar.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 45 Aleisha Amohia 2018-04-09 03:23:32 UTC
Created attachment 73841 [details] [review]
Bug 17698: [FOLLOW-UP] Koha Objects, access borrower through patron sub

See Comment 27
This is ready to be tested.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 46 Katrin Fischer 2018-04-09 06:09:53 UTC
Hi Aleisha, is this ready for retesting or one follow-up missing?
Comment 47 Aleisha Amohia 2018-04-12 00:57:30 UTC
(In reply to Katrin Fischer from comment #46)
> Hi Aleisha, is this ready for retesting or one follow-up missing?

Not yet ready for testing, just rebased. Working on your comments in Comment 42 :)
Comment 48 Aleisha Amohia 2018-04-12 04:26:54 UTC
Created attachment 74044 [details] [review]
Bug 17698: [FOLLOW-UP] Fixing some small issues

Fixing the comments in Comment 42

Ready to test
Comment 49 Katrin Fischer 2018-04-15 12:09:35 UTC
Created attachment 74193 [details] [review]
Bug 17698: (QA follow-up) Changes to use Asset TT plugin (bug 20538)
Comment 50 Katrin Fischer 2018-04-15 12:16:40 UTC
Hi Aleisha, 

I have added a follow-up, to adapt your patches to the recent changes from bug 20538. Then I've started testing, but I can't add a note from the OPAC:

No property noteseen for Koha::Checkout

I've run the database update and updated the Schema, but keep getting this error.

Can you please fix?

The last follow-up I would have counted for a QA follow-up as the changes are clearly what I asked for. If this is a small fix, consider changing back to signed off.
Comment 51 Aleisha Amohia 2018-06-05 23:04:33 UTC
Created attachment 75837 [details] [review]
Bug 17698: Make patron notes show up on staff dashboard

This patch adds a user permission for managing issue notes, and a 'noteseen'
column to the issues table.

To test:
1) Apply Bug 14224 first
2) Apply this patch, update database, rebuild schema.
3) Restart koha-common and memcached
4) Turn on AllowCheckoutNotes syspref if haven't already
5) Issue two items to two different users (one item each)
6) Log into the OPAC as one of the users and add an issue note to their
issue
7) Log out and log back into the OPAC as the other user
8) Disable Javascript
9) Refresh opac-user.pl
10) Leave a checkout note on their issue
11) Enable javascript and log into the Staff Client as a superlibrarian
user
12) Go to your user's account and edit their permissions to have
everything ticked EXCEPT circulate->manage checkout notes.
13) Go to main intranet page. There should be no message saying
'checkout notes pending'.
14) Go to circulation home page. There should be no link to Checkout notes.
15) Go back to user's permissions and tick circulate->manage checkout notes.
16) Go back to main intranet page. There should now be a message at the
bottom saying 'Checkout notes pending: 2'
17) Go to circulation home page. There should be a link to Checkout notes
with a 2 next to it. Click this link
18) Attempt to mark an checkout note as seen. This should update the status
of the checkout note to 'seen' and disable to 'mark as seen' button while
enabling the 'mark as not seen' button.
19) Test both buttons with both issues.
20) Test select all and clear all buttons
21) Confirm that buttons at the bottom are only enabled if a checkbox is
checked
22) Try selecting both issues and using the buttons at the bottom to
mark multiple issue notes at once.
23) Confirm the barcode link to the item works as expected.
24) Confirm the cardnumber link to the user works as expected.
25) Confirm all table details show correctly.

Sponsored-by: Catalyst IT

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 52 Aleisha Amohia 2018-06-05 23:04:41 UTC
Created attachment 75838 [details] [review]
Bug 17698: [FOLLOW-UP] Changing to Koha Objects style, adding circ sidebar

Marcel, can you please have a look at this patch. I tried to implement
the change
my @notes = $schema->resultset('Issue')->search({ 'me.note' => { '!=', undef } }, { prefetch => [ 'borrower', { item => 'biblionumber' } ] });
to
my @notes = Koha::Checkouts->search({ 'me.note' => { '!=', undef } }, { prefetch => [ 'borrower', { item => 'biblionumber' } ] });
but am having problems on the template side. I can access the item and
biblio information about the issue, but not the borrower information,
even though the query is definitely pulling it correctly. Any
suggestions or ideas as to why this breaks?

This patch also adds the implementation of the circSidebar.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 53 Aleisha Amohia 2018-06-05 23:04:48 UTC
Created attachment 75839 [details] [review]
Bug 17698: [FOLLOW-UP] Koha Objects, access borrower through patron sub

See Comment 27
This is ready to be tested.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 54 Aleisha Amohia 2018-06-05 23:04:54 UTC
Created attachment 75840 [details] [review]
Bug 17698: [FOLLOW-UP] Fixing some small issues

Fixing the comments in Comment 42

Ready to test
Comment 55 Aleisha Amohia 2018-06-05 23:05:03 UTC
Created attachment 75841 [details] [review]
Bug 17698: (QA follow-up) Changes to use Asset TT plugin (bug 20538)
Comment 56 Aleisha Amohia 2018-06-05 23:20:20 UTC
Created attachment 75842 [details] [review]
Bug 17698: [FOLLOW-UP] Rebase and schema update

Setting to signed off.
Comment 57 Jonathan Druart 2018-06-12 20:02:03 UTC
1. Bad resolution conflict, permission self_checkout is re-add (I will fix)

2. The IGNORE modifier is missing in the INSERT statement (I will fix)

3. In the JS console: "ReferenceError: $ is not defined", I did not investigate it.

4. When I hit /circ/checkout-notes.pl from the side bar, the page displays "Checkout notes", nothing else. We should add "There is no checkout notes". (I will fix)

5. The alert id=error is displayed then hide in JS, but it's then displayed half a second. We should hide it by default (css)

6. I would move the "mark seen" and "mark not seen" buttons at the top of the table

7. I would display the table on the confirmation screen as well (I will fix)

8. Cursor on "Select all" and "Clear all" links must be adjusted

9. $(".btn-xs").click(function(event){
The selector should be $("button.seen, button.notseen"), you do not want to apply this function to all other btn-xs on the page (maybe there are only two for now, but who knows later?)

10. html filters are missing (I will fix)

11. span element should surround translatable string, to help translators (I will fix).

12. Important: When a note is updated, it's still marked as seen. Is it the expected behavior?

13. What will happen when hundred of notes will be on this table? Not blocker but we will need a "hide seen" buttons to filters the already seen notes.

14. patron-title.inc must be used to display patron's info (I will fix)
Comment 58 Jonathan Druart 2018-06-12 20:04:28 UTC
Created attachment 76011 [details] [review]
Bug 17698: (QA follow-up) Fix minors design issues

This patch includes some changes required on comment 57:

1. Bad resolution conflict, permission self_checkout is re-add

2. The IGNORE modifier is missing in the INSERT statement

4. When I hit /circ/checkout-notes.pl from the side bar, the page displays "Checkout notes", nothing else. We should add "There is no checkout notes".

7. I would display the table on the confirmation screen as well

10. html filters are missing

11. span element should surround translatable string, to help translators

14. patron-title.inc must be used to display patron's info
Comment 59 Aleisha Amohia 2018-06-12 23:17:05 UTC
Created attachment 76012 [details] [review]
Bug 17698: [FOLLOW-UP] Fixing things in Comment 57

3. In the JS console: "ReferenceError: $ is not defined", I did not
investigate it.

Where do you see this in the console? I cannot recreate on opac-user.pl
or on circ/checkout-notes.pl.

5. The alert id=error is displayed then hide in JS, but it's then
displayed half a second. We should hide it by default (css)

Fixed in this patch

6. I would move the "mark seen" and "mark not seen" buttons at the
top of the table

Fixed in this patch

8. Cursor on "Select all" and "Clear all" links must be adjusted

Fixed in this patch

9. $(".btn-xs").click(function(event){
The selector should be $("button.seen, button.notseen"), you
do not want to apply this function to all other btn-xs on
the page (maybe there are only two for now, but who knows
later?)

Fixed in this patch

12. Important: When a note is updated, it's still marked as
seen. Is it the expected behavior?

I don't see this behaviour. When a note is updated it is
marked as not seen.
opav/svc/checkout_notes:79: $issue->set({ notedate =>
dt_from_string(), note => $clean_note, noteseen => 0
})->store;

13. What will happen when hundred of notes will be on this
table? Not blocker but we will need a "hide seen" buttons to
filters the already seen notes.

Added in this patch
Comment 60 Jonathan Druart 2018-06-13 15:23:47 UTC
Created attachment 76028 [details] [review]
Bug 17698: Make patron notes show up on staff dashboard

This patch adds a user permission for managing issue notes, and a 'noteseen'
column to the issues table.

To test:
1) Apply Bug 14224 first
2) Apply this patch, update database, rebuild schema.
3) Restart koha-common and memcached
4) Turn on AllowCheckoutNotes syspref if haven't already
5) Issue two items to two different users (one item each)
6) Log into the OPAC as one of the users and add an issue note to their
issue
7) Log out and log back into the OPAC as the other user
8) Disable Javascript
9) Refresh opac-user.pl
10) Leave a checkout note on their issue
11) Enable javascript and log into the Staff Client as a superlibrarian
user
12) Go to your user's account and edit their permissions to have
everything ticked EXCEPT circulate->manage checkout notes.
13) Go to main intranet page. There should be no message saying
'checkout notes pending'.
14) Go to circulation home page. There should be no link to Checkout notes.
15) Go back to user's permissions and tick circulate->manage checkout notes.
16) Go back to main intranet page. There should now be a message at the
bottom saying 'Checkout notes pending: 2'
17) Go to circulation home page. There should be a link to Checkout notes
with a 2 next to it. Click this link
18) Attempt to mark an checkout note as seen. This should update the status
of the checkout note to 'seen' and disable to 'mark as seen' button while
enabling the 'mark as not seen' button.
19) Test both buttons with both issues.
20) Test select all and clear all buttons
21) Confirm that buttons at the bottom are only enabled if a checkbox is
checked
22) Try selecting both issues and using the buttons at the bottom to
mark multiple issue notes at once.
23) Confirm the barcode link to the item works as expected.
24) Confirm the cardnumber link to the user works as expected.
25) Confirm all table details show correctly.

Sponsored-by: Catalyst IT

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Amended patch: Remove self-checkout permissions
Comment 61 Jonathan Druart 2018-06-13 15:23:54 UTC
Created attachment 76029 [details] [review]
Bug 17698: (follow-up) Changing to Koha Objects style, adding circ sidebar

Marcel, can you please have a look at this patch. I tried to implement
the change
my @notes = $schema->resultset('Issue')->search({ 'me.note' => { '!=', undef } }, { prefetch => [ 'borrower', { item => 'biblionumber' } ] });
to
my @notes = Koha::Checkouts->search({ 'me.note' => { '!=', undef } }, { prefetch => [ 'borrower', { item => 'biblionumber' } ] });
but am having problems on the template side. I can access the item and
biblio information about the issue, but not the borrower information,
even though the query is definitely pulling it correctly. Any
suggestions or ideas as to why this breaks?

This patch also adds the implementation of the circSidebar.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 62 Jonathan Druart 2018-06-13 15:24:00 UTC
Created attachment 76030 [details] [review]
Bug 17698: (follow-up) Koha Objects, access borrower through patron sub

See Comment 27
This is ready to be tested.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 63 Jonathan Druart 2018-06-13 15:24:06 UTC
Created attachment 76031 [details] [review]
Bug 17698: (follow-up) Fixing some small issues

Fixing the comments in Comment 42

Ready to test

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 64 Jonathan Druart 2018-06-13 15:24:13 UTC
Created attachment 76032 [details] [review]
Bug 17698: (QA follow-up) Changes to use Asset TT plugin (bug 20538)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 65 Jonathan Druart 2018-06-13 15:24:19 UTC
Created attachment 76033 [details] [review]
Bug 17698: (follow-up) Rebase and schema update

Setting to signed off.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 66 Jonathan Druart 2018-06-13 15:24:25 UTC
Created attachment 76034 [details] [review]
Bug 17698: (QA follow-up) Fix minors design issues

This patch includes some changes required on comment 57:

1. Bad resolution conflict, permission self_checkout is re-add

2. The IGNORE modifier is missing in the INSERT statement

4. When I hit /circ/checkout-notes.pl from the side bar, the page displays "Checkout notes", nothing else. We should add "There is no checkout notes".

7. I would display the table on the confirmation screen as well

10. html filters are missing

11. span element should surround translatable string, to help translators

14. patron-title.inc must be used to display patron's info

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 67 Jonathan Druart 2018-06-13 15:24:31 UTC
Created attachment 76035 [details] [review]
Bug 17698: (follow-up) Fixing things in Comment 57

3. In the JS console: "ReferenceError: $ is not defined", I did not
investigate it.

Where do you see this in the console? I cannot recreate on opac-user.pl
or on circ/checkout-notes.pl.

5. The alert id=error is displayed then hide in JS, but it's then
displayed half a second. We should hide it by default (css)

Fixed in this patch

6. I would move the "mark seen" and "mark not seen" buttons at the
top of the table

Fixed in this patch

8. Cursor on "Select all" and "Clear all" links must be adjusted

Fixed in this patch

9. $(".btn-xs").click(function(event){
The selector should be $("button.seen, button.notseen"), you
do not want to apply this function to all other btn-xs on
the page (maybe there are only two for now, but who knows
later?)

Fixed in this patch

12. Important: When a note is updated, it's still marked as
seen. Is it the expected behavior?

I don't see this behaviour. When a note is updated it is
marked as not seen.
opav/svc/checkout_notes:79: $issue->set({ notedate =>
dt_from_string(), note => $clean_note, noteseen => 0
})->store;

13. What will happen when hundred of notes will be on this
table? Not blocker but we will need a "hide seen" buttons to
filters the already seen notes.

Added in this patch

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 68 Jonathan Druart 2018-06-13 15:24:37 UTC
Created attachment 76036 [details] [review]
Bug 17698: (QA follow-up) few more things

1. body tag was missing
2. make "Date" column sortable correctly
3. remove CDATA and type="text/javascript"
4. Handle server-side errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 69 Jonathan Druart 2018-06-13 15:24:43 UTC
Created attachment 76037 [details] [review]
Bug 17698: Do not send pending_checkout_notes from all circ scripts

It's safer to send what we need from C4::Auth it's needed from a whole
module.
The SELECT COUNT(*) query will only be done when needed (so not made
from scripts outside of circ)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 70 Jonathan Druart 2018-06-13 15:28:41 UTC
Thanks Aleisha for your quick follow-up, very appreciated.

Take a look at the 2 last follow-ups I have attached, they fixed few more things and remove duplication of code. You should also make sure your commit messages fit the coding guidelines (https://wiki.koha-community.org/wiki/Commit_messages#Examples), maybe you should update your qa-test-tools repo, there are new checks to catch that.

This new enhancement is well polished and the code is easy to read. Congrats!
Comment 71 Nick Clemens 2018-07-23 15:37:43 UTC
Awesome work all!

Pushed to master for 18.11
Comment 72 Martin Renvoize 2018-08-03 13:35:51 UTC
Enhancement, will not be backported to 18.05.x series. Great work though Aleisha, looks like a nice new feature to debut in 18.11.0!