Bug 14224 - patron notes about item shown at check in
Summary: patron notes about item shown at check in
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 27105 35615 17698 18915 19496 19840
  Show dependency treegraph
 
Reported: 2015-05-19 15:19 UTC by Nicole C. Engard
Modified: 2023-12-20 14:15 UTC (History)
16 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This feature adds a "Note" input field to checked out items in the "your summary" section of the patron account in the OPAC. The field allows patrons to write notes about the item, such as "this DVD is scratched", "the binding was torn", etc. The note will be emailed to the library and displayed on check in.
Version(s) released in:


Attachments
Bug 14224: Allow patron notes about item shown at check in (16.01 KB, patch)
2015-08-23 16:39 UTC, Martin Stenberg
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (22.94 KB, patch)
2015-08-25 14:23 UTC, Martin Stenberg
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (24.79 KB, patch)
2016-10-20 01:05 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (24.82 KB, patch)
2016-11-11 02:11 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (29.80 KB, patch)
2016-11-17 00:26 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (30.20 KB, patch)
2016-11-24 23:05 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (36.19 KB, patch)
2016-11-29 03:14 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Just a try with Koha::Issues->find (937 bytes, patch)
2016-11-30 09:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (34.57 KB, patch)
2016-12-01 02:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (34.26 KB, patch)
2016-12-19 22:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (32.74 KB, patch)
2016-12-21 00:56 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (33.55 KB, patch)
2016-12-21 01:00 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (33.45 KB, patch)
2016-12-21 22:34 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (36.10 KB, patch)
2017-01-04 22:50 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (38.55 KB, patch)
2017-01-05 21:55 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (34.47 KB, patch)
2017-01-23 22:30 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (34.49 KB, patch)
2017-01-29 21:16 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14224: Allow patron notes about item shown at check in (34.56 KB, patch)
2017-01-30 10:10 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (32.79 KB, patch)
2017-02-02 02:58 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (32.86 KB, patch)
2017-02-02 10:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Check if the columns do not exist before creating them (2.46 KB, patch)
2017-02-02 10:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Do not reintroduce GetItemIssue, it's dying (992 bytes, patch)
2017-02-02 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Reintroduce previous copyright (638 bytes, patch)
2017-02-02 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Remove SQL query in svc script (1.29 KB, patch)
2017-02-02 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Fix prevent submit (1.48 KB, patch)
2017-02-02 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Make strings translatable (5.91 KB, patch)
2017-02-02 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Replace AllowIssueNotes with AllowCheckoutNotes (7.87 KB, patch)
2017-02-02 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Update Copyright (803 bytes, patch)
2017-02-02 10:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Improve escaped characters (1.17 KB, patch)
2017-02-02 10:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Check if the columns do not exist before creating them (2.44 KB, patch)
2017-02-02 11:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Do not reintroduce GetItemIssue, it's dying (1.02 KB, patch)
2017-02-02 11:43 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14224: Reintroduce previous copyright (694 bytes, patch)
2017-02-02 11:43 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14224: Remove SQL query in svc script (1.35 KB, patch)
2017-02-02 11:43 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14224: Fix prevent submit (1.53 KB, patch)
2017-02-02 11:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14224: Make strings translatable (5.97 KB, patch)
2017-02-02 11:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14224: Replace AllowIssueNotes with AllowCheckoutNotes (7.92 KB, patch)
2017-02-02 11:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14224: Update Copyright (859 bytes, patch)
2017-02-02 11:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14224: Improve escaped characters (1.22 KB, patch)
2017-02-02 11:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14224: Check if the columns do not exist before creating them (2.49 KB, patch)
2017-02-02 11:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (32.60 KB, patch)
2017-04-04 21:06 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Do not reintroduce GetItemIssue, it's dying (1.02 KB, patch)
2017-04-04 21:06 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Reintroduce previous copyright (694 bytes, patch)
2017-04-04 21:06 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Remove SQL query in svc script (1.35 KB, patch)
2017-04-04 21:06 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Fix prevent submit (1.53 KB, patch)
2017-04-04 21:06 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Make strings translatable (5.97 KB, patch)
2017-04-04 21:07 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Replace AllowIssueNotes with AllowCheckoutNotes (7.92 KB, patch)
2017-04-04 21:07 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Update Copyright (859 bytes, patch)
2017-04-04 21:07 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Improve escaped characters (1.22 KB, patch)
2017-04-04 21:07 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Check if the columns do not exist before creating them (2.49 KB, patch)
2017-04-04 21:07 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (32.74 KB, patch)
2017-04-17 07:43 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14224: Do not reintroduce GetItemIssue, it's dying (1.16 KB, patch)
2017-04-17 07:44 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14224: Reintroduce previous copyright (829 bytes, patch)
2017-04-17 07:45 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14224: Remove SQL query in svc script (1.48 KB, patch)
2017-04-17 07:46 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14224: Fix prevent submit (1.67 KB, patch)
2017-04-17 07:56 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14224: Make strings translatable (6.10 KB, patch)
2017-04-17 07:57 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14224: Replace AllowIssueNotes with AllowCheckoutNotes (8.05 KB, patch)
2017-04-17 07:58 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14224: Update Copyright (994 bytes, patch)
2017-04-17 07:59 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14224: Improve escaped characters (1.36 KB, patch)
2017-04-17 08:01 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14224: Check if the columns do not exist before creating them (2.62 KB, patch)
2017-04-17 08:02 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14224: (follow-up): Make äöü display properly after input (2.51 KB, patch)
2017-04-17 08:18 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14224: Allow patron notes about item shown at check in (31.89 KB, patch)
2017-04-24 19:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Do not reintroduce GetItemIssue, it's dying (1.16 KB, patch)
2017-04-24 19:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Reintroduce previous copyright (829 bytes, patch)
2017-04-24 19:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Remove SQL query in svc script (1.48 KB, patch)
2017-04-24 19:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Fix prevent submit (1.67 KB, patch)
2017-04-24 19:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Make strings translatable (6.10 KB, patch)
2017-04-24 19:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Replace AllowIssueNotes with AllowCheckoutNotes (8.05 KB, patch)
2017-04-24 19:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Update Copyright (994 bytes, patch)
2017-04-24 19:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Improve escaped characters (1.36 KB, patch)
2017-04-24 19:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Check if the columns do not exist before creating them (2.62 KB, patch)
2017-04-24 19:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14224: Use encode_json instead of to_json (1.04 KB, patch)
2017-04-24 19:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14424: DBIC Schema changes (3.06 KB, patch)
2017-04-24 19:07 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 Nicole C. Engard 2015-05-19 15:19:20 UTC
It would be nice (per a patron) if there was a way for patrons to enter a note about the item they have checked out - this DVD is scratched - the binding was torn - etc. Then this note emails to the library and shows up at check in so the librarian knows that the item is damages or has an issue.
Comment 1 Martin Stenberg 2015-08-23 16:39:45 UTC Comment hidden (obsolete)
Comment 2 Martin Stenberg 2015-08-25 14:23:23 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2015-12-28 11:23:52 UTC
CONFLICT (content): Merge conflict in opac/opac-user.pl
CONFLICT (content): Merge conflict in C4/Circulation.pm

Tests are missing for the new subroutine.

GetIssues which takes issue_id is more a GetIssue subroutine, don't you think?

Why are you doing an ajax request? It won't work if the user does not have javascript enabled.
Comment 4 Martin Stenberg 2016-01-01 14:18:30 UTC
Hi Jonathan,

Do you think it's worth adding a new subroutine GetIssue for this special case of GetIssues? GetIssues will still return an arrayref when called with issue_id, so the return type matches the plural "s" at least.

I'm doing an ajax request to minimize overhead when sending the request (i.e. no need to send note data for all issues when only one has been changed) and when recieving (i.e. no need to render a new template and send a whole document when only an "ok" or "nok" is needed). And as a bonus it allows the user to write a second note while the first one is beeing sent/processed to the server. 

To be honest I thought javascript was concidered a requirment for Koha. I'm using NoScript myself, and am always positivly surprised when a page works without having to enable javascript.

