Bug 9032 - Share a list
Summary: Share a list
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Lists (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Marcel de Rooy
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 8262 11943 20687
  Show dependency treegraph
 
Reported: 2012-11-08 13:59 UTC by Marcel de Rooy
Modified: 2018-05-01 11:19 UTC (History)
7 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
Bug 9032: Share a list: OPAC foundation (4.78 KB, patch)
2013-05-20 16:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Followup for new dependency Email::Valid (908 bytes, patch)
2013-05-31 10:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Share a list (dbrev for SHARE_INVITE notice) (2.99 KB, patch)
2013-05-31 10:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Share a list (dbrev for SHARE_INVITE notice) (2.99 KB, patch)
2013-05-31 14:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Followup for new dependency Email::Valid (1.04 KB, patch)
2013-05-31 14:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Share a list (part 1: send the invitation) (14.85 KB, patch)
2013-06-01 08:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Share a list (part 1: send the invitation) (15.83 KB, patch)
2013-06-03 11:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032 [Template follow-up] Share a list (part 1: send the invitation) (3.50 KB, patch)
2013-06-05 16:27 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED_OFF] Bug 9032 [Template follow-up] Share a list (part 1: send the invitation) (3.56 KB, patch)
2013-06-06 06:40 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9032: Share a list (dbrev for SHARE_INVITE notice) (3.02 KB, patch)
2013-07-03 15:52 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9032: Followup for new dependency Email::Valid (1.08 KB, patch)
2013-07-03 15:55 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9032: Share a list (part 1: send the invitation) (15.89 KB, patch)
2013-07-03 16:02 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9032 [Template follow-up] Share a list (part 1: send the invitation) (3.56 KB, patch)
2013-07-04 09:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Share a list (part 1: send the invitation) (15.70 KB, patch)
2013-07-25 07:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032 [Template follow-up] Share a list (part 1: send the invitation) (3.56 KB, patch)
2013-07-25 07:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Share a list (database revision for notices) (4.42 KB, patch)
2014-03-05 14:27 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Followup for new dependency Email::Valid (1.17 KB, patch)
2014-03-05 14:28 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Share a list (part 1: send the invitation) (15.59 KB, patch)
2014-03-05 14:34 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032 [Template follow-up] Share a list (part 1: send the invitation) (3.60 KB, patch)
2014-03-05 14:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: [QA Follow-up] Plackify opac-shareshelf.pl (10.33 KB, patch)
2014-03-05 14:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Share a list (accept invitation and remove a share) (24.43 KB, patch)
2014-03-05 14:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Unit tests for sharing lists (9.17 KB, patch)
2014-03-05 14:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Consistent behavior when deleting lists (3.78 KB, patch)
2014-03-05 15:55 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Consistent behavior when deleting lists (4.12 KB, patch)
2014-03-06 09:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Follow-up for bootstrap theme (13.05 KB, patch)
2014-03-06 13:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Share a list (database revision for notices) (4.39 KB, patch)
2014-03-13 11:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Followup for new dependency Email::Valid (1.18 KB, patch)
2014-03-13 14:33 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 9032: Followup for new dependency Email::Valid (1.18 KB, patch)
2014-03-13 14:36 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 9032: Share a list (part 1: send the invitation) (15.61 KB, patch)
2014-03-13 14:37 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 9032 [Template follow-up] Share a list (part 1: send the invitation) (3.61 KB, patch)
2014-03-13 14:37 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 9032: [QA Follow-up] Plackify opac-shareshelf.pl (10.34 KB, patch)
2014-03-13 14:37 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 9032: Share a list (accept invitation and remove a share) (24.44 KB, patch)
2014-03-13 14:38 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 9032: Unit tests for sharing lists (9.18 KB, patch)
2014-03-13 14:38 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 9032: Consistent behavior when deleting lists (4.13 KB, patch)
2014-03-13 14:38 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 9032: Follow-up for bootstrap theme (13.06 KB, patch)
2014-03-13 14:38 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 9032: Share a list (database revision for notices) (4.40 KB, patch)
2014-03-13 14:38 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 9032: Followup for new dependency Email::Valid (1.18 KB, patch)
2014-03-13 14:44 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 9032: Share a list (part 1: send the invitation) (15.61 KB, patch)
2014-03-13 14:45 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 9032: QA Follow-up: Send cookie with redirect (1.22 KB, patch)
2014-03-14 14:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: QA Follow-up: Use loop variable instead of $_ in opac-shareshelf (859 bytes, patch)
2014-03-19 15:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: QA Follow-up: Loop variable and Cancel button (3.62 KB, patch)
2014-03-21 11:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: QA Follow-up: Adding notices to foreign installer files (20.31 KB, patch)
2014-03-21 15:09 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9032: Share a list (part 1: send the invitation) (15.67 KB, patch)
2014-03-26 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9032: Followup for new dependency Email::Valid (1.23 KB, patch)
2014-03-26 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9032 [Template follow-up] Share a list (part 1: send the invitation) (3.67 KB, patch)
2014-03-26 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9032: [QA Follow-up] Plackify opac-shareshelf.pl (10.41 KB, patch)
2014-03-26 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9032: Share a list (accept invitation and remove a share) (24.50 KB, patch)
2014-03-26 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9032: Unit tests for sharing lists (9.24 KB, patch)
2014-03-26 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9032: Consistent behavior when deleting lists (4.19 KB, patch)
2014-03-26 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9032: Follow-up for bootstrap theme (13.12 KB, patch)
2014-03-26 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9032: Share a list (database revision for notices) (4.45 KB, patch)
2014-03-26 14:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9032: QA Follow-up: Send cookie with redirect (1.24 KB, patch)
2014-03-26 14:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9032: QA Follow-up: Loop variable and Cancel button (3.67 KB, patch)
2014-03-26 14:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9032: QA Follow-up: Adding notices to foreign installer files (20.32 KB, patch)
2014-03-26 14:53 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 Marcel de Rooy 2012-11-08 13:59:38 UTC
Bug 7310 already included the following point (among others):
Add a new (opac) feature to private lists: Share a list (with another patron).
Let the user share access to a list by Koha sending an email with a URL including some (temporary) invitation key. 
When the invited patron clicks that URL (when logged in), he gains access (in accordance with the described permission options for that specific list).
The invited patron can always 'delete' the shared list, i.e. delete the share.
The owner can 'unshare' the list and remove all shares for that list.

