Attachment 20033 Details for Bug 10389 – Bug 10389: Share a list (part 2: accept the invitation)

[patch] Bug 10389: Share a list (part 2: accept the invitation)

Bug-10389-Share-a-list-part-2-accept-the-invitatio.patch (text/plain), 15.99 KB, created by Marcel de Rooy on 2013-08-01 10:13:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2013-08-01 10:13:22 UTC

Size: 15.99 KB

patch

obsolete

>From 5bf18cb916e35e3f69eff82fce87f24759e3da4d Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Fri, 12 Jul 2013 16:30:05 +0200
>Subject: [PATCH] Bug 10389: Share a list (part 2: accept the invitation)
>Content-Type: text/plain; charset=utf-8
>
>What it does:
>As a followup on report 9032 where you can send an invitation to share a
>private list, this report handles the response 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.
>
>Test plan:
>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 in opac or staff: P1 shows Private with options
>user 2 accepts share: sees P1, but cannot add or delete items
>user 2 checks your lists display again: P1 shows Shared without options
>user 1 checks your lists display again: P1 shows Shared with options
>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 options
>user 1 accepts, adds one item
>user 1 checks your lists display: P2 shows Shared without options
>user 2 checks your lists display: P2 shows Shared with options
>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
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> C4/VirtualShelves.pm                               |   75 +++++++++++++++++++-
> C4/VirtualShelves/Page.pm                          |    1 +
> .../prog/en/modules/admin/preferences/opac.pref    |    2 +-
> .../prog/en/modules/virtualshelves/shelves.tt      |    2 +-
> .../opac-tmpl/prog/en/modules/opac-shareshelf.tt   |    7 +-
> .../opac-tmpl/prog/en/modules/opac-shelves.tt      |   11 ++--
> opac/opac-shareshelf.pl                            |   46 +++++++-----
> 7 files changed, 114 insertions(+), 30 deletions(-)
>
>diff --git a/C4/VirtualShelves.pm b/C4/VirtualShelves.pm
>index f464199..4531564 100644
>--- a/C4/VirtualShelves.pm
>+++ b/C4/VirtualShelves.pm
>@@ -44,7 +44,7 @@ BEGIN {
>             &ModShelf
>             &ShelfPossibleAction
>             &DelFromShelf &DelShelf
>-            &GetBibliosShelves &AddShare
>+            &GetBibliosShelves &AddShare &AcceptShare &IsSharedList
>     );
>         @EXPORT_OK = qw(
>             &GetAllShelves &ShelvesMax
>@@ -434,6 +434,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 +442,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 +493,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;
>     }
>@@ -667,6 +692,54 @@ sub AddShare {
>     $dbh->do($sql, undef, ($shelfnumber, $key, SHARE_INVITATION_EXPIRY_DAYS));
> }
> 
>+=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);
>+}
>+
> # internal subs
> 
> sub _shelf_count {
>diff --git a/C4/VirtualShelves/Page.pm b/C4/VirtualShelves/Page.pm
>index 7756d92..b2bd238 100644
>--- a/C4/VirtualShelves/Page.pm
>+++ b/C4/VirtualShelves/Page.pm
>@@ -434,6 +434,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} );
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
>index 90295ef..d71da8a 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
>@@ -509,7 +509,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:
>         -
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
>index aac163f..05c3e60 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
>@@ -544,7 +544,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>
>diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-shareshelf.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-shareshelf.tt
>index 63c17bc..1c3dad6 100644
>--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-shareshelf.tt
>+++ b/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()!='';">
>@@ -45,10 +47,7 @@
>         <p>You will be notified if someone accepts your share within two weeks.</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 %]
> 
>diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt
>index 36288ca..b429717 100644
>--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt
>+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt
>@@ -577,9 +577,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 ) %]
>@@ -604,7 +604,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 ) %]
>@@ -615,7 +615,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>
>@@ -677,7 +677,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 ) %]
>diff --git a/opac/opac-shareshelf.pl b/opac/opac-shareshelf.pl
>index 6c95f0d..6b9d21a 100755
>--- a/opac/opac-shareshelf.pl
>+++ b/opac/opac-shareshelf.pl
>@@ -34,7 +34,7 @@ use C4::VirtualShelves;
> #-------------------------------------------------------------------------------
> 
> my $query= new CGI;
>-my ($shelfname, $owner);
>+my $shelfname;
> my ($template, $loggedinuser, $cookie);
> my $errcode=0;
> my (@addr, $fail_addr, @newkey);
>@@ -49,17 +49,19 @@ my $key= $query->param('key')||'';
> 
> check_common_errors();
> load_template("opac-shareshelf.tmpl");
>+(undef, $shelfname)= GetShelf($shelfnumber);
> if($errcode) {
>     #nothing to do
> }
> elsif($op eq 'invite') {
>-    show_invite();
>+    check_invite();
> }
> elsif($op eq 'conf_invite') {
>-    confirm_invite();
>+    check_invite();
>+    confirm_invite() if !$errcode;
> }
> elsif($op eq 'accept') {
>-    show_accept();
>+    do_accept();
> }
> load_template_vars();
> output_html_with_http_headers $query, $cookie, $template->output;
>@@ -81,12 +83,14 @@ sub check_common_errors {
>     }
> }
> 
>-sub show_invite {
>-    return unless check_owner_category();
>+sub check_invite {
>+    my @rv= ShelfPossibleAction($loggedinuser, $shelfnumber, 'invite');
>+    $errcode= $rv[1] if !$rv[0];
>+        #errorcode 4: should be owner
>+        #errorcode 5: should be private list
> }
> 
> sub confirm_invite {
>-    return unless check_owner_category();
>     process_addrlist();
>     if(@addr) {
>         send_invitekey();
>@@ -96,8 +100,22 @@ sub confirm_invite {
>     }
> }
> 
>-sub show_accept {
>-    #TODO Add some code here to accept an invitation (followup report)
>+sub do_accept {
>+    my @rv= ShelfPossibleAction($loggedinuser, $shelfnumber, 'acceptshare');
>+    $errcode= $rv[1] if !$rv[0];
>+    return if $errcode;
>+        #errorcode 5: should be private list
>+        #errorcode 8: should not be owner
>+
>+    my $dbkey= keytostring( stringtokey($key, 0), 1);
>+    if( AcceptShare($shelfnumber, $dbkey, $loggedinuser) ) {
>+        #redirect to view of this shared list
>+        print $query->redirect("/cgi-bin/koha/opac-shelves.pl?display=privateshelves&viewshelf=$shelfnumber");
>+        exit;
>+    }
>+    else {
>+        $errcode= 7; #not accepted (key not found or expired)
>+    }
> }
> 
> sub process_addrlist {
>@@ -149,14 +167,6 @@ sub send_invitekey {
>     }
> }
> 
>-sub check_owner_category {
>-    #FIXME candidate for a module? what held me back is: getting back the two different error codes and the shelfname
>-    (undef,$shelfname,$owner,my $category)= GetShelf($shelfnumber);
>-    $errcode=4 if $owner!= $loggedinuser; #should be owner
>-    $errcode=5 if !$errcode && $category!=1; #should be private
>-    return $errcode==0;
>-}
>-
> sub load_template {
>     my ($file)= @_;
>     ($template, $loggedinuser, $cookie)= get_template_and_user({
>@@ -208,7 +218,7 @@ sub stringtokey {
>     for(my $i=0; $i<@temp-1; $i+=2) {
>         push @retval, $temp[$i]*10+$temp[$i+1];
>     }
>-    return @retval;
>+    return \@retval;
> }
> 
> sub base64ord { #base64 ordinal
>-- 
>1.7.7.6

Actions: View | Diff | Splinter Review

Attachments on bug 10389: 19602 | 20033 | 20034 | 20038