How about keeping the ajax-version and adding a fallback for browsers without javascript?
Comment 5 Jonathan Druart 2016-01-04 11:15:38 UTC
(In reply to Martin Stenberg from comment #4)
> Hi Jonathan,
> 
> Do you think it's worth adding a new subroutine GetIssue for this special
> case of GetIssues? GetIssues will still return an arrayref when called with
> issue_id, so the return type matches the plural "s" at least.

GetIssue would not return an array, but the matching issue.

> I'm doing an ajax request to minimize overhead when sending the request
> (i.e. no need to send note data for all issues when only one has been
> changed) and when recieving (i.e. no need to render a new template and send
> a whole document when only an "ok" or "nok" is needed). And as a bonus it
> allows the user to write a second note while the first one is beeing
> sent/processed to the server. 
> 
> To be honest I thought javascript was concidered a requirment for Koha. I'm
> using NoScript myself, and am always positivly surprised when a page works
> without having to enable javascript.
> 
> How about keeping the ajax-version and adding a fallback for browsers
> without javascript?

You can add 2 forms in the page, the one needing JS can be hidden with a css rule.
When the page is loading, a JS code could hide one and display the other one.
I suppose it's the way to go.
Comment 6 Jonathan Druart 2016-01-04 11:17:04 UTC
Additional note: the ajax part should be moved to svc/ and some of this code put in a subroutine.
Comment 7 Aleisha Amohia 2016-10-20 01:05:42 UTC Comment hidden (obsolete)
Comment 8 Aleisha Amohia 2016-10-20 01:07:21 UTC
(In reply to Aleisha Amohia from comment #7)

> - Still to do:
>   - GetIssue/GetIssues
>   - subroutine for ajax code
>   - moving ajax part to svc/

and tests for new subroutines.
Comment 9 Aleisha Amohia 2016-11-11 02:11:46 UTC Comment hidden (obsolete)
Comment 10 Aleisha Amohia 2016-11-17 00:26:14 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2016-11-17 07:30:05 UTC
Hi Aleisha,
The getter and setter subroutines (GetIssue, SetIssueNote, SendIssueNote, GetPatronNote) should not be added to C4 but use Koha::Issue[s] instead. Let me know if you need help to do so.
And you won't have to provide new tests ;)
Ideally the svc script should not be added, but you should use the REST API instead. However the routes for checkins do not exist yet.
Comment 12 Aleisha Amohia 2016-11-24 23:05:44 UTC Comment hidden (obsolete)
Comment 13 Aleisha Amohia 2016-11-24 23:08:20 UTC
(In reply to Jonathan Druart from comment #11)
> Hi Aleisha,
> The getter and setter subroutines (GetIssue, SetIssueNote, SendIssueNote,
> GetPatronNote) should not be added to C4 but use Koha::Issue[s] instead. Let
> me know if you need help to do so.
> And you won't have to provide new tests ;)
> Ideally the svc script should not be added, but you should use the REST API
> instead. However the routes for checkins do not exist yet.

Hi Jonathan,

My latest patch moves those subroutines into Koha::Issue. Are you sure we don't need tests?
I think we'll leave the svc stuff in the checkins file for now while the routes for checkins do not exist. 

I will now be working on the fallback form for browsers without JS.
Comment 14 Jonathan Druart 2016-11-25 07:43:35 UTC
(In reply to Aleisha Amohia from comment #13)
> (In reply to Jonathan Druart from comment #11)
> > Hi Aleisha,
> > The getter and setter subroutines (GetIssue, SetIssueNote, SendIssueNote,
> > GetPatronNote) should not be added to C4 but use Koha::Issue[s] instead. Let
> > me know if you need help to do so.
> > And you won't have to provide new tests ;)
> > Ideally the svc script should not be added, but you should use the REST API
> > instead. However the routes for checkins do not exist yet.
> 
> Hi Jonathan,
> 
> My latest patch moves those subroutines into Koha::Issue. Are you sure we
> don't need tests?

By using Koha::Issue I meant using it as an object (so no need to add you subroutine to the module).
For instance:
  $issue = Koha::Issue::GetIssue({issue_id => $issue_id});
could be
  $issue = Koha::Issues->find( $issue_id);
and $issue becomes a Koha::Object-based object, not an hashref

  SetIssueNote($issue_id, $clean_note)
could be
  Koha::Issues->find( $issue_id )->set({ notedate => dt_from_string, note = $clean_note })->store;


etc.
Comment 15 Aleisha Amohia 2016-11-29 03:14:39 UTC Comment hidden (obsolete)
Comment 16 Aleisha Amohia 2016-11-29 03:31:00 UTC
(In reply to Jonathan Druart from comment #14)
> By using Koha::Issue I meant using it as an object (so no need to add you
> subroutine to the module).
> For instance:
>   $issue = Koha::Issue::GetIssue({issue_id => $issue_id});
> could be
>   $issue = Koha::Issues->find( $issue_id);
> and $issue becomes a Koha::Object-based object, not an hashref
> 
>   SetIssueNote($issue_id, $clean_note)
> could be
>   Koha::Issues->find( $issue_id )->set({ notedate => dt_from_string, note =
> $clean_note })->store;
> 
> 
> etc.

Hi Jonathan, 

I'm having issues using Koha::Issues->find($issue_id).
I'm receiving this error:
Can't locate object method "_new_from_dbic" via package "Koha::Issue" at /home/vagrant/kohaclone/Koha/Objects.pm line 87
Do you know what could be causing this?

Aleisha
Comment 17 Jonathan Druart 2016-11-29 07:53:59 UTC
(In reply to Aleisha Amohia from comment #16)
> Hi Jonathan, 
> 
> I'm having issues using Koha::Issues->find($issue_id).
> I'm receiving this error:
> Can't locate object method "_new_from_dbic" via package "Koha::Issue" at
> /home/vagrant/kohaclone/Koha/Objects.pm line 87
> Do you know what could be causing this?
> 
> Aleisha

Hi Aleisha,
I have tried this small snippet:
  use Koha::Issues;
  my $issue = Koha::Issues->find(1);
  say $issue->branchcode;
and it works as expected.
Do you have a small part of code to recreate the error you got?
Comment 18 Aleisha Amohia 2016-11-29 22:09:17 UTC
(In reply to Jonathan Druart from comment #17) 
> Hi Aleisha,
> I have tried this small snippet:
>   use Koha::Issues;
>   my $issue = Koha::Issues->find(1);
>   say $issue->branchcode;
> and it works as expected.
> Do you have a small part of code to recreate the error you got?

use Koha::Issues;
my $issue_id = $query->param('issue_id');
if ( $issue_id =~ /\d+/ ) {
    $issue = Koha::Issues->find($issue_id);
}

This returns the error.
I have checked that issue_id is being assigned correctly and it is, so can't work out what I'm doing wrong.
Comment 19 Jonathan Druart 2016-11-30 08:22:45 UTC
(In reply to Aleisha Amohia from comment #18)

That should work. Do you have a patch applying against master I could test to recreate the problem?
Comment 20 Aleisha Amohia 2016-11-30 08:29:53 UTC
(In reply to Jonathan Druart from comment #19)
> (In reply to Aleisha Amohia from comment #18)
> 
> That should work. Do you have a patch applying against master I could test
> to recreate the problem?

Feel free to try with the most recent patch here.
Comment 21 Jonathan Druart 2016-11-30 09:28:21 UTC
Created attachment 57829 [details] [review]
Bug 14224: Just a try with Koha::Issues->find
Comment 22 Jonathan Druart 2016-11-30 09:28:57 UTC
(In reply to Aleisha Amohia from comment #20)
> (In reply to Jonathan Druart from comment #19)
> > (In reply to Aleisha Amohia from comment #18)
> > 
> > That should work. Do you have a patch applying against master I could test
> > to recreate the problem?
> 
> Feel free to try with the most recent patch here.

It works as expected for me.
Your try :)
Comment 23 Aleisha Amohia 2016-11-30 10:35:48 UTC
(In reply to Jonathan Druart from comment #22)
> It works as expected for me.
> Your try :)

Have no idea why that exact thing hasn't been working for me on multiple different dev boxes. Will try it again when I get into work tomorrow. Thanks :)
Comment 24 Aleisha Amohia 2016-12-01 02:43:58 UTC Comment hidden (obsolete)
Comment 25 Hugo Agud 2016-12-01 07:09:57 UTC
I have tried it, but I get the following error:
https://postimg.org/image/wj3mqhszj/

on plack.log
No property note for Koha::IssueNo property notedate for Koha::IssueNo property notedate for Koha::IssueNo property notedate for Koha::Issue

Tested on KohadevBox debian jessy 8
Comment 26 Hugo Agud 2016-12-01 07:34:13 UTC
Fixed the previous error (cache error) tested and signed
Comment 27 Jonathan Druart 2016-12-13 15:30:30 UTC
Aleisha, Have a look at your patches, there are some weird stuffs to fix:
1/ kohastructure: you re-add the entire item table
2/ update db: you should specify the "after" sql clause to keep the same order as the kohastructure (not necessary useful but good practice to have)
3/ There is no need to use strict and warnings alongside Modern::Perl;
4/ The svc script has a BibLibre copyright
5/ I don't understand the need of the new subroutine in the svc script (ajax_auth_cgi). You should not need it, have a look at how the other svc scripts deal with authentication
6/ QA script fails with:
 FAIL   installer/data/mysql/sysprefs.sql
   FAIL   sysprefs_order
                Not blocker: Sysprefs AllowIssueNotes is bad placed (see bug 10610)
Comment 28 Aleisha Amohia 2016-12-19 22:47:20 UTC Comment hidden (obsolete)
Comment 29 Aleisha Amohia 2016-12-19 22:48:26 UTC
(In reply to Jonathan Druart from comment #27)
> Aleisha, Have a look at your patches, there are some weird stuffs to fix:
> 1/ kohastructure: you re-add the entire item table
> 2/ update db: you should specify the "after" sql clause to keep the same
> order as the kohastructure (not necessary useful but good practice to have)
> 3/ There is no need to use strict and warnings alongside Modern::Perl;
> 4/ The svc script has a BibLibre copyright
> 5/ I don't understand the need of the new subroutine in the svc script
> (ajax_auth_cgi). You should not need it, have a look at how the other svc
> scripts deal with authentication
> 6/ QA script fails with:
>  FAIL   installer/data/mysql/sysprefs.sql
>    FAIL   sysprefs_order
>                 Not blocker: Sysprefs AllowIssueNotes is bad placed (see bug
> 10610)

Hi Jonathan,

I fixed all of these except the first point - what do you mean I re-add the entire items table? Sorry, I don't understand.

Aleisha
Comment 30 Jonathan Druart 2016-12-20 12:21:57 UTC
(In reply to Aleisha Amohia from comment #29)
> I fixed all of these except the first point - what do you mean I re-add the
> entire items table? Sorry, I don't understand.

Hi Aleisha,
Look at your patch, the kohastructure.sql file: You drop and recreate the issues table.
Note that you must add the 2 new columns to the old_issues table as well.
Comment 31 Aleisha Amohia 2016-12-21 00:56:51 UTC Comment hidden (obsolete)
Comment 32 Aleisha Amohia 2016-12-21 01:00:35 UTC
Created attachment 58326 [details] [review]
Bug 14224: Allow patron notes about item shown at check in

This patch adds a "Note" input field to checked out items in the "your summary"
section. The field allows patrons to write notes about the item checked out,
such as "this DVD is scratched", "the binding was torn", etc. The note will be
emailed to the library and displayed on item check in.

Patch adds two fields to the "issues" table - "note" and "notedate".
Patch adds syspref "AllowIssueNotes" - default off.

Test Plan:
1) Apply this patch
2) Update database - perl installer/data/mysql/updatedatabase.pl
3) (You may have to rebuild schema but I think the patch should apply
    the schema changes for you)
4) Turn on 'AllowIssueNote' syspref
5) Check out three different items to a borrower (may be easiest to check
    out to yourself)
6) Log in as that borrower (or yourself) on the OPAC side and go to your
summary
7) Confirm text field shows under Note column for all checkouts. Set a
note for each issue, confirm all save.
8) Check the message_queue in mysql for the entries for ALL THREE issue
notes.
9) Disable javascript in your browser
10) Refresh your summary page. Confirm that you can no longer edit the
notes in the text field. Click the 'Create/edit note' button and confirm
you are redirected to a new page.
11) Confirm that the correct title and author show for the note button
you clicked.
12) Set the note and click Submit -> confirm you are redirected
back to summary page and note is saved
13) Confirm there is a new entry in message_queue
14) Enable javascript and go back to the your checkouts page in the
staff client for the borrower you issued the items to
15) Check in TWO items
16) Confirm that the issue notes show under the two items you checked
in, and are accurate to the item (i.e. the right issue note under the
right item)
17) Go to circ/returns.pl and check in the final item using the barcode.
Confirm the issue note shows.