This report is created for this point only.
Comment 1 Marcel de Rooy 2013-05-20 16:12:11 UTC
(In reply to comment #0)
> The owner can 'unshare' the list and remove all shares for that list.

I am having second thoughts about this part of the functionality. I would rather opt now for copy and delete without unsharing.
Comment 2 Marcel de Rooy 2013-05-20 16:15:11 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2013-05-20 16:18:10 UTC
Please wait for some more patches :)
Comment 4 Marcel de Rooy 2013-05-31 10:47:08 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2013-05-31 10:47:12 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2013-05-31 14:11:55 UTC
Comment from Robin on dev list:

On this note, it's also best to use modules that are packaged in Debian
squeeze (which I figure we'll try to support as long as we can) where
possible, just to save me the work in making packages for them :) Also,
cpan modules that have no license that allows us to redistribute them
are a no-no. This doesn't apply to libemail-valid-perl however, it's in
squeeze already.
Comment 7 Marcel de Rooy 2013-05-31 14:15:42 UTC Comment hidden (obsolete)
Comment 8 Marcel de Rooy 2013-05-31 14:15:46 UTC Comment hidden (obsolete)
Comment 9 Marcel de Rooy 2013-06-01 08:00:21 UTC Comment hidden (obsolete)
Comment 10 Marcel de Rooy 2013-06-01 08:13:19 UTC
As part of ongoing development, these three patches are imho ready for at least signoff. They should not be pushed without the followup on bug 10389.
This allows for gradual test and qa on smaller patches.

I would also welcome followups on getting the text but especially the design on a higher level; I am no expert in that area :)

Thanks.
Comment 11 Marcel de Rooy 2013-06-03 11:50:25 UTC Comment hidden (obsolete)
Comment 12 Marcel de Rooy 2013-06-03 11:51:16 UTC
Small adjustment: one line for debugging purposes removed. Adds Share button on view private shelf form.
Comment 13 Owen Leonard 2013-06-05 16:27:49 UTC Comment hidden (obsolete)
Comment 14 Owen Leonard 2013-06-05 16:32:11 UTC
I see now that you can enter multiple email addresses when sharing. Text should be added to make that clear, including specifying what separator is required (a comma works, what about something else?).

