From 83378de477d6ca07a051cc5a996378187479c5df Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 12 Jul 2013 16:30:05 +0200 Subject: [PATCH] Bug 9032: Share a list (accept invitation and remove a share) Content-Type: text/plain; charset=utf-8 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 Rebased on March 3, 2014. Combined changes for AcceptShare and RemoveShare. --- C4/VirtualShelves.pm | 98 +++++++++++++++++++- C4/VirtualShelves/Page.pm | 14 +++- .../prog/en/modules/admin/preferences/opac.pref | 2 +- .../prog/en/modules/virtualshelves/shelves.tt | 2 +- .../opac-tmpl/prog/en/modules/opac-shareshelf.tt | 13 ++- .../opac-tmpl/prog/en/modules/opac-shelves.tt | 29 +++++- opac/opac-shareshelf.pl | 97 +++++++++++++++---- 7 files changed, 220 insertions(+), 35 deletions(-) diff --git a/C4/VirtualShelves.pm b/C4/VirtualShelves.pm index 8aee037..a43bbf7 100644 --- a/C4/VirtualShelves.pm +++ b/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 diff --git a/C4/VirtualShelves/Page.pm b/C4/VirtualShelves/Page.pm index 259e12d..609dcca 100644 --- a/C4/VirtualShelves/Page.pm +++ b/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} ); 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 d541440..e53a6ae 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 @@ -521,7 +521,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 aca9485..b2db5e4 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt @@ -538,7 +538,7 @@ function placeHold () { [% shelveslooppri.shelfname |html %] [% shelveslooppri.count %] item(s) [% IF ( shelveslooppri.sortfield == "author" ) %]Author[% ELSIF ( shelveslooppri.sortfield == "copyrightdate" ) %]Year[% ELSIF (shelveslooppri.sortfield == "itemcallnumber") %]Call number[% ELSE %]Title[% END %] - [% IF ( shelveslooppri.viewcategory1 ) %]Private[% END %] + [% IF ( shelveslooppri.viewcategory1 ) %][% IF !shelveslooppri.shares %]Private[% ELSE %]Shared[% END %][% END %] [% IF ( shelveslooppri.viewcategory2 ) %]Public[% END %] 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 58a2b51..9d00492 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 %]
You can only share a list if you are the owner.
[% END %] [% IF errcode==5 %]
You cannot share a public list.
[% END %] [% IF errcode==6 %]
Sorry, but you did not enter any valid email address.
[% END %] + [% IF errcode==7 %]
Sorry, but we could not accept this key. The invitation may have expired. Contact the patron who sent you the invitation.
[% END %] + [% IF errcode==8 %]
As owner of a list you cannot accept an invitation for sharing it.
[% END %] [% ELSIF op=='invite' %]
@@ -37,18 +39,19 @@
[% ELSIF op=='conf_invite' %] + [% IF approvedaddress %]

An invitation to share list [% shelfname %] has been sent to [% approvedaddress %].

+ [% END %] [% IF failaddress %] -

The following addresses appear to be invalid. Please correct them and try again. These are: [% failaddress %]

+

Something went wrong while processing the following addresses. Please check them. These are: [% failaddress %]

[% END %] + [% IF approvedaddress %]

You will receive an email notification if someone accepts your share within two weeks.

+ [% END %]

Return to your lists

[% ELSIF op=='accept' %] - [%# TODO: Replace the following two lines %] -

Thank you for testing this feature.

-

Your signoff will certainly help in finishing the remaining part!

- + [%# 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 d6b2042..5bbebca 100644 --- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt +++ b/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() { [% END %] +[%# When using the next block, add the parameter for shelfnumber and add a tag to end the form %] +[% BLOCK remove_share %] +
+ + + + +[% END %] + [% IF ( OpacNav ) %]
[% ELSIF ( loggedinusername ) %]
[% ELSE %]
[% END %]
[% INCLUDE 'masthead.inc' %] @@ -371,6 +381,10 @@ $(document).ready(function() { [% END %] + [% ELSIF showprivateshelves %] + [% INCLUDE remove_share %] + + [% END %] @@ -596,9 +610,9 @@ $(document).ready(function() {