Sponsored-by: Region Halland
Comment 33 Aleisha Amohia 2016-12-21 01:00:59 UTC
(In reply to Jonathan Druart from comment #30)
> Hi Aleisha,
> Look at your patch, the kohastructure.sql file: You drop and recreate the
> issues table.
> Note that you must add the 2 new columns to the old_issues table as well.

Fixed that all up - ready for testing again.
Comment 34 Owen Leonard 2016-12-21 15:54:58 UTC
I think this is a great new feature. A few issues and suggestions:

- The preference should be retrieved via the template instead of in the Perl script:

https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML7:_use_system_preference_TT_plugin

- The date in the checkin message should be formatted using the KohaDates plugin.
- On the circulation page, I think it makes more sense to put the patron note in the column which shows the "Checked in" message since that's where the librarian would be looking for confirmation of the transaction.

Since there is no way to find out what note was attached to something you just checked in on returns.pl, I wonder if the notification shouldn't work the same as the hold notice: In a blocking modal?
Comment 35 Aleisha Amohia 2016-12-21 22:34:41 UTC
Created attachment 58372 [details] [review]
Bug 14224: Allow patron notes about item shown at check in

This patch adds a "Note" input field to checked out items in the "your summary"
section. The field allows patrons to write notes about the item checked out,
such as "this DVD is scratched", "the binding was torn", etc. The note will be
emailed to the library and displayed on item check in.

Patch adds two fields to the "issues" table - "note" and "notedate".
Patch adds syspref "AllowIssueNotes" - default off.

Test Plan:
1) Apply this patch
2) Update database - perl installer/data/mysql/updatedatabase.pl
3) (You may have to rebuild schema but I think the patch should apply
    the schema changes for you)
4) Turn on 'AllowIssueNote' syspref
5) Check out three different items to a borrower (may be easiest to check
    out to yourself)
6) Log in as that borrower (or yourself) on the OPAC side and go to your
summary
7) Confirm text field shows under Note column for all checkouts. Set a
note for each issue, confirm all save.
8) Check the message_queue in mysql for the entries for ALL THREE issue
notes.
9) Disable javascript in your browser
10) Refresh your summary page. Confirm that you can no longer edit the
notes in the text field. Click the 'Create/edit note' button and confirm
you are redirected to a new page.
11) Confirm that the correct title and author show for the note button
you clicked.
12) Set the note and click Submit -> confirm you are redirected
back to summary page and note is saved
13) Confirm there is a new entry in message_queue
14) Enable javascript and go back to the your checkouts page in the
staff client for the borrower you issued the items to
15) Check in TWO items
16) Confirm that the issue notes show under the "Date due" column for
the two items you checked in, and are accurate to the item (i.e. the
right issue note under the right item)
17) Go to circ/returns.pl and check in the final item using the barcode.
Confirm the issue note shows and the date is formatted correctly.

Sponsored-by: Region Halland
Comment 36 Aleisha Amohia 2016-12-21 22:38:17 UTC
Hi Owen,