How will the sharing feature handle a case where two patrons have the same email address? We find that this happens with families with children--the parent will put their email address on the records of each of their children.
Comment 15 Marcel de Rooy 2013-06-06 06:40:05 UTC Comment hidden (obsolete)
Comment 16 Marcel de Rooy 2013-06-06 07:02:57 UTC
(In reply to comment #14)
> I see now that you can enter multiple email addresses when sharing. Text
> should be added to make that clear, including specifying what separator is
> required (a comma works, what about something else?).
> 
> How will the sharing feature handle a case where two patrons have the same
> email address? We find that this happens with families with children--the
> parent will put their email address on the records of each of their children.

Hi Owen,
Thanks for testing and followup.
What about one line under the form like: You can enter multiple email addresses, if you separate them by semicolons.
Note: I used [,;:] in the code, but was having doubts since some of these chars actually are allowed in an email address when placed between quotes. The semicolon seems to be used more often as separator between mail addresses.

The share feature uses the email addresses given. If an address is repeated, two mails will be sent. The address used does not need to be the patron email address. Crucial in the acceptance is that you confirm with your login. Seems more flexible and 'secure'.
The children in your example just need to accept with their own login.
Comment 17 Owen Leonard 2013-07-03 15:52:46 UTC Comment hidden (obsolete)
Comment 18 Owen Leonard 2013-07-03 15:55:21 UTC Comment hidden (obsolete)
Comment 19 Owen Leonard 2013-07-03 16:02:23 UTC Comment hidden (obsolete)
Comment 20 Owen Leonard 2013-07-03 16:03:42 UTC
Note to QA: Because of the order in which the patches were signed off the first one, "[Template follow-up] Share a list (part 1: send the invitation)" should be applied last.
Comment 21 Marcel de Rooy 2013-07-04 09:56:20 UTC Comment hidden (obsolete)
Comment 22 Marcel de Rooy 2013-07-04 10:02:06 UTC
(In reply to Owen Leonard from comment #20)
> Note to QA: Because of the order in which the patches were signed off the
> first one, "[Template follow-up] Share a list (part 1: send the invitation)"
> should be applied last.
I have rearranged the order.
Thanks Owen for testing.
QA: These patches are ready for QA, separate from the followup reports. They can stay in the QA queue, if they pass, until at least part2 comes along (bug 10389). Will submit patches for that soon.
Part 3 will handle the orphaned lists and permissions for taking ownership, moderating public/shared list names.
Comment 23 Marcel de Rooy 2013-07-25 07:29:16 UTC Comment hidden (obsolete)
Comment 24 Marcel de Rooy 2013-07-25 07:29:24 UTC Comment hidden (obsolete)
Comment 25 Marcel de Rooy 2013-07-25 07:30:20 UTC
Rebased the last two patches.
Comment 26 Jonathan Druart 2013-08-20 14:22:37 UTC
QA comment:

The code:
- Perltidies the new file please.
- Use the CPLv3 license:
  FAIL	opac/opac-shareshelf.pl
   FAIL	  forbidden patterns
     forbidden pattern: Koha is now under the GPLv3 license (line 9)
- The new letter should be added to others languages.
- The new pl file contains a lot of routines. Some of them use global variables which are not defined as global.
- Maybe SHARE_INVITATION_EXPIRY_DAYS should be a new syspref? Hum... or not.
- Note that ADDDATE is a mysqlism, but Koha doesn't offer a better way to do.

The ergonomic:
- The page opac-shelves.pl?display=privateshelves displays a link for editing and deleting, but a button for sharing. Is it intentional?
- There is no back button if I enter a bad email address.

The feature:
The invitation is sent to the message queue. The doc suggests process_message_queue.pl runs every 1 to 4 hours. We can imagine that a user tells to his friend: "I just share a list with you. You will receive an email!". The friend refresh, refresh, refresh his inbox and nothing append. The guy will resent the invitation again and again :)
I have 2 suggestions:
* directly use the sendmail command.
* Add a check: if an invitation for this list and this email address has been sent some time ago, an alert is raised and no new invitation is sent.

Marked as Failed QA.
Comment 27 Galen Charlton 2013-08-20 19:39:08 UTC
(In reply to Jonathan Druart from comment #26)
> The invitation is sent to the message queue. The doc suggests
> process_message_queue.pl runs every 1 to 4 hours. We can imagine that a user
> tells to his friend: "I just share a list with you. You will receive an
> email!". The friend refresh, refresh, refresh his inbox and nothing append.
> The guy will resent the invitation again and again :)
> I have 2 suggestions:
> * directly use the sendmail command.

I disagree with this.  There is nothing preventing process_message_queue.pl from being run more frequently; in fact, the crontab that goes with the Debian package runs it every 15 minutes -- and it would be reasonable for a library to run it more frequently.  I would be loath to give up the centralization and templating options that the message queue gives us.

I see that crontab.example does need to be updated, though: it runs the message queue every hour, and it probably wouldn't hurt to bump up the frequencye.

> * Add a check: if an invitation for this list and this email address has
> been sent some time ago, an alert is raised and no new invitation is sent.

This would be reasonable.
Comment 28 Owen Leonard 2013-08-20 19:53:12 UTC
(In reply to Galen Charlton from comment #27)
> I disagree with this.  There is nothing preventing process_message_queue.pl
> from being run more frequently;

This is the same issue that came up in discussions of Bug 7067 - allow patron self registration via the opac. If your patrons are registering through the OPAC and expecting a confirmation email you would want process_message_queue.pl to run as often as once a minute. The snag is that then your message queue "digest" options don't work because the digest never has the time to accumulate messages.
Comment 29 Galen Charlton 2013-08-20 20:15:38 UTC
(In reply to Owen Leonard from comment #28)
> (In reply to Galen Charlton from comment #27)
> > I disagree with this.  There is nothing preventing process_message_queue.pl
> > from being run more frequently;
> 
> This is the same issue that came up in discussions of Bug 7067 - allow
> patron self registration via the opac. If your patrons are registering
> through the OPAC and expecting a confirmation email you would want
> process_message_queue.pl to run as often as once a minute. The snag is that
> then your message queue "digest" options don't work because the digest never
> has the time to accumulate messages.

Fair point.  One possibility would be to allow certain message types be marked as high-priority (which would include the interpretation that they're not meant to ever be digested), then add a switch to process_message_queue.pl to have it process just the high-priority ones.  I'm willing to put a bit of time into that if folks are amenable to this approach.
Comment 30 Katrin Fischer 2013-08-20 20:45:31 UTC
I think the only messages digested like Owen describes are the checkin and checkout slips - so maybe we could turn it around and have a setting to only send those out every x minutes? I think the other mails could be sent out pretty immediately after being generated without problems.
Comment 31 Marcel de Rooy 2013-09-18 11:56:31 UTC
(In reply to Jonathan Druart from comment #26)
First of all, thanks for your QA, although the results were negative :)

> - The new letter should be added to others languages.
I would really get rid of that restriction. I started some development on that (look for i18n). But unfortunately there is not that much support for it.

> - The new pl file contains a lot of routines. Some of them use global
> variables which are not defined as global.
Please explain.

> Marked as Failed QA.
If this should really fail for the reasons given, I will have to let this development rest for some time. I do not have the time to keep everything in the air now ;) Note that this impacts also unit tests and the accept a share-followup reports.
Comment 32 Jonathan Druart 2013-09-19 09:00:45 UTC
(In reply to M. de Rooy from comment #31)
> (In reply to Jonathan Druart from comment #26)
> First of all, thanks for your QA, although the results were negative :)
> 
> > - The new letter should be added to others languages.
> I would really get rid of that restriction. I started some development on
> that (look for i18n). But unfortunately there is not that much support for
> it.

So should this patch depends on them?

> > - The new pl file contains a lot of routines. Some of them use global
> > variables which are not defined as global.
> Please explain.

If you use a global variable in a routine, you should declare it as 'our' instead of 'my'.

> > Marked as Failed QA.
> If this should really fail for the reasons given, I will have to let this
> development rest for some time. I do not have the time to keep everything in
> the air now ;) Note that this impacts also unit tests and the accept a
> share-followup reports.
Comment 33 Marcel de Rooy 2013-09-19 10:45:55 UTC
(In reply to Jonathan Druart from comment #32)
> If you use a global variable in a routine, you should declare it as 'our'
> instead of 'my'.
These variables are only used in some subroutines of this script, eliminating the need for passing them as parameters. I do not need them to be (global) package variables. A lexical variable declared with my is sufficient here.
Comment 34 Jonathan Druart 2013-09-19 12:34:06 UTC
(In reply to M. de Rooy from comment #33)
> (In reply to Jonathan Druart from comment #32)
> > If you use a global variable in a routine, you should declare it as 'our'
> > instead of 'my'.
> These variables are only used in some subroutines of this script,
> eliminating the need for passing them as parameters. I do not need them to
> be (global) package variables. A lexical variable declared with my is
> sufficient here.

That does not work with Plack.
Moreover, using global variables is not a good practice.
Comment 35 Jonathan Druart 2013-09-19 12:36:33 UTC
With Plack, on opac-shareshelf.pl?shelfnumber=1, I got an error 500 (internal server error) with the following in logs:
Variable "$op" is not available at /home/koha/src/opac/opac-shareshelf.pl line 70.
Variable "$errcode" is not available at /home/koha/src/opac/opac-shareshelf.pl line 71.
Variable "$shelfnumber" is not available at /home/koha/src/opac/opac-shareshelf.pl line 74.
Variable "@addr" is not available at /home/koha/src/opac/opac-shareshelf.pl line 91.
Variable "$errcode" is not available at /home/koha/src/opac/opac-shareshelf.pl line 95.
Variable "$addrlist" is not available at /home/koha/src/opac/opac-shareshelf.pl line 104.
Variable "$fail_addr" is not available at /home/koha/src/opac/opac-shareshelf.pl line 105.
Variable "@addr" is not available at /home/koha/src/opac/opac-shareshelf.pl line 110.
Variable "$shelfnumber" is not available at /home/koha/src/opac/opac-shareshelf.pl line 121.
Variable "@addr" is not available at /home/koha/src/opac/opac-shareshelf.pl line 125.
Variable "@newkey" is not available at /home/koha/src/opac/opac-shareshelf.pl line 126.
Variable "$loggedinuser" is not available at /home/koha/src/opac/opac-shareshelf.pl line 133.
Variable "$shelfname" is not available at /home/koha/src/opac/opac-shareshelf.pl line 135.
Variable "$shelfname" is not available at /home/koha/src/opac/opac-shareshelf.pl line 154.
Variable "$owner" is not available at /home/koha/src/opac/opac-shareshelf.pl line 154.
Variable "$shelfnumber" is not available at /home/koha/src/opac/opac-shareshelf.pl line 154.
Variable "$errcode" is not available at /home/koha/src/opac/opac-shareshelf.pl line 155.
Variable "$loggedinuser" is not available at /home/koha/src/opac/opac-shareshelf.pl line 155.
Variable "$template" is not available at /home/koha/src/opac/opac-shareshelf.pl line 162.
Variable "$loggedinuser" is not available at /home/koha/src/opac/opac-shareshelf.pl line 162.
Variable "$cookie" is not available at /home/koha/src/opac/opac-shareshelf.pl line 162.
Variable "$query" is not available at /home/koha/src/opac/opac-shareshelf.pl line 164.
Variable "$template" is not available at /home/koha/src/opac/opac-shareshelf.pl line 171.
Variable "$errcode" is not available at /home/koha/src/opac/opac-shareshelf.pl line 172.
Variable "$op" is not available at /home/koha/src/opac/opac-shareshelf.pl line 173.
Variable "$shelfnumber" is not available at /home/koha/src/opac/opac-shareshelf.pl line 174.
Variable "$shelfname" is not available at /home/koha/src/opac/opac-shareshelf.pl line 175.
Variable "@addr" is not available at /home/koha/src/opac/opac-shareshelf.pl line 176.
Variable "$fail_addr" is not available at /home/koha/src/opac/opac-shareshelf.pl line 177.
Variable "@base64alphabet" is not available at /home/koha/src/opac/opac-shareshelf.pl line 225.
no query in gettemplate at /home/koha/src/C4/Templates.pm line 232.
no query in themelanguage at /home/koha/src/C4/Templates.pm line 282.
Can't call method "cookie" on an undefined value at /home/koha/src/C4/Templates.pm line 331.
Comment 36 Marcel de Rooy 2013-09-19 12:49:29 UTC
(In reply to Jonathan Druart from comment #34)
> That does not work with Plack.
> Moreover, using global variables is not a good practice.

Thanks. I was somewhat expecting to hear Plack coming :)
We should add this to the coding guidelines. It is not there yet.
For the record, I did not use global variables (my variables are lexical). Do you have a reference to a best practice or so about not using lexical variables on 'root level'? This is not in the coding guidelines too?
Comment 37 Marcel de Rooy 2013-09-19 13:15:29 UTC
Jonathan,
You convinced me for the Plack reason.

But your other argument?
Note that PBP on the opposite tends to move to using lexical variables instead of global package vars (where possible). So we should not use our if we can solve it differently. In this case for example, i could pass lower my variables via parameters..
Comment 38 Jonathan Druart 2013-09-20 08:06:18 UTC
(In reply to M. de Rooy from comment #37)
> Jonathan,
> You convinced me for the Plack reason.
> 
> But your other argument?
> Note that PBP on the opposite tends to move to using lexical variables
> instead of global package vars (where possible). So we should not use our if
> we can solve it differently. In this case for example, i could pass lower my
> variables via parameters..

Hello Marcel,
It is a vocabulary issue :)
What I wrongly called a global variable was a variable declared outside of the routine where it is used.
Obviously, passing the variables in parameter is better than declaring them as global (our).
Comment 39 Marcel de Rooy 2014-03-01 10:07:08 UTC
Hope to revive this development next week by rebasing, making the requested QA changes and including work from the former dependency bug 10389 and bug 10441.
Comment 40 Marcel de Rooy 2014-03-05 14:27:54 UTC Comment hidden (obsolete)
Comment 41 Marcel de Rooy 2014-03-05 14:28:31 UTC Comment hidden (obsolete)
Comment 42 Marcel de Rooy 2014-03-05 14:34:15 UTC Comment hidden (obsolete)
Comment 43 Marcel de Rooy 2014-03-05 14:38:53 UTC Comment hidden (obsolete)
Comment 44 Marcel de Rooy 2014-03-05 14:39:54 UTC Comment hidden (obsolete)
Comment 45 Marcel de Rooy 2014-03-05 14:44:06 UTC Comment hidden (obsolete)
Comment 46 Marcel de Rooy 2014-03-05 14:48:03 UTC Comment hidden (obsolete)
Comment 47 Marcel de Rooy 2014-03-05 14:50:12 UTC
Before changing the status, I will still add at lease one patch.
This is to handle the bootstrap theme.
Still thinking about one additional point too..
Comment 48 Marcel de Rooy 2014-03-05 15:55:39 UTC Comment hidden (obsolete)
Comment 49 Marcel de Rooy 2014-03-06 09:00:00 UTC Comment hidden (obsolete)
Comment 50 Marcel de Rooy 2014-03-06 13:47:59 UTC Comment hidden (obsolete)
Comment 51 Marcel de Rooy 2014-03-06 13:53:01 UTC
This is ready for signoff again.
Note that some of the patches already had a signoff.
Some have been amended while rebasing (as noted). Some patches are new.

In other words: Please help to get this further. Thanks!
Comment 52 Marcel de Rooy 2014-03-13 11:05:21 UTC Comment hidden (obsolete)
Comment 53 Marcel de Rooy 2014-03-13 11:06:38 UTC
Trivial rebase of the dbrev for easier signoff :)
Comment 54 Dobrica Pavlinusic 2014-03-13 14:28:01 UTC
I was testing this patch and when I copy/paste URL without being logged in into OPAC I get following error:

ERROR: You do not have adequate permission for that action on list 1330.

However, after that, I'm not logged in into OPAC. If I login after that I indeed see shared list and everything works fine. This might be problem with Koha login mechanism instead of this patch, but it's just interesting to note.

If I'm already logged in into OPAC it works as expected.

Other than that, this patch provides very useful addition to Koha, thanks.
Comment 55 Dobrica Pavlinusic 2014-03-13 14:33:45 UTC Comment hidden (obsolete)
Comment 56 Dobrica Pavlinusic 2014-03-13 14:36:58 UTC Comment hidden (obsolete)
Comment 57 Dobrica Pavlinusic 2014-03-13 14:37:18 UTC Comment hidden (obsolete)
Comment 58 Dobrica Pavlinusic 2014-03-13 14:37:36 UTC Comment hidden (obsolete)
Comment 59 Dobrica Pavlinusic 2014-03-13 14:37:52 UTC Comment hidden (obsolete)
Comment 60 Dobrica Pavlinusic 2014-03-13 14:38:09 UTC Comment hidden (obsolete)
Comment 61 Dobrica Pavlinusic 2014-03-13 14:38:21 UTC Comment hidden (obsolete)
Comment 62 Dobrica Pavlinusic 2014-03-13 14:38:34 UTC Comment hidden (obsolete)
Comment 63 Dobrica Pavlinusic 2014-03-13 14:38:45 UTC Comment hidden (obsolete)
Comment 64 Dobrica Pavlinusic 2014-03-13 14:38:54 UTC Comment hidden (obsolete)
Comment 65 Dobrica Pavlinusic 2014-03-13 14:44:44 UTC Comment hidden (obsolete)
Comment 66 Dobrica Pavlinusic 2014-03-13 14:45:36 UTC Comment hidden (obsolete)
Comment 67 Dobrica Pavlinusic 2014-03-13 15:22:18 UTC
Some of patches in this searies might change order because of git bz barfing on me when changing bug status to signed off more than once, but thay do apply on top of master as they are ordered now and don't show any diff compared to my version on disk which I signed off. So, sorry if I re-ordered them :-)
Comment 68 Marcel de Rooy 2014-03-14 14:44:52 UTC Comment hidden (obsolete)
Comment 69 Marcel de Rooy 2014-03-14 14:49:03 UTC
(In reply to Dobrica Pavlinusic from comment #67)
> Some of patches in this searies might change order because of git bz barfing
> on me when changing bug status to signed off more than once, but thay do
> apply on top of master as they are ordered now and don't show any diff
> compared to my version on disk which I signed off. So, sorry if I re-ordered
> them :-)

No worries. Looks good. Sent an small fix for the login problem.
Comment 70 Jonathan Druart 2014-03-19 15:04:15 UTC
QA comment:

I did not check if the current code fixes all QA issues raised on comment 26.

Additional remarks:

1/ In AddShare: you remove the 10 oldest entries and add 1? What I missed?

2/ AddShare:
    return !$dbh->err;
    => useless, $dbh->do insert returns the number of inserted rows.

3/ C4::VirtualShelves::ShelfPossibleAction
Why do you use numeric code? It should be better to use an readable code. Moreover, these codes are used in the template.

4/  return if !defined($i) || !$i || $i eq '0E0'; #not found
    return 1;
Could be replaced with
 return 1 if $i > 0
I think.

5/ It could be great to replace quotes with simple quotes around sql queries.

6/ (In reply to M. de Rooy from comment #31)
> (In reply to Jonathan Druart from comment #26)
> > - The new letter should be added to others languages.
> I would really get rid of that restriction. I started some development on
> that (look for i18n). But unfortunately there is not that much support for
> it.

Yes, but this one cannot be pushed as it and this one is not marked as dependent on the other bug report.

7/
opac/opac-shareshelf.pl
+ for( 0..$#$alphabet )

It could be good to name the variable loop.
Comment 71 Marcel de Rooy 2014-03-19 15:46:06 UTC
(In reply to Jonathan Druart from comment #70)
> QA comment:
Thanks for taking a look.

> 1/ In AddShare: you remove the 10 oldest entries and add 1? What I missed?
Just housekeeping. 10 is just an arbitrary number. (Could add this at some point in cleanup_database.)

> 2/ AddShare:
>     return !$dbh->err;
>     => useless, $dbh->do insert returns the number of inserted rows.
Not sure. Could that be 0E0 ? Think that this is safer.

> 3/ C4::VirtualShelves::ShelfPossibleAction
> Why do you use numeric code? It should be better to use an readable code.
> Moreover, these codes are used in the template.
Have been using error numbers in shareshelf everywhere. Would recommend to leave it as-is for now.

> 4/  return if !defined($i) || !$i || $i eq '0E0'; #not found
>     return 1;
> Could be replaced with
>  return 1 if $i > 0
> I think.
Not sure about that too.

> 6/ (In reply to M. de Rooy from comment #31)
> > (In reply to Jonathan Druart from comment #26)
> > > - The new letter should be added to others languages.
> > I would really get rid of that restriction. I started some development on
> > that (look for i18n). But unfortunately there is not that much support for
> > it.
> Yes, but this one cannot be pushed as it and this one is not marked as
> dependent on the other bug report.
I abandoned the i18n reports. Getting that into Koha is too much for me on my own. Even reports of this size are hard to get in.
In the commit message I wrote that I will add the other 10? languages as soon as this is ready for a push. (It will only trigger more merge conflicts.)

> 7/
> opac/opac-shareshelf.pl
> + for( 0..$#$alphabet )
> 
> It could be good to name the variable loop.
No big deal. Will do that.
Comment 72 Marcel de Rooy 2014-03-19 15:58:03 UTC Comment hidden (obsolete)
Comment 73 Marcel de Rooy 2014-03-19 16:01:56 UTC
(In reply to Jonathan Druart from comment #70)

> 5/ It could be great to replace quotes with simple quotes around sql queries.

Can we do that for C4/VirtualShelves.pm on another report (since there are much more double quotes over there) and not put that load here?
Will open one for that purpose.
Like to wait with code changes of that nature until this code moves..
Comment 74 Jonathan Druart 2014-03-20 11:49:44 UTC
(In reply to Jonathan Druart from comment #26)
> QA comment:
> - The new letter should be added to others languages.

So what do you suggest for that? When will your provide a patch? After the push?

> The ergonomic:
> - The page opac-shelves.pl?display=privateshelves displays a link for
> editing and deleting, but a button for sharing. Is it intentional?
> - There is no back button if I enter a bad email address.

It seems you did not answer on that.

(In reply to M. de Rooy from comment #71)
> (In reply to Jonathan Druart from comment #70)
> > 1/ In AddShare: you remove the 10 oldest entries and add 1? What I missed?
> Just housekeeping. 10 is just an arbitrary number. (Could add this at some
> point in cleanup_database.)

Yes, I am under the impression that the deletion should be done somewhere else.
Comment 75 Marcel de Rooy 2014-03-20 12:52:50 UTC
(In reply to Jonathan Druart from comment #74)
Thanks again.

> (In reply to Jonathan Druart from comment #26)
> > QA comment:
> > - The new letter should be added to others languages.
> 
> So what do you suggest for that? When will your provide a patch? After the
> push?
As soon as the RM is willing to push, I will do it right away.

> > The ergonomic:
> > - The page opac-shelves.pl?display=privateshelves displays a link for
> > editing and deleting, but a button for sharing. Is it intentional?
> > - There is no back button if I enter a bad email address.
> 
> It seems you did not answer on that.
Sorry, could have missed that. I fixed the button for bootstrap btw. I left it in prog, since that will go away soon.
If you want, I can send a followup for a back button.

> (In reply to M. de Rooy from comment #71)
> > (In reply to Jonathan Druart from comment #70)
> > > 1/ In AddShare: you remove the 10 oldest entries and add 1? What I missed?
> > Just housekeeping. 10 is just an arbitrary number. (Could add this at some
> > point in cleanup_database.)
> 
> Yes, I am under the impression that the deletion should be done somewhere
> else.
I suggest to leave it here now, but I will open a new report and move it then to cleanup-database.pl. Agreed?
Comment 76 Katrin Fischer 2014-03-20 12:58:29 UTC
I think there are not enough changes to the sample notices to expect a lot of merge problems in that area. I'd prefer to have the patch ready when Galen reviews this, so there are no additional dependencies that can get lost or forgotten. Also the patch should be tested.
Comment 77 Marcel de Rooy 2014-03-21 11:21:19 UTC Comment hidden (obsolete)
Comment 78 Marcel de Rooy 2014-03-21 11:25:44 UTC
(In reply to Katrin Fischer from comment #76)
> I think there are not enough changes to the sample notices to expect a lot
> of merge problems in that area. I'd prefer to have the patch ready when
> Galen reviews this, so there are no additional dependencies that can get
> lost or forgotten. Also the patch should be tested.

OK Katrin. I'll still try to do it.. We should have changed this.. :)
Comment 79 Marcel de Rooy 2014-03-21 15:09:39 UTC Comment hidden (obsolete)
Comment 80 Jonathan Druart 2014-03-26 14:52:00 UTC
Created attachment 26577 [details] [review]
Bug 9032: Share a list (part 1: send the invitation)

Adds a Share button for OPAC private lists.
Allows you to send an invitation to share a list.
Checks on validity of email addresses (with Email::Valid).

Test plan:
1) Sharing depends on syspref and login.
Toggle the pref OpacAllowSharingPrivateList.
If enabled, you should see the Share button in OPAC/Private lists.
Click on the Share button. You should get Share a list.
Logout and try to go back to opac/opac-shareshelf.pl
It should now present you the login form.

2) Try to share a public list or a list you do not own.
Find a security hole in the interface. Or hack the shareshelf URL and replace
the shelfnumber with a public list number.

3) Enter no email address or invalid ones (no domain, forbidden chars).
If you enter no address, submit should not work.
If you enter only wrong addresses (separated by: ,:; ), you get a message.

4) Test if sending the invitation works.
Share one of your private lists. Enter your own email address.
After your proc_message_queue cronjob ran, you should have an email.
Check also if you see a new record in the virtualshelfshares table.
Note that the followup patch handles the second part of accepting this share.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Rebased.
Small amendation: rewording on email notification in template.

Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 81 Jonathan Druart 2014-03-26 14:52:16 UTC
Created attachment 26578 [details] [review]
Bug 9032: Followup for new dependency Email::Valid

Just adds this dependency to PerlDependencies module.
Note from Robin: libemail-valid-perl is in squeeze already.

Test plan:
You should see Email::Valid now under About/Perl modules.
If Email::Valid is not yet installed on your system, please do so.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Rebased on March 3, 2014.

Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 82 Jonathan Druart 2014-03-26 14:52:24 UTC
Created attachment 26579 [details] [review]
Bug 9032 [Template follow-up] Share a list (part 1: send the invitation)

This patch makes a few changes to the "share a list" template:

- We don't use tables for layout of forms. This patch corrects the
  display of the share form to use Koha standard form markup.
- Revise the confirmation message displayed after a list is shared to be
  more readable.
- Add a link back to private lists to the confirmation message.
- Add T:T markup to make this page display the left-hand navigation
  which is shown on other patron-related pages when a user is logged in.

To test, apply this patch on top of other patches pending for Bug 9032
and confirm that the list share form looks correct and works correctly.
Confirm that share confirmation page reads well.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Rebased March 5, 2014.

Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 83 Jonathan Druart 2014-03-26 14:52:32 UTC
Created attachment 26580 [details] [review]
Bug 9032: [QA Follow-up] Plackify opac-shareshelf.pl

Plackified opac-shareshelf by moving global my variables into hashref.
Slightly changed base64ord and base64chr functions to more generic ones.

Test plan:
Verify if sending an share invitation still works (under prog theme).

Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 84 Jonathan Druart 2014-03-26 14:52:39 UTC
Created attachment 26581 [details] [review]
Bug 9032: Share a list (accept invitation and remove a share)

This patch handles:

[1] The response (acceptance) by the invited person.
If he accepts this share, the private list of the sender will be shown under
Your lists on the shelves page. In OPAC 'Your private lists' has been renamed
to Your lists (just as in Staff). The Type column shows Private or Shared for
these lists; a list appears as Shared as soon as an invitation has been
accepted. The owner has the options to Edit, Delete or Share; the invited
person does not have these options on the shared list.

[2] Removing an accepted share.
If a user accepted a share, he should also be able to remove it again.
The Remove Share button is visible on OPAC when viewing Your lists or
a particular shared list.

Note: AddShare has been extended to return a possible database error.
If the share invite could not be added, a mail will not be sent.

Test plan (for prog theme):
Enable pref OpacAllowSharingPrivateLists
User 1 creates new private list P1, perms: D-A-D, adds 2 items, sends share
User 1 checks your lists display: is P1 Private with Edit button?
User 2 accepts share: sees P1, but cannot add or delete items
User 2 checks your lists display again: P1 shows Shared without Edit?
User 1 checks your lists display again: P1 shows Shared with Edit?
User 2 tries to accept share again: should fail now
User 3 tries to accept share: should also fail
User 3 tries again, modifies shelfnumber and/or key in url: should also fail

User 2 creates new private list P2, perms: A-A-A, no items, sends share
User 2 checks your lists display: P2 shows Private with Edit?
User 1 accepts, adds one item
User 1 checks your lists display: P2 shows Shared without Edit?
User 2 checks your lists display: P2 shows Shared with Edit?
User 2 deletes item of user 1 (allowed)
User 2 deletes list P2
User 1 checks your lists display in opac or staff: P2 is gone?

User 1 creates private list P3, sends a share.
User 1 creates private list P4, adds one item, sends a share.
User 2 accepts the share for P3.
User 2 checks the shelves display, and removes share P3.
User 2 accepts the share for P4.
User 2 views shelf P4 with one item and confirms Remove share on that form.
User 2 checks shelves display again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Rebased on March 3, 2014. Combined changes for AcceptShare and RemoveShare.

Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 85 Jonathan Druart 2014-03-26 14:52:45 UTC
Created attachment 26582 [details] [review]
Bug 9032: Unit tests for sharing lists

This patch combines some work from report 10389 and 10441.
With thanks to Srdjan and Jonathan Druart for their contributions.

Adds tests for AddShare, AcceptShare, IsSharedList, RemoveShare.
This patch also restores the inadvertently deleted DelShelf tests
in bug 10499.

Test plan: Run the test VirtualShelves.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Rebased and squashed on March 4, 2013.

Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 86 Jonathan Druart 2014-03-26 14:52:52 UTC
Created attachment 26583 [details] [review]
Bug 9032: Consistent behavior when deleting lists

DelShelf deletes a list regardless whether it is private, shared or
public. HandleDelBorrower had another approach, trying to save shared
and public lists by setting the owner to NULL.
This patch makes both routines behave consistent.
A new report 11889 has been opened to discuss the 'disowning' feature.

NOTE: I did not add a db revision here to handle possible cases of lists
without owner in the current data. Such public (or shared) lists can still
be used without any problem. Bug 11889 and a new planned report for a lists
management tool will address this topic further on. After that, all goals
of umbrella report 7310 should be realized.

Test plan:
Create a list P1 with user1 that allows adding by other users.
Add a patron (user2).
Login as user2 and create some lists, add some items.
Let user2 add some entries to P1 too.
Delete patron user2.
Verify that his lists are gone, but his entries in P1 are kept (nullified).

Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 87 Jonathan Druart 2014-03-26 14:52:59 UTC
Created attachment 26584 [details] [review]
Bug 9032: Follow-up for bootstrap theme

Adds new template opac-shareshelf.tt.
Modifies template opac-shelves.tt: Share button, Remove Share button, label
"Your lists" instead of "Your private lists", list category Shared.

Test plan:
Verify if the Share a list-features work in bootstrap by:
[1] Switch to bootstrap. Go to Lists.
[2] Share one of your private lists.
[3] Login as another user and accept the invitation.
[4] Remove the share again.
[5] Check if Share and Remove share do not popup for public lists.

Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 88 Jonathan Druart 2014-03-26 14:53:06 UTC
Created attachment 26585 [details] [review]
Bug 9032: Share a list (database revision for notices)

Adds notice SHARE_INVITE in updatedatabase and sample_notices.
[Amendation: also adds SHARE_ACCEPT notice.]

Test plan:
Install or upgrade and verify that you have the new notice under Tools.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Rebased on March 3, 2014.
Amended the patch to add the second notice SHARE_ACCEPT.
Tested upgrade with updatestructure step of web installer.
Tested install by deleting all notices and running sample_notices.sql

NOTE: Will add the notices to the other sql files in the other
language directories as soon as this set of patches is ready to be
pushed in order to reduce the need to rebase them constantly.

Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 89 Jonathan Druart 2014-03-26 14:53:14 UTC
Created attachment 26586 [details] [review]
Bug 9032: QA Follow-up: Send cookie with redirect

Small fix to resolve a problem Dobrica noted on the report.
If you had to login when accepting the share, the cookie was not passed
back to the server when redirecting to shelf display.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Accepted a share while logged in and while not logged in.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 90 Jonathan Druart 2014-03-26 14:53:22 UTC
Created attachment 26587 [details] [review]
Bug 9032: QA Follow-up: Loop variable and Cancel button

To satisfy QA needs:
Use loop variable instead of $_ in opac-shareshelf.pl
Adds Cancel button to Invite form (prog and bootstrap) in opac-shareshelf.tt.
Likewise adds Return link under an error message in opac-shareshelf.tt.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 91 Jonathan Druart 2014-03-26 14:53:30 UTC
Created attachment 26588 [details] [review]
Bug 9032: QA Follow-up: Adding notices to foreign installer files

Adds the notices SHARE_INVITE and SHARE_ACCEPT to sample_notices.sql for the
foreign language installer files in installer/data/mysql.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
I tested each installer file by deleting all notices from letter, running
the file and counting the records. I did so with and without this patch.
The results below show that in each case two notices were added as
expected. Note that the English file contains 25 notices including the two
new ones.

BEFORE THIS PATCH:
de-DE/mandatory/sample_notices.sql: 23 notices
es-ES/mandatory/sample_notices.sql: 18 notices
fr-FR/1-Obligatoire/sample_notices.sql: 18 notices
it-IT/necessari/notices.sql: 21 notices
nb-NO/1-Obligatorisk/sample_notices.sql: 23 notices
pl-PL/mandatory/sample_notices.sql: 18 notices
ru-RU/mandatory/sample_notices.sql: 18 notices
uk-UA/mandatory/sample_notices.sql: 17 notices

AFTER THIS PATCH:
de-DE/mandatory/sample_notices.sql: 25 notices
es-ES/mandatory/sample_notices.sql: 20 notices
fr-FR/1-Obligatoire/sample_notices.sql: 20 notices
it-IT/necessari/notices.sql: 23 notices
nb-NO/1-Obligatorisk/sample_notices.sql: 25 notices
pl-PL/mandatory/sample_notices.sql: 20 notices
ru-RU/mandatory/sample_notices.sql: 20 notices
uk-UA/mandatory/sample_notices.sql: 19 notices

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 92 Marcel de Rooy 2014-03-26 15:21:02 UTC
Thanks for QA, Jonathan.
Comment 93 Galen Charlton 2014-04-20 22:53:37 UTC
Pushed to master, along with my usual set of follow-ups.  One follow-up deserving special mention is the last one, which restores the documented interpretation of the sharedate column.

Thanks, Marcel!
Comment 94 Marcel de Rooy 2014-04-21 14:38:37 UTC
(In reply to Galen Charlton from comment #93)
> Pushed to master, along with my usual set of follow-ups.  One follow-up
> deserving special mention is the last one, which restores the documented
> interpretation of the sharedate column.
Thanks for the follow-ups. Retested the feature successfully in current master along with new parameter of cleanup-database.