Bug 9032: add ability to accept list share invitations and remove shares

This patch handles:

[1] The response (acceptance) by the invited person.

If the person 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, they 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>
Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>

C4/VirtualShelves.pm

@@ -44,7 +44,8 @@ BEGIN {
             &ModShelf
             &ShelfPossibleAction
             &DelFromShelf &DelShelf
-            &GetBibliosShelves &AddShare
+            &GetBibliosShelves
+            &AddShare &AcceptShare &RemoveShare &IsSharedList
     );
         @EXPORT_OK = qw(
             &GetAllShelves &ShelvesMax
@@ -434,6 +435,7 @@ ShelfPossibleAction($loggedinuser, $shelfnumber, $action);
 C<$loggedinuser,$shelfnumber,$action>
 
 $action can be "view", "add", "delete", "manage", "new_public", "new_private".
+New additional actions are: invite, acceptshare.
 Note that add/delete here refers to adding/deleting entries from the list. Deleting the list itself falls under manage.
 new_public and new_private refers to creating a new public or private list.
 The distinction between deleting your own entries from the list or entries from
@@ -441,6 +443,8 @@ others is made in DelFromShelf.
 
 Returns 1 if the user can do the $action in the $shelfnumber shelf.
 Returns 0 otherwise.
+For the actions invite and acceptshare a second errorcode is returned if the
+result is false. See opac-shareshelf.pl
 
 =cut
 
@@ -490,6 +494,28 @@ sub ShelfPossibleAction {
         #DelFromShelf checks the situation per biblio
         return 1 if $user>0 && ($shelf->{allow_delete_own}==1 || $shelf->{allow_delete_other}==1);
     }
+    elsif($action eq 'invite') {
+        #for sharing you must be the owner and the list must be private
+        if( $shelf->{category}==1 ) {
+            return 1 if $shelf->{owner}==$user;
+            return (0, 4); # code 4: should be owner
+        }
+        else {
+            return (0, 5); # code 5: should be private list
+        }
+    }
+    elsif($action eq 'acceptshare') {
+        #the key for accepting is checked later in AcceptShare
+        #you must not be the owner, list must be private
+        if( $shelf->{category}==1 ) {
+            return (0, 8) if $shelf->{owner}==$user;
+                #code 8: should not be owner
+            return 1;
+        }
+        else {
+            return (0, 5); # code 5: should be private list
+        }
+    }
     elsif($action eq 'manage') {
         return 1 if $user && $shelf->{owner}==$user;
     }
@@ -665,6 +691,76 @@ sub AddShare {
     $dbh->do($sql);
     $sql="INSERT INTO virtualshelfshares (shelfnumber, invitekey, sharedate) VALUES (?, ?, ADDDATE(NOW(),?))";
     $dbh->do($sql, undef, ($shelfnumber, $key, SHARE_INVITATION_EXPIRY_DAYS));
+    return !$dbh->err;
+}
+
+=head2 AcceptShare
+
+     my $result= AcceptShare($shelfnumber, $key, $borrowernumber);
+
+Checks acceptation of a share request.
+Key must be found for this shelf. Invitation must not have expired.
+Returns true when accepted, false otherwise.
+
+=cut
+
+sub AcceptShare {
+    my ($shelfnumber, $key, $borrowernumber)= @_;
+    return if !$shelfnumber || !$key || !$borrowernumber;
+
+    my $sql;
+    my $dbh = C4::Context->dbh;
+    $sql="
+UPDATE virtualshelfshares
+SET invitekey=NULL, sharedate=NULL, borrowernumber=?
+WHERE shelfnumber=? AND invitekey=? AND sharedate>NOW()
+    ";
+    my $i= $dbh->do($sql, undef, ($borrowernumber, $shelfnumber, $key));
+    return if !defined($i) || !$i || $i eq '0E0'; #not found
+    return 1;
+}
+
+=head2 IsSharedList
+
+     my $bool= IsSharedList( $shelfnumber );
+
+IsSharedList checks if a (private) list has shares.
+Note that such a check would not be useful for public lists. A public list has
+no shares, but is visible for anyone by nature..
+Used to determine the list type in the display of Your lists (all private).
+Returns boolean value.
+
+=cut
+
+sub IsSharedList {
+    my ($shelfnumber) = @_;
+    my $dbh = C4::Context->dbh;
+    my $sql="SELECT id FROM virtualshelfshares WHERE shelfnumber=? AND borrowernumber IS NOT NULL";
+    my $sth = $dbh->prepare($sql);
+    $sth->execute($shelfnumber);
+    my ($rv)= $sth->fetchrow_array;
+    return defined($rv);
+}
+
+=head2 RemoveShare
+
+     RemoveShare( $user, $shelfnumber );
+
+RemoveShare removes a share for specific shelf and borrower.
+Returns true if a record could be deleted.
+
+=cut
+
+sub RemoveShare {
+    my ($user, $shelfnumber)= @_;
+    my $dbh = C4::Context->dbh;
+    my $sql="
+DELETE FROM virtualshelfshares
+WHERE borrowernumber=? AND shelfnumber=?
+    ";
+    my $n= $dbh->do($sql,undef,($user, $shelfnumber));
+    return if !defined($n) || !$n || $n eq '0E0'; #nothing removed
+    return 1;
 }
 
 # internal subs

C4/VirtualShelves/Page.pm

@@ -360,13 +360,22 @@ sub shelfpage {
 
         #Deleting a shelf (asking for confirmation if it has entries)
             foreach ( $query->param() ) {
-                /DEL-(\d+)/ or next;
+                /(DEL|REMSHR)-(\d+)/ or next;
                 $delflag = 1;
-                my $number = $1;
+                my $number = $2;
                 unless ( defined $shelflist->{$number} || defined $privshelflist->{$number} ) {
                     push( @paramsloop, { unrecognized => $number } );
                     last;
                 }
+                #remove a share
+                if(/REMSHR/) {
+                    RemoveShare($loggedinuser, $number);
+                    delete $shelflist->{$number} if exists $shelflist->{$number};
+                    delete $privshelflist->{$number} if exists $privshelflist->{$number};
+                    $stay=0;
+                    next;
+                }
+                #
                 unless ( ShelfPossibleAction( $loggedinuser, $number, 'manage' ) ) {
                     push( @paramsloop, { nopermission => $shelfnumber } );
                     last;
@@ -434,6 +443,7 @@ sub shelfpage {
         $shelflist->{$element}->{ownername} = defined($member) ? $member->{firstname} . " " . $member->{surname} : '';
         $numberCanManage++ if $canmanage;    # possibly outmoded
         if ( $shelflist->{$element}->{'category'} eq '1' ) {
+            $shelflist->{$element}->{shares} = IsSharedList($element);
             push( @shelveslooppriv, $shelflist->{$element} );
         } else {
             push( @shelvesloop, $shelflist->{$element} );

koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref

@@ -529,7 +529,7 @@ OPAC:
               choices:
                   no: "Don't allow"
                   yes: Allow
-            - opac users to share private lists with other patrons. This feature is not active yet but will be released soon
+            - opac users to share private lists with other patrons.
 
     Privacy:
         -

koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt

@@ -538,7 +538,7 @@ function placeHold () {
         <td><a href="shelves.pl?[% IF ( shelveslooppri.showprivateshelves ) %]display=privateshelves&amp;[% END %]viewshelf=[% shelveslooppri.shelf %]&amp;shelfoff=[% shelfoff %]">[% shelveslooppri.shelfname |html %]</a></td>
         <td>[% shelveslooppri.count %] item(s)</td>
         <td>[% IF ( shelveslooppri.sortfield == "author" ) %]Author[% ELSIF ( shelveslooppri.sortfield == "copyrightdate" ) %]Year[% ELSIF (shelveslooppri.sortfield == "itemcallnumber") %]Call number[% ELSE %]Title[% END %]</td>
-        <td>[% IF ( shelveslooppri.viewcategory1 ) %]Private[% END %]
+        <td>[% IF ( shelveslooppri.viewcategory1 ) %][% IF !shelveslooppri.shares %]Private[% ELSE %]Shared[% END %][% END %]
 			[% IF ( shelveslooppri.viewcategory2 ) %]Public[% END %]
 		</td>
         <td>

koha-tmpl/opac-tmpl/prog/en/modules/opac-shareshelf.tt

@@ -19,6 +19,8 @@
         [% IF errcode==4 %]<div class="dialog alert">You can only share a list if you are the owner.</div>[% END %]
         [% IF errcode==5 %]<div class="dialog alert">You cannot share a public list.</div>[% END %]
         [% IF errcode==6 %]<div class="dialog alert">Sorry, but you did not enter any valid email address.</div>[% END %]
+        [% IF errcode==7 %]<div class="dialog alert">Sorry, but we could not accept this key. The invitation may have expired. Contact the patron who sent you the invitation.</div>[% END %]
+        [% IF errcode==8 %]<div class="dialog alert">As owner of a list you cannot accept an invitation for sharing it.</div>[% END %]
 
     [% ELSIF op=='invite' %]
         <form method="post" onsubmit="return $('#invite_address').val().trim()!='';">
@@ -37,18 +39,19 @@
         </form>
 
     [% ELSIF op=='conf_invite' %]
+        [% IF approvedaddress %]
         <p>An invitation to share list <i>[% shelfname %]</i> has been sent to [% approvedaddress %].</p>
+        [% END %]
         [% IF failaddress %]
-            <p>The following addresses appear to be invalid. Please correct them and try again. These are: [% failaddress %]</p>
+            <p>Something went wrong while processing the following addresses. Please check them. These are: [% failaddress %]</p>
         [% END %]
+        [% IF approvedaddress %]
         <p>You will receive an email notification if someone accepts your share within two weeks.</p>
+        [% END %]
         <p><a href="/cgi-bin/koha/opac-shelves.pl?display=privateshelves">Return to your lists</a></p>
 
     [% ELSIF op=='accept' %]
-        [%# TODO: Replace the following two lines %]
-        <p>Thank you for testing this feature.</p>
-        <p>Your signoff will certainly help in finishing the remaining part!</p>
-
+        [%# Nothing to do: we already display an error or we redirect. %]
     [% END %]
 [%# End of essential part %]
 

koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt

@@ -8,6 +8,7 @@
 var MSG_REMOVE_FROM_LIST = _("Are you sure you want to remove these items from the list?");
 var MSG_REMOVE_ONE_FROM_LIST = _("Are you sure you want to remove this item from the list?");
 var MSG_CONFIRM_DELETE_LIST = _("Are you sure you want to delete this list?");
+var MSG_CONFIRM_REMOVE_SHARE = _("Are you sure you want to remove this share?");
 
 [% IF ( opacuserlogin ) %][% IF ( RequestOnOpac ) %]
 function holdSelections() {
@@ -239,6 +240,15 @@ $(document).ready(function() {
     </li>
 [% END %]
 
+[%# When using the next block, add the parameter for shelfnumber and add a tag to end the form %]
+[% BLOCK remove_share %]
+    <form action="opac-shelves.pl" method="post">
+        <input type="hidden" name="shelves" value="1" />
+        <input type="hidden" name="display" value="privateshelves" />
+        <input type="hidden" name="shelfoff" value="[% shelfoff %]" />
+        <input type="submit" class="removeshare" onclick="return confirmDelete(MSG_CONFIRM_REMOVE_SHARE);" value="Remove share" />
+[% END %]
+
 [% IF ( OpacNav ) %]<div id="doc3" class="yui-t1">[% ELSIF ( loggedinusername ) %]<div id="doc3" class="yui-t1">[% ELSE %]<div id="doc3" class="yui-t7">[% END %]
     <div id="bd">
       [% INCLUDE 'masthead.inc' %]
@@ -371,6 +381,10 @@ $(document).ready(function() {
                             <input type="submit" class="Share" value="Share" />
                         </form>
                     [% END %]
+                [% ELSIF showprivateshelves %]
+                    [% INCLUDE remove_share %]
+                    <input type="hidden" name="REMSHR-[% shelfnumber %]" value="1" />
+                    </form>
                 [% END %]
 
 
@@ -602,9 +616,9 @@ $(document).ready(function() {
                   <ul class="link-tabs">
                   [% IF ( opacuserlogin ) %]
                   [% IF ( showprivateshelves ) %]
-                    <li id="privateshelves_tab" class="on"><a href="/cgi-bin/koha/opac-shelves.pl?display=privateshelves">Your private lists</a></li>
+                    <li id="privateshelves_tab" class="on"><a href="/cgi-bin/koha/opac-shelves.pl?display=privateshelves">Your lists</a></li>
                   [% ELSE %]
-                    <li id="privateshelves_tab" class="off"><a href="/cgi-bin/koha/opac-shelves.pl?display=privateshelves">Your private lists</a></li>
+                    <li id="privateshelves_tab" class="off"><a href="/cgi-bin/koha/opac-shelves.pl?display=privateshelves">Your lists</a></li>
                   [% END %]
                   [% END %]
                   [% IF ( showpublicshelves ) %]
@@ -629,7 +643,7 @@ $(document).ready(function() {
                           <th>List name</th>
                           <th>Contents</th>
                           <th>Type</th>
-                          <th>&nbsp;</th>
+                          <th>Options</th>
                         </tr>
                         [% FOREACH shelveslooppri IN shelveslooppriv %]
                           [% UNLESS ( loop.odd ) %]
@@ -640,7 +654,7 @@ $(document).ready(function() {
                               <td><a href="/cgi-bin/koha/opac-shelves.pl?display=privateshelves&amp;viewshelf=[% shelveslooppri.shelf %]&amp;sortfield=[% shelveslooppri.sortfield %]">[% shelveslooppri.shelfname |html %]</a></td>
                               <td>[% IF ( shelveslooppri.count ) %][% shelveslooppri.count %] [% IF ( shelveslooppri.single ) %]item[% ELSE %]items[% END %][% ELSE %]Empty[% END %]</td>
                               <td>
-                                [% IF ( shelveslooppri.viewcategory1 ) %]Private[% END %]
+                                [% IF ( shelveslooppri.viewcategory1 ) %][% IF !shelveslooppri.shares %]Private[% ELSE %]Shared[% END %][% END %]
                                 [% IF ( shelveslooppri.viewcategory2 ) %]Public[% END %]
                               </td>
                               <td>
@@ -670,6 +684,10 @@ $(document).ready(function() {
                                   <input type="submit" class="Share" value="Share" />
                                 </form>
                               [% END %]
+                            [% ELSIF shelveslooppri.shares %]
+                                [% INCLUDE remove_share %]
+                                <input type="hidden" name="REMSHR-[% shelveslooppri.shelf %]" value="1" />
+                                </form>
                             [% END %]&nbsp;
                             </td>
                           </tr>
@@ -702,7 +720,8 @@ $(document).ready(function() {
                         <tr>
                           <th>List name</th>
                           <th>Contents</th>
-                          <th>Type</th><th>&nbsp;</th>
+                          <th>Type</th>
+                          <th>Options</th>
                         </tr>
                     [% FOREACH shelvesloo IN shelvesloop %]
                       [% UNLESS ( loop.odd ) %]

opac/opac-shareshelf.pl

@@ -22,6 +22,7 @@ use warnings;
 
 use constant KEYLENGTH => 10;
 use constant TEMPLATE_NAME => 'opac-shareshelf.tmpl';
+use constant SHELVES_URL => '/cgi-bin/koha/opac-shelves.pl?display=privateshelves&viewshelf=';
 
 use CGI;
 use Email::Valid;
@@ -29,6 +30,7 @@ use Email::Valid;
 use C4::Auth;
 use C4::Context;
 use C4::Letters;
+use C4::Members ();
 use C4::Output;
 use C4::VirtualShelves;
 
@@ -55,8 +57,16 @@ sub _init {
     $param->{addrlist} = $query->param('invite_address')||'';
     $param->{key} = $query->param('key')||'';
     $param->{appr_addr} = [];
-
+    $param->{fail_addr} = [];
     $param->{errcode} = check_common_errors($param);
+
+    #get some list details
+    my @temp;
+    @temp= GetShelf( $param->{shelfnumber} ) if !$param->{errcode};
+    $param->{shelfname} = @temp? $temp[1]: '';
+    $param->{owner} = @temp? $temp[2]: -1;
+    $param->{category} = @temp? $temp[3]: -1;
+
     load_template($param);
     return $param;
 }
@@ -94,14 +104,57 @@ sub confirm_invite {
 
 sub show_accept {
     my ($param) = @_;
-    #TODO Add some code here to accept an invitation (followup report)
+
+    my @rv= ShelfPossibleAction($param->{loggedinuser},
+        $param->{shelfnumber}, 'acceptshare');
+    $param->{errcode} = $rv[1] if !$rv[0];
+    return if $param->{errcode};
+        #errorcode 5: should be private list
+        #errorcode 8: should not be owner
+
+    my $dbkey= keytostring( stringtokey($param->{key}, 0), 1);
+    if( AcceptShare($param->{shelfnumber}, $dbkey, $param->{loggedinuser} ) ) {
+        notify_owner($param);
+        #redirect to view of this shared list
+        print $param->{query}->redirect(SHELVES_URL.$param->{shelfnumber});
+        exit;
+    }
+    else {
+        $param->{errcode} = 7; #not accepted (key not found or expired)
+    }
+}
+
+sub notify_owner {
+    my ($param) = @_;
+
+    my $toaddr=  C4::Members::GetNoticeEmailAddress( $param->{owner} );
+    return if !$toaddr;
+
+    #prepare letter
+    my $letter= C4::Letters::GetPreparedLetter(
+        module => 'members',
+        letter_code => 'SHARE_ACCEPT',
+        branchcode => C4::Context->userenv->{"branch"},
+        tables => { borrowers => $param->{loggedinuser}, },
+        substitute => {
+            listname => $param->{shelfname},
+        },
+    );
+
+    #send letter to queue
+    C4::Letters::EnqueueLetter( {
+        letter                 => $letter,
+        message_transport_type => 'email',
+        from_address => C4::Context->preference('KohaAdminEmailAddress'),
+        to_address             => $toaddr,
+    });
 }
 
 sub process_addrlist {
     my ($param) = @_;
     my @temp= split /[,:;]/, $param->{addrlist};
     my @appr_addr;
-    my $fail_addr='';
+    my @fail_addr;
     foreach my $a (@temp) {
         $a=~s/^\s+//;
         $a=~s/\s+$//;
@@ -109,11 +162,11 @@ sub process_addrlist {
             push @appr_addr, $a;
         }
         else {
-            $fail_addr.= ($fail_addr? '; ': '').$a;
+            push @fail_addr, $a;
         }
     }
     $param->{appr_addr}= \@appr_addr;
-    $param->{fail_addr}= $fail_addr;
+    $param->{fail_addr}= \@fail_addr;
 }
 
 sub send_invitekey {
@@ -124,9 +177,17 @@ sub send_invitekey {
         $param->{shelfnumber}."&op=accept&key=";
         #TODO Waiting for the right http or https solution (BZ 8952 a.o.)
 
+    my @ok; #the addresses that were processed well
     foreach my $a ( @{$param->{appr_addr}} ) {
         my @newkey= randomlist(KEYLENGTH, 64); #generate a new key
 
+        #add a preliminary share record
+        if( ! AddShare( $param->{shelfnumber}, keytostring(\@newkey,1) ) ) {
+            push @{$param->{fail_addr}}, $a;
+            next;
+        }
+        push @ok, $a;
+
         #prepare letter
         my $letter= C4::Letters::GetPreparedLetter(
             module => 'members',
@@ -146,21 +207,16 @@ sub send_invitekey {
             from_address           => $fromaddr,
             to_address             => $a,
         });
-        #add a preliminary share record
-        AddShare( $param->{shelfnumber}, keytostring(\@newkey,1));
     }
+    $param->{appr_addr}= \@ok;
 }
 
 sub check_owner_category {
     my ($param)= @_;
-    #TODO candidate for a module?
-    #need to get back the two different error codes and the shelfname
-
-    ( undef, $param->{shelfname}, $param->{owner}, my $category ) =
-    GetShelf( $param->{shelfnumber} );
+    #sharing user should be the owner
+    #list should be private
     $param->{errcode}=4 if $param->{owner}!= $param->{loggedinuser};
-    $param->{errcode}=5 if !$param->{errcode} && $category!=1;
-        #should be private
+    $param->{errcode}=5 if !$param->{errcode} && $param->{category}!=1;
     return !defined $param->{errcode};
 }
 
@@ -178,14 +234,15 @@ sub load_template {
 sub load_template_vars {
     my ($param) = @_;
     my $template = $param->{template};
-    my $str= join '; ', @{$param->{appr_addr}};
+    my $appr= join '; ', @{$param->{appr_addr}};
+    my $fail= join '; ', @{$param->{fail_addr}};
     $template->param(
         errcode         => $param->{errcode},
         op              => $param->{op},
         shelfnumber     => $param->{shelfnumber},
         shelfname       => $param->{shelfname},
-        approvedaddress => $str,
-        failaddress     => $param->{fail_addr},
+        approvedaddress => $appr,
+        failaddress     => $fail,
     );
 }
 
@@ -214,14 +271,14 @@ sub stringtokey {
     my @temp=split '', $str||'';
     if($flgBase64) {
         my $alphabet= [ 'A'..'Z', 'a'..'z', 0..9, '+', '/' ];
-        return map { alphabet_ordinal($_, $alphabet); } @temp;
+        return [ map { alphabet_ordinal($_, $alphabet); } @temp ];
     }
-    return () if $str!~/^\d+$/;
+    return [] if $str!~/^\d+$/;
     my @retval;
     for(my $i=0; $i<@temp-1; $i+=2) {
         push @retval, $temp[$i]*10+$temp[$i+1];
     }
-    return @retval;
+    return \@retval;
 }
 
 sub alphabet_ordinal {