(In reply to Owen Leonard from comment #34)
> I think this is a great new feature. A few issues and suggestions:
> 
> - The preference should be retrieved via the template instead of in the Perl
> script:
> 
> https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML7:
> _use_system_preference_TT_plugin
> 
> - The date in the checkin message should be formatted using the KohaDates
> plugin.
> - On the circulation page, I think it makes more sense to put the patron
> note in the column which shows the "Checked in" message since that's where
> the librarian would be looking for confirmation of the transaction.
>

I have implemented all these changes in the latest patch.

> Since there is no way to find out what note was attached to something you
> just checked in on returns.pl, I wonder if the notification shouldn't work
> the same as the hold notice: In a blocking modal?

I am working on a follow-up feature in Bug 17698 that allows a 'seen' system for the patron notes - similar to how we approve tags and comments made in the OPAC. I think this suggestion would be best implemented in that patch rather than this one if that's okay with you (would like to get this signed off and passed so I can finish 17698)!

Thanks
Aleisha
Comment 37 Aleisha Amohia 2017-01-04 22:50:51 UTC Comment hidden (obsolete)
Comment 38 Aleisha Amohia 2017-01-05 21:55:21 UTC
Created attachment 58622 [details] [review]
Bug 14224: Allow patron notes about item shown at check in

This patch adds a "Note" input field to checked out items in the "your summary"
section. The field allows patrons to write notes about the item checked out,
such as "this DVD is scratched", "the binding was torn", etc. The note will be
emailed to the library and displayed on item check in.

Patch adds two fields to the "issues" table - "note" and "notedate".
Patch adds syspref "AllowIssueNotes" - default off.

Test Plan:
1) Apply this patch
2) Update database
3) Rebuild schema
4) Turn on 'AllowIssueNote' syspref
5) Check out three different items to a borrower (may be easiest to check
    out to yourself)
6) Log in as that borrower (or yourself) on the OPAC side and go to your
summary
7) Confirm text field shows under Note column for all checkouts. Set a
note for each issue, confirm all save.
8) Check the message_queue in mysql for the entries for ALL THREE issue
notes.
9) Disable javascript in your browser
10) Refresh your summary page. Confirm that you can no longer edit the
notes in the text field. Click the 'Create/edit note' button and confirm
you are redirected to a new page.
11) Confirm that the correct title and author show for the note button
you clicked.
12) Set the note and click Submit -> confirm you are redirected
back to summary page and note is saved
13) Confirm there is a new entry in message_queue
14) Enable javascript and go back to the your checkouts page in the
staff client for the borrower you issued the items to
15) Check in TWO items
16) Confirm that the issue notes show under the "Date due" column for
the two items you checked in, and are accurate to the item (i.e. the
right issue note under the right item)
17) Go to circ/returns.pl and check in the final item using the barcode.
Confirm the issue note shows and the date is formatted correctly.

Sponsored-by: Region Halland
Comment 39 Aleisha Amohia 2017-01-05 21:56:14 UTC
I realised I had taken some shortcuts with the object-oriented stuff and wasn't doing it properly. The latest patch fixes that. This is ready to test.
Comment 40 Mark Tompsett 2017-01-17 06:23:51 UTC
(In reply to Aleisha Amohia from comment #39)
> I realised I had taken some shortcuts with the object-oriented stuff and
> wasn't doing it properly. The latest patch fixes that. This is ready to test.

We already had a Koha::Issues become Koha::Checkouts.
This seems to go backwards.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17796
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16870
Comment 41 Jonathan Druart 2017-01-17 07:54:28 UTC
Aleisha, it seems that your last patch kept the 2 subroutines in C4::Items. Since you are not using them, you can remove them.
Comment 42 Aleisha Amohia 2017-01-23 22:30:34 UTC
Created attachment 59469 [details] [review]
Bug 14224: Allow patron notes about item shown at check in

This patch adds a "Note" input field to checked out items in the "your summary"
section. The field allows patrons to write notes about the item checked out,
such as "this DVD is scratched", "the binding was torn", etc. The note will be
emailed to the library and displayed on item check in.

Patch adds two fields to the "issues" table - "note" and "notedate".
Patch adds syspref "AllowIssueNotes" - default off.

Test Plan:
1) Apply this patch
2) Update database
3) Rebuild schema
4) Turn on 'AllowIssueNote' syspref
5) Check out three different items to a borrower (may be easiest to check
    out to yourself)
6) Log in as that borrower (or yourself) on the OPAC side and go to your
summary
7) Confirm text field shows under Note column for all checkouts. Set a
note for each issue, confirm all save.
8) Check the message_queue in mysql for the entries for ALL THREE issue
notes.
9) Disable javascript in your browser
10) Refresh your summary page. Confirm that you can no longer edit the
notes in the text field. Click the 'Create/edit note' button and confirm
you are redirected to a new page.
11) Confirm that the correct title and author show for the note button
you clicked.
12) Set the note and click Submit -> confirm you are redirected
back to summary page and note is saved
13) Confirm there is a new entry in message_queue
14) Enable javascript and go back to the your checkouts page in the
staff client for the borrower you issued the items to
15) Check in TWO items
16) Confirm that the issue notes show under the "Date due" column for
the two items you checked in, and are accurate to the item (i.e. the
right issue note under the right item)
17) Go to circ/returns.pl and check in the final item using the barcode.
Confirm the issue note shows and the date is formatted correctly.

Sponsored-by: Region Halland
Comment 43 Owen Leonard 2017-01-24 13:39:24 UTC
(In reply to Aleisha Amohia from comment #36)

> > - On the circulation page, I think it makes more sense to put the patron
> > note in the column which shows the "Checked in" message since that's where
> > the librarian would be looking for confirmation of the transaction.

Was it supposed to work this way in your latest version? The note was still appearing in the first column of the table in my latest tests.
Comment 44 Aleisha Amohia 2017-01-24 18:30:24 UTC
(In reply to Owen Leonard from comment #43)
> Was it supposed to work this way in your latest version? The note was still
> appearing in the first column of the table in my latest tests.

The first column of the table also has the Checked In message. I moved it out of the Title (etc) column into that column, so yes, this is the way it is meant to work. :)
Comment 45 Josef Moravec 2017-01-24 20:36:15 UTC
Please, upgrade your templates to bootstrap 3 (see bug 16239) and 

https://wiki.koha-community.org/wiki/Interface_patterns
http://getbootstrap.com/css/#grid
Comment 46 Aleisha Amohia 2017-01-29 21:16:37 UTC
Created attachment 59637 [details] [review]
Bug 14224: Allow patron notes about item shown at check in

This patch adds a "Note" input field to checked out items in the "your summary"
section. The field allows patrons to write notes about the item checked out,
such as "this DVD is scratched", "the binding was torn", etc. The note will be
emailed to the library and displayed on item check in.

Patch adds two fields to the "issues" table - "note" and "notedate".
Patch adds syspref "AllowIssueNotes" - default off.

Test Plan:
1) Apply this patch
2) Update database
3) Rebuild schema
4) Turn on 'AllowIssueNote' syspref
5) Check out three different items to a borrower (may be easiest to check
    out to yourself)
6) Log in as that borrower (or yourself) on the OPAC side and go to your
summary
7) Confirm text field shows under Note column for all checkouts. Set a
note for each issue, confirm all save.
8) Check the message_queue in mysql for the entries for ALL THREE issue
notes.
9) Disable javascript in your browser
10) Refresh your summary page. Confirm that you can no longer edit the
notes in the text field. Click the 'Create/edit note' button and confirm
you are redirected to a new page.
11) Confirm that the correct title and author show for the note button
you clicked.
12) Set the note and click Submit -> confirm you are redirected
back to summary page and note is saved
13) Confirm there is a new entry in message_queue
14) Enable javascript and go back to the your checkouts page in the
staff client for the borrower you issued the items to
15) Check in TWO items
16) Confirm that the issue notes show under the "Date due" column for
the two items you checked in, and are accurate to the item (i.e. the
right issue note under the right item)
17) Go to circ/returns.pl and check in the final item using the barcode.
Confirm the issue note shows and the date is formatted correctly.

