From d97c4b9665636392618447ca81e75d9f292705df Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 12 Jul 2013 16:30:05 +0200 Subject: [PATCH] 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 Signed-off-by: Dobrica Pavlinusic Signed-off-by: Jonathan Druart Signed-off-by: Galen Charlton --- C4/VirtualShelves.pm | 98 ++++++++++++++++++- C4/VirtualShelves/Page.pm | 14 ++- .../en/modules/admin/preferences/opac.pref | 2 +- .../prog/en/modules/virtualshelves/shelves.tt | 2 +- .../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 8aee037e8f..a43bbf7965 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 259e12d624..609dcca48b 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 39b03ceb83..5483e0a0b1 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 @@ -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: - 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 05f28f1a02..4d2ffee185 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 58a2b514b1..9d00492b45 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 46ac2edf66..bb9b366e5c 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 %] @@ -602,9 +616,9 @@ $(document).ready(function() {
    [% IF ( opacuserlogin ) %] [% IF ( showprivateshelves ) %] -
  • Your private lists
    • +
  • Your lists
    • [% ELSE %] -
  • Your private lists
    • +
  • Your lists
    • [% END %] [% END %] [% IF ( showpublicshelves ) %] @@ -629,7 +643,7 @@ $(document).ready(function() { List name Contents Type -   + Options [% FOREACH shelveslooppri IN shelveslooppriv %] [% UNLESS ( loop.odd ) %] @@ -640,7 +654,7 @@ $(document).ready(function() { [% shelveslooppri.shelfname |html %] [% IF ( shelveslooppri.count ) %][% shelveslooppri.count %] [% IF ( shelveslooppri.single ) %]item[% ELSE %]items[% END %][% ELSE %]Empty[% END %] - [% IF ( shelveslooppri.viewcategory1 ) %]Private[% END %] + [% IF ( shelveslooppri.viewcategory1 ) %][% IF !shelveslooppri.shares %]Private[% ELSE %]Shared[% END %][% END %] [% IF ( shelveslooppri.viewcategory2 ) %]Public[% END %] @@ -670,6 +684,10 @@ $(document).ready(function() { [% END %] + [% ELSIF shelveslooppri.shares %] + [% INCLUDE remove_share %] + + [% END %]  @@ -702,7 +720,8 @@ $(document).ready(function() { List name Contents - Type  + Type + Options [% FOREACH shelvesloo IN shelvesloop %] [% UNLESS ( loop.odd ) %] diff --git a/opac/opac-shareshelf.pl b/opac/opac-shareshelf.pl index 3055fa7102..1ab31bdf8f 100755 --- a/opac/opac-shareshelf.pl +++ b/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 { -- 2.39.5