Bug 9261 - Allow librarians to make purchase suggestions for patrons
Summary: Allow librarians to make purchase suggestions for patrons
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Jonathan Druart
URL:
Keywords:
: 8527 9777 (view as bug list)
Depends on:
Blocks: 11802 12332
  Show dependency treegraph
 
Reported: 2012-12-11 14:43 UTC by Kyle M Hall
Modified: 2014-06-18 15:19 UTC (History)
9 users (show)

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


Attachments
Bug 9261 - Allow librarians to make purchase suggestions for patrons (11.43 KB, patch)
2012-12-11 14:46 UTC, Kyle M Hall
Details | Diff | Splinter Review
Verification of Previous Patch. OK. (11.52 KB, patch)
2013-01-16 00:57 UTC, Corey
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup (8.13 KB, patch)
2013-01-17 15:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup (9.21 KB, patch)
2013-01-17 16:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons (11.45 KB, patch)
2013-03-21 11:10 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup (8.85 KB, patch)
2013-03-21 11:11 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons (11.50 KB, patch)
2013-07-31 11:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup (8.79 KB, patch)
2013-07-31 11:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup 2 (2.47 KB, patch)
2013-07-31 14:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup 2 (2.49 KB, patch)
2013-07-31 14:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup 3 (905 bytes, patch)
2013-09-05 16:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup 4 (846 bytes, patch)
2013-11-08 17:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons (11.59 KB, patch)
2013-11-13 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup (8.88 KB, patch)
2013-11-13 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup 2 (2.55 KB, patch)
2013-11-13 11:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup 3 (969 bytes, patch)
2013-11-13 11:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup 4 (891 bytes, patch)
2013-11-13 11:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9261: QA followup: ergonomic improvement and GPL version change (4.18 KB, patch)
2013-11-13 11:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9261: Title is required when adding a new suggestion (1.89 KB, patch)
2013-11-13 11:09 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2012-12-11 14:43:21 UTC
It would be nice if librarians could make purchase suggestions in the name of a give patron.

Some reasons we've been given:
  * We have many patrons in rural areas that do not have computers/Internet. 
  * Others are homebound as well. They receive items through our mailorder service and place orders exclusively by phone or mail.
Comment 1 Kyle M Hall 2012-12-11 14:46:51 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2012-12-11 14:48:55 UTC
*** Bug 8527 has been marked as a duplicate of this bug. ***
Comment 3 Corey 2013-01-16 00:57:25 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2013-01-16 14:25:08 UTC
QA comments:

The patch is clean and works well.
But I have some remarks:
 - template_name   => "members/purchase-suggestions.tmpl" could be .tt
 - in suggestion.pl, %hashlists is passed twice.
 - in suggestion.pl, $redirect could be eq to "purcharse-suggestion" with a new param borrowernumber=[%borrowernumber%] in order to open the possibility for an other script to use this redirection.
 - in purchase-suggestion.tt, some includes are useless: calendar.inc, tablesorter.js
 - The new tab is sometimes displayed on 2 lines and sometimes on 1 lines depending on which tab is enabled. I don't know how to fix that...
 - on the way, could you fix 4 withespaces:
   * suggestion/suggestion.pl                                                 FAIL
    forbidden patterns          FAIL
      The patch introduces a forbidden pattern: withespace character  (77)
      The patch introduces a forbidden pattern: withespace character  (138)
   * koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt  FAIL
    forbidden patterns          FAIL
      The patch introduces a forbidden pattern: withespace character  (71)
      The patch introduces a forbidden pattern: withespace character  (62)

Marked as Failed QA
Comment 5 Kyle M Hall 2013-01-17 15:44:43 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2013-01-17 16:05:02 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2013-01-17 16:07:43 UTC
>  - template_name   => "members/purchase-suggestions.tmpl" could be .tt

Fixed!

>  - in suggestion.pl, %hashlists is passed twice.

Fixed!

>  - in suggestion.pl, $redirect could be eq to "purcharse-suggestion" with a
> new param borrowernumber=[%borrowernumber%] in order to open the possibility
> for an other script to use this redirection.

Fixed!

>  - in purchase-suggestion.tt, some includes are useless: calendar.inc,
> tablesorter.js

Fixed!

>  - The new tab is sometimes displayed on 2 lines and sometimes on 1 lines
> depending on which tab is enabled. I don't know how to fix that...

This is because the tabs are not the same width on every page. Sometimes they are wide enough to get both words on one line, sometimes they are not. I've fixed this by forcing each word to always be on a separate line.

>  - on the way, could you fix 4 withespaces:
>    * suggestion/suggestion.pl                                               
> FAIL
>     forbidden patterns          FAIL
>       The patch introduces a forbidden pattern: withespace character  (77)
>       The patch introduces a forbidden pattern: withespace character  (138)
>    * koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt
> FAIL
>     forbidden patterns          FAIL
>       The patch introduces a forbidden pattern: withespace character  (71)
>       The patch introduces a forbidden pattern: withespace character  (62)
> 