Sponsored-by: Region Halland
Comment 47 Aleisha Amohia 2017-01-29 21:17:39 UTC
(In reply to Josef Moravec from comment #45)
> Please, upgrade your templates to bootstrap 3 (see bug 16239) and 
> 
> https://wiki.koha-community.org/wiki/Interface_patterns
> http://getbootstrap.com/css/#grid

I only found one place I needed to update a button to bootstrap 3. This is ready to sign off again, unless someone else can find other places that need updating ... would really like to get this one through now.
Comment 48 Josef Moravec 2017-01-30 10:09:17 UTC
Ad 3) Rebuild schema

Change for for table issues are in your patch, but for old_issues not - probably better to be consistant here, just for more convinience testing. But not reason to fail the patch
Comment 49 Josef Moravec 2017-01-30 10:10:53 UTC
Created attachment 59641 [details] [review]
[SIGNED-OFF] Bug 14224: Allow patron notes about item shown at check in

This patch adds a "Note" input field to checked out items in the "your summary"
section. The field allows patrons to write notes about the item checked out,
such as "this DVD is scratched", "the binding was torn", etc. The note will be
emailed to the library and displayed on item check in.

Patch adds two fields to the "issues" table - "note" and "notedate".
Patch adds syspref "AllowIssueNotes" - default off.

Test Plan:
1) Apply this patch
2) Update database
3) Rebuild schema
4) Turn on 'AllowIssueNote' syspref
5) Check out three different items to a borrower (may be easiest to check
    out to yourself)
6) Log in as that borrower (or yourself) on the OPAC side and go to your
summary
7) Confirm text field shows under Note column for all checkouts. Set a
note for each issue, confirm all save.
8) Check the message_queue in mysql for the entries for ALL THREE issue
notes.
9) Disable javascript in your browser
10) Refresh your summary page. Confirm that you can no longer edit the
notes in the text field. Click the 'Create/edit note' button and confirm
you are redirected to a new page.
11) Confirm that the correct title and author show for the note button
you clicked.
12) Set the note and click Submit -> confirm you are redirected
back to summary page and note is saved
13) Confirm there is a new entry in message_queue
14) Enable javascript and go back to the your checkouts page in the
staff client for the borrower you issued the items to
15) Check in TWO items
16) Confirm that the issue notes show under the "Date due" column for
the two items you checked in, and are accurate to the item (i.e. the
right issue note under the right item)
17) Go to circ/returns.pl and check in the final item using the barcode.
Confirm the issue note shows and the date is formatted correctly.

Sponsored-by: Region Halland

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 50 Jonathan Druart 2017-01-31 09:51:52 UTC
I'd like to get feedbacks about these few questions:
1. Maybe the pref AllowIssueNotes should be renamed AllowCheckoutNotes?
2. Does everybody think that "PATRON_NOTE" template code is meaningful enough?


(In reply to Jonathan Druart from comment #41)
> Aleisha, it seems that your last patch kept the 2 subroutines in C4::Items.
> Since you are not using them, you can remove them.

Note that this is still valid, C4::Circulation::GetItemIssues is readded by your patch whereas it has been removed by bug 17679.
Comment 51 Katrin Fischer 2017-02-01 22:08:28 UTC
(In reply to Jonathan Druart from comment #50)
> I'd like to get feedbacks about these few questions:
> 1. Maybe the pref AllowIssueNotes should be renamed AllowCheckoutNotes?

I think the initial devs might not have been aware of our terminology:
https://wiki.koha-community.org/wiki/Terminology
Could we fix this later on in a separate patch later on? This has been taking a long time already.

> 2. Does everybody think that "PATRON_NOTE" template code is meaningful
> enough?

Don't have a better idea right now - it matches how it's called in the GUI.
Comment 52 Aleisha Amohia 2017-02-02 02:58:53 UTC
Created attachment 59761 [details] [review]
Bug 14224: Allow patron notes about item shown at check in

This patch adds a "Note" input field to checked out items in the "your summary"
section. The field allows patrons to write notes about the item checked out,
such as "this DVD is scratched", "the binding was torn", etc. The note will be
emailed to the library and displayed on item check in.

Patch adds two fields to the "issues" table - "note" and "notedate".
Patch adds syspref "AllowIssueNotes" - default off.

Test Plan:
1) Apply this patch
2) Update database
3) Rebuild schema
4) Turn on 'AllowIssueNote' syspref
5) Check out three different items to a borrower (may be easiest to check
    out to yourself)
6) Log in as that borrower (or yourself) on the OPAC side and go to your
summary
7) Confirm text field shows under Note column for all checkouts. Set a
note for each issue, confirm all save.
8) Check the message_queue in mysql for the entries for ALL THREE issue
notes.
9) Disable javascript in your browser
10) Refresh your summary page. Confirm that you can no longer edit the
notes in the text field. Click the 'Create/edit note' button and confirm
you are redirected to a new page.
11) Confirm that the correct title and author show for the note button
you clicked.
12) Set the note and click Submit -> confirm you are redirected
back to summary page and note is saved
13) Confirm there is a new entry in message_queue
14) Enable javascript and go back to the your checkouts page in the
staff client for the borrower you issued the items to
15) Check in TWO items
16) Confirm that the issue notes show under the "Date due" column for
the two items you checked in, and are accurate to the item (i.e. the
right issue note under the right item)
17) Go to circ/returns.pl and check in the final item using the barcode.
Confirm the issue note shows and the date is formatted correctly.

Sponsored-by: Region Halland

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 53 Aleisha Amohia 2017-02-02 03:00:54 UTC
(In reply to Jonathan Druart from comment #50)
> (In reply to Jonathan Druart from comment #41)
> > Aleisha, it seems that your last patch kept the 2 subroutines in C4::Items.
> > Since you are not using them, you can remove them.
> 
> Note that this is still valid, C4::Circulation::GetItemIssues is readded by
> your patch whereas it has been removed by bug 17679.

Fixed in the latest patch.

(In reply to Katrin Fischer from comment #51)
> (In reply to Jonathan Druart from comment #50)
> > I'd like to get feedbacks about these few questions:
> > 1. Maybe the pref AllowIssueNotes should be renamed AllowCheckoutNotes?
> 
> I think the initial devs might not have been aware of our terminology:
> https://wiki.koha-community.org/wiki/Terminology
> Could we fix this later on in a separate patch later on? This has been
> taking a long time already.

I agree with Katrin - any other additional comments about language etc. can be fixed once this has passed, but would really like to get this through. 
Reattached patch removes the GetItemIssues from the patch.
Comment 54 Jonathan Druart 2017-02-02 10:40:51 UTC
Created attachment 59779 [details] [review]
Bug 14224: Allow patron notes about item shown at check in

This patch adds a "Note" input field to checked out items in the "your summary"
section. The field allows patrons to write notes about the item checked out,
such as "this DVD is scratched", "the binding was torn", etc. The note will be
emailed to the library and displayed on item check in.

Patch adds two fields to the "issues" table - "note" and "notedate".
Patch adds syspref "AllowIssueNotes" - default off.

Test Plan:
1) Apply this patch
2) Update database
3) Rebuild schema
4) Turn on 'AllowIssueNote' syspref
5) Check out three different items to a borrower (may be easiest to check
    out to yourself)
6) Log in as that borrower (or yourself) on the OPAC side and go to your
summary
7) Confirm text field shows under Note column for all checkouts. Set a
note for each issue, confirm all save.
8) Check the message_queue in mysql for the entries for ALL THREE issue
notes.
9) Disable javascript in your browser
10) Refresh your summary page. Confirm that you can no longer edit the
notes in the text field. Click the 'Create/edit note' button and confirm
you are redirected to a new page.
11) Confirm that the correct title and author show for the note button
you clicked.
12) Set the note and click Submit -> confirm you are redirected
back to summary page and note is saved
13) Confirm there is a new entry in message_queue
14) Enable javascript and go back to the your checkouts page in the
staff client for the borrower you issued the items to
15) Check in TWO items
16) Confirm that the issue notes show under the "Date due" column for
the two items you checked in, and are accurate to the item (i.e. the
right issue note under the right item)
17) Go to circ/returns.pl and check in the final item using the barcode.
Confirm the issue note shows and the date is formatted correctly.

