Bugzilla – Attachment 25851 Details for
Bug 9032
Share a list
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 9032: [QA Follow-up] Plackify opac-shareshelf.pl
Bug-9032-QA-Follow-up-Plackify-opac-shareshelfpl.patch (text/plain), 10.33 KB, created by
Marcel de Rooy
on 2014-03-05 14:39:54 UTC
(
hide
)
Description:
Bug 9032: [QA Follow-up] Plackify opac-shareshelf.pl
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2014-03-05 14:39:54 UTC
Size:
10.33 KB
patch
obsolete
>From 3c049f2f58d17a72bfc0b61ff72a5f3ec8eba8e2 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Fri, 28 Feb 2014 21:41:09 +0100 >Subject: [PATCH] Bug 9032: [QA Follow-up] Plackify opac-shareshelf.pl >Content-Type: text/plain; charset=utf-8 > >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). >--- > opac/opac-shareshelf.pl | 185 +++++++++++++++++++++++++---------------------- > 1 files changed, 99 insertions(+), 86 deletions(-) > >diff --git a/opac/opac-shareshelf.pl b/opac/opac-shareshelf.pl >index 6c95f0d..3055fa7 100755 >--- a/opac/opac-shareshelf.pl >+++ b/opac/opac-shareshelf.pl >@@ -6,7 +6,7 @@ > # > # Koha is free software; you can redistribute it and/or modify it under the > # terms of the GNU General Public License as published by the Free Software >-# Foundation; either version 2 of the License, or (at your option) any later >+# Foundation; either version 3 of the License, or (at your option) any later > # version. > # > # Koha is distributed in the hope that it will be useful, but WITHOUT ANY >@@ -21,6 +21,7 @@ use strict; > use warnings; > > use constant KEYLENGTH => 10; >+use constant TEMPLATE_NAME => 'opac-shareshelf.tmpl'; > > use CGI; > use Email::Valid; >@@ -33,106 +34,107 @@ use C4::VirtualShelves; > > #------------------------------------------------------------------------------- > >-my $query= new CGI; >-my ($shelfname, $owner); >-my ($template, $loggedinuser, $cookie); >-my $errcode=0; >-my (@addr, $fail_addr, @newkey); >-my @base64alphabet= ('A'..'Z', 'a'..'z', 0..9, '+', '/'); >- >-my $shelfnumber= $query->param('shelfnumber')||0; >-my $op= $query->param('op')||''; >-my $addrlist= $query->param('invite_address')||''; >-my $key= $query->param('key')||''; >+my $pvar= _init( {} ); >+if(! $pvar->{errcode} ) { >+ show_invite( $pvar ) if $pvar->{op} eq 'invite'; >+ confirm_invite( $pvar ) if $pvar->{op} eq 'conf_invite'; >+ show_accept( $pvar ) if $pvar->{op} eq 'accept'; >+} >+load_template_vars( $pvar ); >+output_html_with_http_headers $pvar->{query}, $pvar->{cookie}, >+ $pvar->{template}->output; > > #------------------------------------------------------------------------------- > >-check_common_errors(); >-load_template("opac-shareshelf.tmpl"); >-if($errcode) { >- #nothing to do >-} >-elsif($op eq 'invite') { >- show_invite(); >-} >-elsif($op eq 'conf_invite') { >- confirm_invite(); >-} >-elsif($op eq 'accept') { >- show_accept(); >-} >-load_template_vars(); >-output_html_with_http_headers $query, $cookie, $template->output; >+sub _init { >+ my ($param) = @_; >+ my $query = new CGI; >+ $param->{query} = $query; >+ $param->{shelfnumber} = $query->param('shelfnumber')||0; >+ $param->{op} = $query->param('op')||''; >+ $param->{addrlist} = $query->param('invite_address')||''; >+ $param->{key} = $query->param('key')||''; >+ $param->{appr_addr} = []; > >-#------------------------------------------------------------------------------- >+ $param->{errcode} = check_common_errors($param); >+ load_template($param); >+ return $param; >+} > > sub check_common_errors { >- if($op!~/^(invite|conf_invite|accept)$/) { >- $errcode=1; #no operation specified >- return; >+ my ($param) = @_; >+ if( $param->{op} !~ /^(invite|conf_invite|accept)$/ ) { >+ return 1; #no operation specified > } >- if($shelfnumber!~/^\d+$/) { >- $errcode=2; #invalid shelf number >- return; >+ if( $param->{shelfnumber} !~ /^\d+$/ ) { >+ return 2; #invalid shelf number > } >- if(!C4::Context->preference('OpacAllowSharingPrivateLists')) { >- $errcode=3; #not or no longer allowed? >- return; >+ if( ! C4::Context->preference('OpacAllowSharingPrivateLists') ) { >+ return 3; #not or no longer allowed? > } >+ return; > } > > sub show_invite { >- return unless check_owner_category(); >+ my ($param) = @_; >+ return unless check_owner_category( $param ); > } > > sub confirm_invite { >- return unless check_owner_category(); >- process_addrlist(); >- if(@addr) { >- send_invitekey(); >+ my ($param) = @_; >+ return unless check_owner_category( $param ); >+ process_addrlist( $param ); >+ if( @{$param->{appr_addr}} ) { >+ send_invitekey( $param ); > } > else { >- $errcode=6; #not one valid address >+ $param->{errcode}=6; #not one valid address > } > } > > sub show_accept { >+ my ($param) = @_; > #TODO Add some code here to accept an invitation (followup report) > } > > sub process_addrlist { >- my @temp= split /[,:;]/, $addrlist; >- $fail_addr=''; >+ my ($param) = @_; >+ my @temp= split /[,:;]/, $param->{addrlist}; >+ my @appr_addr; >+ my $fail_addr=''; > foreach my $a (@temp) { > $a=~s/^\s+//; > $a=~s/\s+$//; >- if(IsEmailAddress($a)) { >- push @addr, $a; >+ if( IsEmailAddress($a) ) { >+ push @appr_addr, $a; > } > else { > $fail_addr.= ($fail_addr? '; ': '').$a; > } > } >+ $param->{appr_addr}= \@appr_addr; >+ $param->{fail_addr}= $fail_addr; > } > > sub send_invitekey { >+ my ($param) = @_; > my $fromaddr= C4::Context->preference('KohaAdminEmailAddress'); >- my $url= 'http://'.C4::Context->preference('OPACBaseURL'); >- $url.= "/cgi-bin/koha/opac-shareshelf.pl?shelfnumber=$shelfnumber"; >- $url.= "&op=accept&key="; >- #FIXME Waiting for the right http or https solution (BZ 8952 a.o.) >+ my $url= 'http://'.C4::Context->preference('OPACBaseURL'). >+ "/cgi-bin/koha/opac-shareshelf.pl?shelfnumber=". >+ $param->{shelfnumber}."&op=accept&key="; >+ #TODO Waiting for the right http or https solution (BZ 8952 a.o.) > >- foreach my $a (@addr) { >- @newkey=randomlist(KEYLENGTH, 64); #generate a new key >+ foreach my $a ( @{$param->{appr_addr}} ) { >+ my @newkey= randomlist(KEYLENGTH, 64); #generate a new key > > #prepare letter > my $letter= C4::Letters::GetPreparedLetter( > module => 'members', > letter_code => 'SHARE_INVITE', > branchcode => C4::Context->userenv->{"branch"}, >- tables => { borrowers => $loggedinuser, }, >+ tables => { borrowers => $param->{loggedinuser}, }, > substitute => { >- listname => $shelfname, >+ listname => $param->{shelfname}, > shareurl => $url.keytostring(\@newkey,0), > }, > ); >@@ -145,41 +147,50 @@ sub send_invitekey { > to_address => $a, > }); > #add a preliminary share record >- AddShare($shelfnumber,keytostring(\@newkey,1)); >+ AddShare( $param->{shelfnumber}, keytostring(\@newkey,1)); > } > } > > 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; >+ 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} ); >+ $param->{errcode}=4 if $param->{owner}!= $param->{loggedinuser}; >+ $param->{errcode}=5 if !$param->{errcode} && $category!=1; >+ #should be private >+ return !defined $param->{errcode}; > } > > sub load_template { >- my ($file)= @_; >- ($template, $loggedinuser, $cookie)= get_template_and_user({ >- template_name => $file, >- query => $query, >+ my ($param)= @_; >+ ($param->{template}, $param->{loggedinuser}, $param->{cookie} ) = >+ get_template_and_user( { >+ template_name => TEMPLATE_NAME, >+ query => $param->{query}, > type => "opac", > authnotrequired => 0, #should be a user >- }); >+ } ); > } > > sub load_template_vars { >+ my ($param) = @_; >+ my $template = $param->{template}; >+ my $str= join '; ', @{$param->{appr_addr}}; > $template->param( >- errcode => $errcode, >- op => $op, >- shelfnumber => $shelfnumber, >- shelfname => $shelfname, >- approvedaddress => (join '; ', @addr), >- failaddress => $fail_addr, >+ errcode => $param->{errcode}, >+ op => $param->{op}, >+ shelfnumber => $param->{shelfnumber}, >+ shelfname => $param->{shelfname}, >+ approvedaddress => $str, >+ failaddress => $param->{fail_addr}, > ); > } > > sub IsEmailAddress { >- #FIXME candidate for a module? >+ #TODO candidate for a module? > return Email::Valid->address($_[0])? 1: 0; > } > >@@ -192,7 +203,8 @@ sub randomlist { > sub keytostring { > my ($keyref, $flgBase64)= @_; > if($flgBase64) { >- return join '', map { base64chr($_); } @$keyref; >+ my $alphabet= [ 'A'..'Z', 'a'..'z', 0..9, '+', '/' ]; >+ return join '', map { alphabet_char($_, $alphabet); } @$keyref; > } > return join '', map { sprintf("%02d",$_); } @$keyref; > } >@@ -201,7 +213,8 @@ sub stringtokey { > my ($str, $flgBase64)= @_; > my @temp=split '', $str||''; > if($flgBase64) { >- return map { base64ord($_); } @temp; >+ my $alphabet= [ 'A'..'Z', 'a'..'z', 0..9, '+', '/' ]; >+ return map { alphabet_ordinal($_, $alphabet); } @temp; > } > return () if $str!~/^\d+$/; > my @retval; >@@ -211,16 +224,16 @@ sub stringtokey { > return @retval; > } > >-sub base64ord { #base64 ordinal >- my ($char)=@_; >- return 0 -ord('A')+ord($char) if $char=~/[A-Z]/; >- return 26-ord('a')+ord($char) if $char=~/[a-z]/; >- return 52-ord('0')+ord($char) if $char=~/[0-9]/; >- return 62 if $char eq '+'; >- return 63 if $char eq '/'; >- return; >+sub alphabet_ordinal { >+ my ($char, $alphabet) = @_; >+ for( 0..$#$alphabet ) { >+ return $_ if $char eq $alphabet->[$_]; >+ } >+ return ''; #ignore missing chars > } > >-sub base64chr { #reverse operation for ord >- return $_[0]=~/^\d+$/ && $_[0]<64? $base64alphabet[$_[0]]: undef; >+sub alphabet_char { >+#reverse operation for ordinal; ignore invalid numbers >+ my ($num, $alphabet) = @_; >+ return $num =~ /^\d+$/ && $num<=$#$alphabet? $alphabet->[$num]: ''; > } >-- >1.7.7.6
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 9032
:
18260
|
18542
|
18543
|
18546
|
18547
|
18587
|
18619
|
18682
|
18692
|
19377
|
19378
|
19379
|
19394
|
19934
|
19935
|
25847
|
25848
|
25849
|
25850
|
25851
|
25852
|
25853
|
25862
|
25886
|
25893
|
26253
|
26291
|
26292
|
26293
|
26294
|
26295
|
26296
|
26297
|
26298
|
26299
|
26300
|
26302
|
26303
|
26351
|
26458
|
26509
|
26513
|
26577
|
26578
|
26579
|
26580
|
26581
|
26582
|
26583
|
26584
|
26585
|
26586
|
26587
|
26588