Fixed!
Comment 8 Jonathan Druart 2013-02-28 13:57:09 UTC
Could you rebase once again please (and perhaps squash these 2 patch into 1) ?
Comment 9 Brendan Gallagher 2013-03-21 11:10:46 UTC Comment hidden (obsolete)
Comment 10 Brendan Gallagher 2013-03-21 11:11:07 UTC Comment hidden (obsolete)
Comment 11 Nicole C. Engard 2013-03-21 16:56:47 UTC
*** Bug 9777 has been marked as a duplicate of this bug. ***
Comment 12 Srdjan Jankovic 2013-06-11 05:28:46 UTC
I got
Can't use an undefined value as an ARRAY reference at /usr/lib/perl5/DBI.pm line 2054.
when clicking on the "New purchase suggestion" button (empty list)
Comment 13 Kyle M Hall 2013-07-31 11:54:43 UTC Comment hidden (obsolete)
Comment 14 Kyle M Hall 2013-07-31 11:55:06 UTC Comment hidden (obsolete)
Comment 15 Owen Leonard 2013-07-31 14:18:04 UTC
A couple of issues:

- The template tries to build an old YUI-style button instead of 
  the current Bootstrap version.

- The table of suggestions should link to the suggestions detail 
  page for that entry if the user has the correct permissions.
  (/cgi-bin/koha/suggestion/suggestion.pl?suggestionid=X&op=show
  I assume, a page which should probably be made more useful somehow).
Comment 16 Kyle M Hall 2013-07-31 14:54:44 UTC Comment hidden (obsolete)
Comment 17 Kyle M Hall 2013-07-31 14:56:39 UTC Comment hidden (obsolete)
Comment 18 Owen Leonard 2013-07-31 19:59:01 UTC
This latest patch doesn't remove the YUI JavaScript, so even though the button looks correct there is still a JS error on the page.
Comment 19 Kyle M Hall 2013-09-05 16:38:31 UTC Comment hidden (obsolete)
Comment 20 Katrin Fischer 2013-09-14 13:21:05 UTC
Hi Kyle,

starting with the tests and a first code review:

Would you be ok updating to GPLv3+?
FAIL	members/purchase-suggestions.pl
   OK	  pod
   FAIL	  forbidden patterns
		forbidden pattern: Koha is now under the GPLv3 license (line 9)

Is this still relevant?
+    DHTMLcalendar_dateformat => C4::Dates->DHTMLcalendar(),

I wonder if we need a way to allow for disabling this functionality in the patron account or a way to differentiate between suggestions created by the user in the OPAC and by a staff member. And not failing for this - but so you can take a look at the other 2 things.
Comment 21 mathieu saby 2013-10-13 09:19:45 UTC
Hi Kyle,
this patch would be very interesting for us.
Do you think you can fix the issues raised by Katrin?

For the idea of making a distinction between 2 kinds of suggestion, I don't think it is necessary. If a patron fill a printed suggestion form and if we copy it in Koha (that's the way we could use it, to manage all our suggestions in Koha), he won't be surprised to see this suggestion in OPAC.

M. Saby
Comment 22 Kyle M Hall 2013-11-08 17:24:34 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2013-11-13 11:04:27 UTC
QA comment:
All works as expected, no bug found.

Maybe the redirect stuff could be used when clicking on a suggestion from the patron page (with a link "back to the patron's suggestions", instead of "Back to suggestions"). But it is not blocker.

2 errors from qa tools:
 FAIL	members/purchase-suggestions.pl
   FAIL	  forbidden patterns
		forbidden pattern: Koha is now under the GPLv3 license (line 9)

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt
   FAIL	  forbidden patterns
		forbidden pattern: To include datatables files, please use the include file (see bug 10868) (line 11)

Will be fixed in a followup. Please Kyle protests if you disagree with the GPL version change (I only write that for the sake of form, I know you will agree :)).

Marked as Passed QA.
Comment 24 Jonathan Druart 2013-11-13 11:08:38 UTC
Created attachment 22897 [details] [review]
Bug 9261 - Allow librarians to make purchase suggestions for patrons

This patch adds a new tab to the patron side menu for purchase suggestions.
From this new tab, a librarian can view the patron's existing purchase suggestios
and also create new suggestions in the name of that patron.

Test Plan:
1) Apply patch
2) Ensure the system preference 'suggestions' is enabled
3) View the details for a patron
4) Click the new 'Purchase suggestions' tab
5) Click the 'New purchase suggestion' button
6) Add the new purchase suggestions
7) You should now end up back at the borrowers purchase suggetions
8) Verify the new purchase suggestion was added

Signed-off-by: Corey Fuimaono <agent.075@gmail.com>

Step though the test plan. All OK.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 25 Jonathan Druart 2013-11-13 11:08:57 UTC
Created attachment 22898 [details] [review]
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 26 Jonathan Druart 2013-11-13 11:09:03 UTC
Created attachment 22899 [details] [review]
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup 2

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 27 Jonathan Druart 2013-11-13 11:09:08 UTC
Created attachment 22900 [details] [review]
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup 3

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 28 Jonathan Druart 2013-11-13 11:09:13 UTC
Created attachment 22901 [details] [review]
Bug 9261 - Allow librarians to make purchase suggestions for patrons - QA Followup 4

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 29 Jonathan Druart 2013-11-13 11:09:17 UTC
Created attachment 22902 [details] [review]
Bug 9261: QA followup: ergonomic improvement and GPL version change

This patch:
- changes the GPL version from 2 to 3.
- uses the datatables.inc file
- removes the single pixel before and after the DT (modifying the sDom
  value).

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 30 Jonathan Druart 2013-11-13 11:09:23 UTC
Created attachment 22903 [details] [review]
Bug 9261: Title is required when adding a new suggestion

I don't know if it was a bug but at the OPAC, title is a required field
and not on the staff interface.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 31 Galen Charlton 2013-11-15 00:27:12 UTC
Pushed to master.  Thanks, Kyle!