Sponsored-by: Region Halland

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 55 Jonathan Druart 2017-02-02 10:40:58 UTC
Created attachment 59780 [details] [review]
Bug 14224: Check if the columns do not exist before creating them

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 56 Jonathan Druart 2017-02-02 10:41:04 UTC
Created attachment 59781 [details] [review]
Bug 14224: Do not reintroduce GetItemIssue, it's dying

Bug 17680 is removing it.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 57 Jonathan Druart 2017-02-02 10:41:10 UTC
Created attachment 59782 [details] [review]
Bug 14224: Reintroduce previous copyright

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 58 Jonathan Druart 2017-02-02 10:41:15 UTC
Created attachment 59783 [details] [review]
Bug 14224: Remove SQL query in svc script

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 59 Jonathan Druart 2017-02-02 10:41:20 UTC
Created attachment 59784 [details] [review]
Bug 14224: Fix prevent submit

That did not work, the form was submitted anyway

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 60 Jonathan Druart 2017-02-02 10:41:27 UTC
Created attachment 59785 [details] [review]
Bug 14224: Make strings translatable

The strings should be translatable.
This patch also removes the error as it appears that we only have 1
error.
To improve we could surround the store with an eval.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 61 Jonathan Druart 2017-02-02 10:41:33 UTC
Created attachment 59786 [details] [review]
Bug 14224: Replace AllowIssueNotes with AllowCheckoutNotes

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 62 Jonathan Druart 2017-02-02 10:42:29 UTC
I'd like either a signoff on these follow-ups or someone else to QA.
Comment 63 Jonathan Druart 2017-02-02 10:51:26 UTC
Created attachment 59787 [details] [review]
Bug 14224: Update Copyright

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 64 Jonathan Druart 2017-02-02 10:51:31 UTC
Created attachment 59788 [details] [review]
Bug 14224: Improve escaped characters

To recreate the problem, submit a note like
  doh"doh

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 65 Jonathan Druart 2017-02-02 10:53:17 UTC
And 2 more.
I do not really care about the number of patches, someone can squashed them.

I think that makes the changes more readable and separate the different problems.
Comment 66 Josef Moravec 2017-02-02 10:58:10 UTC
(In reply to Jonathan Druart from comment #55)
> Created attachment 59780 [details] [review] [review]
> Bug 14224: Check if the columns do not exist before creating them
> 
> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Probably better to ommit "IGNORE" when using column_exists function, what do you think?
Comment 67 Josef Moravec 2017-02-02 11:10:13 UTC
(In reply to Josef Moravec from comment #66)
> (In reply to Jonathan Druart from comment #55)
> > Created attachment 59780 [details] [review] [review] [review]
> > Bug 14224: Check if the columns do not exist before creating them
> > 
> > Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
> 
> Probably better to ommit "IGNORE" when using column_exists function, what do
> you think?

But it's only think, otherwise everything looks good
Comment 68 Jonathan Druart 2017-02-02 11:24:45 UTC
Created attachment 59796 [details] [review]
Bug 14224: Check if the columns do not exist before creating them

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 69 Jonathan Druart 2017-02-02 11:25:39 UTC
(In reply to Josef Moravec from comment #66)
> (In reply to Jonathan Druart from comment #55)
> > Created attachment 59780 [details] [review] [review] [review]
> > Bug 14224: Check if the columns do not exist before creating them
> > 
> > Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
> 
> Probably better to ommit "IGNORE" when using column_exists function, what do
> you think?

Yes definitely, especially because bug 17234.
Comment 70 Josef Moravec 2017-02-02 11:43:18 UTC
Created attachment 59798 [details] [review]
Bug 14224: Do not reintroduce GetItemIssue, it's dying

Bug 17680 is removing it.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 71 Josef Moravec 2017-02-02 11:43:31 UTC
Created attachment 59799 [details] [review]
Bug 14224: Reintroduce previous copyright

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 72 Josef Moravec 2017-02-02 11:43:46 UTC
Created attachment 59800 [details] [review]
Bug 14224: Remove SQL query in svc script

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 73 Josef Moravec 2017-02-02 11:44:04 UTC
Created attachment 59801 [details] [review]
Bug 14224: Fix prevent submit

That did not work, the form was submitted anyway

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 74 Josef Moravec 2017-02-02 11:44:15 UTC
Created attachment 59802 [details] [review]
Bug 14224: Make strings translatable

The strings should be translatable.
This patch also removes the error as it appears that we only have 1
error.
To improve we could surround the store with an eval.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 75 Josef Moravec 2017-02-02 11:44:24 UTC
Created attachment 59803 [details] [review]
Bug 14224: Replace AllowIssueNotes with AllowCheckoutNotes

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 76 Josef Moravec 2017-02-02 11:44:37 UTC
Created attachment 59804 [details] [review]
Bug 14224: Update Copyright

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 77 Josef Moravec 2017-02-02 11:44:46 UTC
Created attachment 59805 [details] [review]
Bug 14224: Improve escaped characters

To recreate the problem, submit a note like
  doh"doh

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 78 Josef Moravec 2017-02-02 11:44:59 UTC
Created attachment 59806 [details] [review]
Bug 14224: Check if the columns do not exist before creating them

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 79 Josef Moravec 2017-02-02 11:46:54 UTC
(In reply to Jonathan Druart from comment #62)
> I'd like either a signoff on these follow-ups or someone else to QA.

Signed your followups
Comment 80 Jonathan Druart 2017-02-02 11:54:24 UTC
Thanks Josef!
Comment 81 Aleisha Amohia 2017-04-04 21:06:29 UTC
Created attachment 61847 [details] [review]
Bug 14224: Allow patron notes about item shown at check in

This patch adds a "Note" input field to checked out items in the "your summary"
section. The field allows patrons to write notes about the item checked out,
such as "this DVD is scratched", "the binding was torn", etc. The note will be
emailed to the library and displayed on item check in.

Patch adds two fields to the "issues" table - "note" and "notedate".
Patch adds syspref "AllowIssueNotes" - default off.

Test Plan:
1) Apply this patch
2) Update database
3) Rebuild schema
4) Turn on 'AllowIssueNote' syspref
5) Check out three different items to a borrower (may be easiest to check
    out to yourself)
6) Log in as that borrower (or yourself) on the OPAC side and go to your
summary
7) Confirm text field shows under Note column for all checkouts. Set a
note for each issue, confirm all save.
8) Check the message_queue in mysql for the entries for ALL THREE issue
notes.
9) Disable javascript in your browser
10) Refresh your summary page. Confirm that you can no longer edit the
notes in the text field. Click the 'Create/edit note' button and confirm
you are redirected to a new page.
11) Confirm that the correct title and author show for the note button
you clicked.
12) Set the note and click Submit -> confirm you are redirected
back to summary page and note is saved
13) Confirm there is a new entry in message_queue
14) Enable javascript and go back to the your checkouts page in the
staff client for the borrower you issued the items to
15) Check in TWO items
16) Confirm that the issue notes show under the "Date due" column for
the two items you checked in, and are accurate to the item (i.e. the
right issue note under the right item)
17) Go to circ/returns.pl and check in the final item using the barcode.
Confirm the issue note shows and the date is formatted correctly.

Sponsored-by: Region Halland

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 82 Aleisha Amohia 2017-04-04 21:06:38 UTC
Created attachment 61848 [details] [review]
Bug 14224: Do not reintroduce GetItemIssue, it's dying

Bug 17680 is removing it.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 83 Aleisha Amohia 2017-04-04 21:06:45 UTC
Created attachment 61849 [details] [review]
Bug 14224: Reintroduce previous copyright

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 84 Aleisha Amohia 2017-04-04 21:06:52 UTC
Created attachment 61850 [details] [review]
Bug 14224: Remove SQL query in svc script

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 85 Aleisha Amohia 2017-04-04 21:06:59 UTC
Created attachment 61851 [details] [review]
Bug 14224: Fix prevent submit

That did not work, the form was submitted anyway

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 86 Aleisha Amohia 2017-04-04 21:07:06 UTC
Created attachment 61852 [details] [review]
Bug 14224: Make strings translatable

The strings should be translatable.
This patch also removes the error as it appears that we only have 1
error.
To improve we could surround the store with an eval.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 87 Aleisha Amohia 2017-04-04 21:07:12 UTC
Created attachment 61853 [details] [review]
Bug 14224: Replace AllowIssueNotes with AllowCheckoutNotes

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 88 Aleisha Amohia 2017-04-04 21:07:18 UTC
Created attachment 61854 [details] [review]
Bug 14224: Update Copyright

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 89 Aleisha Amohia 2017-04-04 21:07:25 UTC
Created attachment 61855 [details] [review]
Bug 14224: Improve escaped characters

To recreate the problem, submit a note like
  doh"doh

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 90 Aleisha Amohia 2017-04-04 21:07:31 UTC
Created attachment 61856 [details] [review]
Bug 14224: Check if the columns do not exist before creating them

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 91 Marc Véron 2017-04-06 10:33:35 UTC
Test plan step 4): 
AllowIssueNotes should read AllowCheckoutNotes

------
 
Test plan step 7): 
Enter "Buchrücken ist beschädigt", click "Submit changes", field shows: "Buchr�cken ist besch�digt". Same with éàè etc.
Same in message "...has been saved and sent to the library."

Text seems to be OK in database (select note from issues;)

If I reload the page, text displays correctly.

------

Test plan step 17:
If I return an item with a hold placed on it, the note does not show. I see only the message about confirming hold.
Comment 92 Marc Véron 2017-04-17 05:34:21 UTC
(In reply to Marc Véron from comment #91)
> Test plan step 4): 
> AllowIssueNotes should read AllowCheckoutNotes
> 
> ------
>  
> Test plan step 7): 
> Enter "Buchrücken ist beschädigt", click "Submit changes", field shows:
> "Buchr�cken ist besch�digt". Same with éàè etc.
> Same in message "...has been saved and sent to the library."
> 
> Text seems to be OK in database (select note from issues;)
> 
> If I reload the page, text displays correctly.
> 
> ------
> 

Hi Aleisha, the issue with öäüéàè etc. can bi fixed with the following:

--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
@@ -938,6 +938,7 @@ Using this account is not recommended because some parts of Koha will not functi
                     var $self = $(this);
                     var title = $(this).parent().siblings('.title').html();
                     var $noteinput = $(this).siblings('input[name="note"]').first();
+                    var $noteformessage =  $noteinput.val();
 
                     var ajaxData = {
                         'action': 'issuenote',
@@ -957,9 +958,8 @@ Using this account is not recommended because some parts of Koha will not functi
                             $("#notesaved").removeClass("alert-error");
                             $("#notesaved").addClass("alert-info");
                             $noteinput.data('origvalue', data.note);
-                            $noteinput.val(data.note);
                             message = "<p>" + _("Your note about %s has been saved and sent to the library.").format(title) + "</p>";
-                            message += "<p style=\"font-style:italic;\">" + data.note + "</p>";
+                            message += "<p style=\"font-style:italic;\">" + $noteformessage + "</p>";
                             $self.hide();
                         } else if(data.status == 'removed') {
                             $("#notesaved").removeClass("alert-error");
Comment 93 Marc Véron 2017-04-17 06:07:38 UTC
(In reply to Marc Véron from comment #91)
> Test plan step 4): 
> AllowIssueNotes should read AllowCheckoutNotes
> 
> ------

> 
> Test plan step 17:
> If I return an item with a hold placed on it, the note does not show. I see
> only the message about confirming hold.

The problem is NOT related to this patch set. If there is a hold, the pop-up window concerning the hold hides other messages on the main screen (returns.pl).
:(
Comment 94 Marc Véron 2017-04-17 07:24:18 UTC
(In reply to Marc Véron from comment #93)
> (In reply to Marc Véron from comment #91)
> > Test plan step 4): 
> > AllowIssueNotes should read AllowCheckoutNotes
> > 
> > ------
> > Test plan step 17:
> > If I return an item with a hold placed on it, the note does not show. I see
> > only the message about confirming hold.
> 
> The problem is NOT related to this patch set. If there is a hold, the pop-up
> window concerning the hold hides other messages on the main screen
> (returns.pl).
> :(
See: Bug 18438 - Check in: Modal about holds hides important check in messages
Comment 95 Marc Véron 2017-04-17 07:43:46 UTC
Created attachment 62196 [details] [review]
Bug 14224: Allow patron notes about item shown at check in

This patch adds a "Note" input field to checked out items in the "your summary"
section. The field allows patrons to write notes about the item checked out,
such as "this DVD is scratched", "the binding was torn", etc. The note will be
emailed to the library and displayed on item check in.

Patch adds two fields to the "issues" table - "note" and "notedate".
Patch adds syspref "AllowIssueNotes" - default off.

Test Plan:
1) Apply this patch
2) Update database
3) Rebuild schema
4) Turn on 'AllowIssueNote' syspref
5) Check out three different items to a borrower (may be easiest to check
    out to yourself)
6) Log in as that borrower (or yourself) on the OPAC side and go to your
summary
7) Confirm text field shows under Note column for all checkouts. Set a
note for each issue, confirm all save.
8) Check the message_queue in mysql for the entries for ALL THREE issue
notes.
9) Disable javascript in your browser
10) Refresh your summary page. Confirm that you can no longer edit the
notes in the text field. Click the 'Create/edit note' button and confirm
you are redirected to a new page.
11) Confirm that the correct title and author show for the note button
you clicked.
12) Set the note and click Submit -> confirm you are redirected
back to summary page and note is saved
13) Confirm there is a new entry in message_queue
14) Enable javascript and go back to the your checkouts page in the
staff client for the borrower you issued the items to
15) Check in TWO items
16) Confirm that the issue notes show under the "Date due" column for
the two items you checked in, and are accurate to the item (i.e. the
right issue note under the right item)
17) Go to circ/returns.pl and check in the final item using the barcode.
Confirm the issue note shows and the date is formatted correctly.

Sponsored-by: Region Halland

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 96 Marc Véron 2017-04-17 07:44:50 UTC
Created attachment 62197 [details] [review]
Bug 14224: Do not reintroduce GetItemIssue, it's dying

Bug 17680 is removing it.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 97 Marc Véron 2017-04-17 07:45:50 UTC
Created attachment 62198 [details] [review]
Bug 14224: Reintroduce previous copyright

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 98 Marc Véron 2017-04-17 07:46:38 UTC
Created attachment 62199 [details] [review]
Bug 14224: Remove SQL query in svc script

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 99 Marc Véron 2017-04-17 07:56:47 UTC
Created attachment 62200 [details] [review]
Bug 14224: Fix prevent submit

That did not work, the form was submitted anyway

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 100 Marc Véron 2017-04-17 07:57:41 UTC
Created attachment 62201 [details] [review]
Bug 14224: Make strings translatable

The strings should be translatable.
This patch also removes the error as it appears that we only have 1
error.
To improve we could surround the store with an eval.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 101 Marc Véron 2017-04-17 07:58:42 UTC
Created attachment 62202 [details] [review]
Bug 14224: Replace AllowIssueNotes with AllowCheckoutNotes

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 102 Marc Véron 2017-04-17 07:59:59 UTC
Created attachment 62203 [details] [review]
Bug 14224: Update Copyright

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 103 Marc Véron 2017-04-17 08:01:02 UTC
Created attachment 62204 [details] [review]
Bug 14224: Improve escaped characters

To recreate the problem, submit a note like
  doh"doh

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 104 Marc Véron 2017-04-17 08:02:21 UTC
Created attachment 62205 [details] [review]
Bug 14224: Check if the columns do not exist before creating them

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 105 Marc Véron 2017-04-17 08:05:34 UTC
Tested all 10 patches together, works as expected.
I will add a follow-up patch for comment #92 (minor issue with display of äöü)
Comment 106 Marc Véron 2017-04-17 08:18:57 UTC
Created attachment 62206 [details] [review]
Bug 14224: (follow-up): Make äöü display properly after input

This patch iixes display of öäü etc. immediately after input.
To reproduce do the following for test plan step 7):
- Enter "Buchrücken ist beschädigt",
- Click "Submit changes"
Result:
- Field shows: "Buchr�cken ist besch�digt". Same with éàè etc.
- Same in message "...has been saved and sent to the library."

To test apply patch, repeat steps above, verify that text appears
correctly.

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 107 Jonathan Druart 2017-04-24 19:05:39 UTC
Created attachment 62633 [details] [review]
Bug 14224: Allow patron notes about item shown at check in

This patch adds a "Note" input field to checked out items in the "your summary"
section. The field allows patrons to write notes about the item checked out,
such as "this DVD is scratched", "the binding was torn", etc. The note will be
emailed to the library and displayed on item check in.

Patch adds two fields to the "issues" table - "note" and "notedate".
Patch adds syspref "AllowIssueNotes" - default off.

Test Plan:
1) Apply this patch
2) Update database
3) Rebuild schema
4) Turn on 'AllowIssueNote' syspref
5) Check out three different items to a borrower (may be easiest to check
    out to yourself)
6) Log in as that borrower (or yourself) on the OPAC side and go to your
summary
7) Confirm text field shows under Note column for all checkouts. Set a
note for each issue, confirm all save.
8) Check the message_queue in mysql for the entries for ALL THREE issue
notes.
9) Disable javascript in your browser
10) Refresh your summary page. Confirm that you can no longer edit the
notes in the text field. Click the 'Create/edit note' button and confirm
you are redirected to a new page.
11) Confirm that the correct title and author show for the note button
you clicked.
12) Set the note and click Submit -> confirm you are redirected
back to summary page and note is saved
13) Confirm there is a new entry in message_queue
14) Enable javascript and go back to the your checkouts page in the
staff client for the borrower you issued the items to
15) Check in TWO items
16) Confirm that the issue notes show under the "Date due" column for
the two items you checked in, and are accurate to the item (i.e. the
right issue note under the right item)
17) Go to circ/returns.pl and check in the final item using the barcode.
Confirm the issue note shows and the date is formatted correctly.

Sponsored-by: Region Halland

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 108 Jonathan Druart 2017-04-24 19:05:48 UTC
Created attachment 62634 [details] [review]
Bug 14224: Do not reintroduce GetItemIssue, it's dying

Bug 17680 is removing it.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 109 Jonathan Druart 2017-04-24 19:05:58 UTC
Created attachment 62635 [details] [review]
Bug 14224: Reintroduce previous copyright

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 110 Jonathan Druart 2017-04-24 19:06:08 UTC
Created attachment 62636 [details] [review]
Bug 14224: Remove SQL query in svc script

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 111 Jonathan Druart 2017-04-24 19:06:15 UTC
Created attachment 62637 [details] [review]
Bug 14224: Fix prevent submit

That did not work, the form was submitted anyway

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 112 Jonathan Druart 2017-04-24 19:06:23 UTC
Created attachment 62638 [details] [review]
Bug 14224: Make strings translatable

The strings should be translatable.
This patch also removes the error as it appears that we only have 1
error.
To improve we could surround the store with an eval.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 113 Jonathan Druart 2017-04-24 19:06:33 UTC
Created attachment 62639 [details] [review]
Bug 14224: Replace AllowIssueNotes with AllowCheckoutNotes

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 114 Jonathan Druart 2017-04-24 19:06:39 UTC
Created attachment 62640 [details] [review]
Bug 14224: Update Copyright

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 115 Jonathan Druart 2017-04-24 19:06:46 UTC
Created attachment 62641 [details] [review]
Bug 14224: Improve escaped characters

To recreate the problem, submit a note like
  doh"doh

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 116 Jonathan Druart 2017-04-24 19:06:53 UTC
Created attachment 62642 [details] [review]
Bug 14224: Check if the columns do not exist before creating them

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 117 Jonathan Druart 2017-04-24 19:07:00 UTC
Created attachment 62643 [details] [review]
Bug 14224: Use encode_json instead of to_json

To handle unicode characters properly
Comment 118 Jonathan Druart 2017-04-24 19:07:08 UTC
Created attachment 62644 [details] [review]
Bug 14424: DBIC Schema changes
Comment 119 Jonathan Druart 2017-04-24 19:08:27 UTC
(In reply to Marc Véron from comment #106)
> Created attachment 62206 [details] [review] [review]
> Bug 14224: (follow-up): Make äöü display properly after input

Marc, This patch worked but actually could hide a problem. The issue was in the svc script, encode_json must have been used.
Comment 120 Kyle M Hall 2017-04-28 13:13:31 UTC
Pushed to master for 17.05, thanks Jonathan!
Comment 121 Katrin Fischer 2017-05-13 12:34:08 UTC
This won't get ported back to 16.11.x as it is a new feature
Comment 122 Marcel de Rooy 2017-07-07 07:17:52 UTC
<<borrowers.firstname>> <<borrowers.surname>> has added a note to the item <<biblio.item>> - <<biblio.author>> (<<biblio.biblionumber>>).

What is biblio.item supposed to print here?
I got this mail:
M De Rooy has added a note to the item  - Schwartz, Randal L (23).

And must say that it might not be that clear..
Do we need biblio.title and an item number ?
Comment 123 Aleisha Amohia 2017-07-09 22:23:38 UTC
(In reply to Marcel de Rooy from comment #122)
> <<borrowers.firstname>> <<borrowers.surname>> has added a note to the item
> <<biblio.item>> - <<biblio.author>> (<<biblio.biblionumber>>).
> 
> What is biblio.item supposed to print here?
> I got this mail:
> M De Rooy has added a note to the item  - Schwartz, Randal L (23).
> 
> And must say that it might not be that clear..
> Do we need biblio.title and an item number ?

Thank you for noticing this bug - I have made a new bug report to fix this at Bug 18915
Comment 124 Kelly McElligott 2017-10-19 18:22:23 UTC
We had a question about what email this note would be emailed to- the Kohaadminemail set up in System preferences or the Item's Home Branch's email set up in Libraries and Groups.  However, when testing this new system preference "allowcheckoutnotes' it appears that the patron that left the note receives the email not the librarian.  Can you verify that this is the case.?
Comment 125 Jonathan Druart 2017-10-19 20:07:32 UTC
(In reply to Kelly McElligott from comment #124)
> We had a question about what email this note would be emailed to- the
> Kohaadminemail set up in System preferences or the Item's Home Branch's
> email set up in Libraries and Groups.  However, when testing this new system
> preference "allowcheckoutnotes' it appears that the patron that left the
> note receives the email not the librarian.  Can you verify that this is the
> case.?

Yes that sounds like a bug, could you report it?
Comment 126 Christopher Brannon 2018-01-31 19:40:47 UTC
(In reply to Jonathan Druart from comment #125)
> (In reply to Kelly McElligott from comment #124)
> > We had a question about what email this note would be emailed to- the
> > Kohaadminemail set up in System preferences or the Item's Home Branch's
> > email set up in Libraries and Groups.  However, when testing this new system
> > preference "allowcheckoutnotes' it appears that the patron that left the
> > note receives the email not the librarian.  Can you verify that this is the
> > case.?
> 
> Yes that sounds like a bug, could you report it?

I can confirm this is the case.  :/
Comment 127 Aleisha Amohia 2018-01-31 20:22:34 UTC
(In reply to Christopher Brannon from comment #126)
> (In reply to Jonathan Druart from comment #125)
> > (In reply to Kelly McElligott from comment #124)
> > > We had a question about what email this note would be emailed to- the
> > > Kohaadminemail set up in System preferences or the Item's Home Branch's
> > > email set up in Libraries and Groups.  However, when testing this new system
> > > preference "allowcheckoutnotes' it appears that the patron that left the
> > > note receives the email not the librarian.  Can you verify that this is the
> > > case.?
> > 
> > Yes that sounds like a bug, could you report it?
> 
> I can confirm this is the case.  :/

I believe this has been fixed in Bug 19496!