Attachment 10051 Details for Bug 6679 – Bug 6679: Fixing 23 perlcritic violations in C4/Tags.pm

[patch] Bug 6679: Fixing 23 perlcritic violations in C4/Tags.pm

Bug-6679-Fixing-23-perlcritic-violations-in-C4Tags.patch (text/plain), 7.12 KB, created by Jared Camins-Esakov on 2012-06-09 19:38:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jared Camins-Esakov

Created: 2012-06-09 19:38:31 UTC

Size: 7.12 KB

patch

obsolete

>From 14a9bf30c4eae8661ff49481dcd12cc482e7d3a0 Mon Sep 17 00:00:00 2001
>From: Mason James <mtj@kohaaloha.com>
>Date: Thu, 24 May 2012 08:16:01 +1200
>Subject: [PATCH] Bug 6679: Fixing 23 perlcritic violations in C4/Tags.pm
>Content-Type: text/plain; charset="UTF-8"
>
>return" statement with explicit "undef" at line XXX, column 26.  See page 199
>of PBP.  (Severity: 5)
>
>note: i have tested the Tags feature OK, after appliying this patch
>
>Note: Rebased on master 06/09/2012 by jcamins
>---
> C4/Tags.pm |   46 +++++++++++++++++++++++-----------------------
> 1 files changed, 23 insertions(+), 23 deletions(-)
>
>diff --git a/C4/Tags.pm b/C4/Tags.pm
>index 700be18..8ca2f4c 100644
>--- a/C4/Tags.pm
>+++ b/C4/Tags.pm
>@@ -117,13 +117,13 @@ sub get_count_by_tag_status  {
> }
> 
> sub remove_tag {
>-	my $tag_id  = shift or return undef;
>+    my $tag_id  = shift or return;
> 	my $user_id = (@_) ? shift : undef;
> 	my $rows = (defined $user_id) ?
> 			get_tag_rows({tag_id=>$tag_id, borrowernumber=>$user_id}) :
> 			get_tag_rows({tag_id=>$tag_id}) ;
> 	$rows or return 0;
>-	(scalar(@$rows) == 1) or return undef;	# should never happen (duplicate ids)
>+   (scalar(@$rows) == 1) or return;	# should never happen (duplicate ids)
> 	my $row = shift(@$rows);
> 	($tag_id == $row->{tag_id}) or return 0;
> 	my $tags = get_tags({term=>$row->{term}, biblionumber=>$row->{biblionumber}});
>@@ -145,25 +145,25 @@ sub remove_tag {
> }
> 
> sub delete_tag_index {
>-	(@_) or return undef;
>+      (@_) or return;
> 	my $sth = C4::Context->dbh->prepare("DELETE FROM tags_index WHERE term = ? AND biblionumber = ? LIMIT 1");
> 	$sth->execute(@_);
> 	return $sth->rows || 0;
> }
> sub delete_tag_approval {
>-	(@_) or return undef;
>+   (@_) or return;
> 	my $sth = C4::Context->dbh->prepare("DELETE FROM tags_approval WHERE term = ? LIMIT 1");
> 	$sth->execute(shift);
> 	return $sth->rows || 0;
> }
> sub delete_tag_row_by_id {
>-	(@_) or return undef;
>+ (@_) or return;
> 	my $sth = C4::Context->dbh->prepare("DELETE FROM tags_all WHERE tag_id = ? LIMIT 1");
> 	$sth->execute(shift);
> 	return $sth->rows || 0;
> }
> sub delete_tag_rows_by_ids {
>-	(@_) or return undef;
>+  (@_) or return;
> 	my $i=0;
> 	foreach(@_) {
> 		$i += delete_tag_row_by_id($_);
>@@ -359,7 +359,7 @@ sub get_approval_rows {		# i.e., from tags_approval
> }
> 
> sub is_approved {
>-	my $term = shift or return undef;
>+   my $term = shift or return;
> 	my $sth = C4::Context->dbh->prepare("SELECT approved FROM tags_approval WHERE term = ?");
> 	$sth->execute($term);
> 	unless ($sth->rows) {
>@@ -370,7 +370,7 @@ sub is_approved {
> }
> 
> sub get_tag_index {
>-	my $term = shift or return undef;
>+ my $term = shift or return;
> 	my $sth;
> 	if (@_) {
> 		$sth = C4::Context->dbh->prepare("SELECT * FROM tags_index WHERE term = ? AND biblionumber = ?");
>@@ -384,7 +384,7 @@ sub get_tag_index {
> 
> sub whitelist {
> 	my $operator = shift;
>-	defined $operator or return undef; # have to test defined to allow =0 (kohaadmin)
>+        defined $operator or return; # have to test defined to allow =0 (kohaadmin)
> 	if ($ext_dict) {
> 		foreach (@_) {
> 			spellcheck($_) or next;
>@@ -406,7 +406,7 @@ sub whitelist {
> # a term mistakenly, you can still reverse it. But there is no going back to "neutral".
> sub blacklist {
> 	my $operator = shift;
>-	defined $operator or return undef; # have to test defined to allow =0 (kohaadmin)
>+ defined $operator or return; # have to test defined to allow =0 (kohaadmin)
> 	foreach (@_) {
> 		my $aref = get_approval_rows({term=>$_});
> 		if ($aref and scalar @$aref) {
>@@ -419,14 +419,14 @@ sub blacklist {
> }
> sub add_filter {
> 	my $operator = shift;
>-	defined $operator or return undef; # have to test defined to allow =0 (kohaadmin)
>+      defined $operator or return; # have to test defined to allow =0 (kohaadmin)
> 	my $query = "INSERT INTO tags_blacklist (regexp,y,z) VALUES (?,?,?)";
> 	# my $sth = C4::Context->dbh->prepare($query);
> 	return scalar @_;
> }
> sub remove_filter {
> 	my $operator = shift;
>-	defined $operator or return undef; # have to test defined to allow =0 (kohaadmin)
>+     defined $operator or return; # have to test defined to allow =0 (kohaadmin)
> 	my $query = "REMOVE FROM tags_blacklist WHERE blacklist_id = ?";
> 	# my $sth = C4::Context->dbh->prepare($query);
> 	# $sth->execute($term);
>@@ -435,7 +435,7 @@ sub remove_filter {
> 
> sub add_tag_approval {	# or disapproval
> 	$debug and warn "add_tag_approval(" . join(", ",map {defined($_) ? $_ : 'UNDEF'} @_) . ")";
>-	my $term = shift or return undef;
>+  my $term = shift or return;
> 	my $query = "SELECT * FROM tags_approval WHERE term = ?";
> 	my $sth = C4::Context->dbh->prepare($query);
> 	$sth->execute($term);
>@@ -460,8 +460,8 @@ sub add_tag_approval {	# or disapproval
> 
> sub mod_tag_approval {
> 	my $operator = shift;
>-	defined $operator or return undef; # have to test defined to allow =0 (kohaadmin)
>-	my $term     = shift or return undef;
>+ defined $operator or return; # have to test defined to allow =0 (kohaadmin)
>+    my $term     = shift or return;
> 	my $approval = (scalar @_ ? shift : 1);	# default is to approve
> 	my $query = "UPDATE tags_approval SET approved_by=?, approved=?, date_approved=NOW() WHERE term = ?";
> 	$debug and print STDERR "mod_tag_approval query: $query\nmod_tag_approval args: ($operator,$approval,$term)\n";
>@@ -470,8 +470,8 @@ sub mod_tag_approval {
> }
> 
> sub add_tag_index {
>-	my $term         = shift or return undef;
>-	my $biblionumber = shift or return undef;
>+ my $term         = shift or return;
>+    my $biblionumber = shift or return;
> 	my $query = "SELECT * FROM tags_index WHERE term = ? AND biblionumber = ?";
> 	my $sth = C4::Context->dbh->prepare($query);
> 	$sth->execute($term,$biblionumber);
>@@ -484,7 +484,7 @@ sub add_tag_index {
> }
> 
> sub get_tag {		# by tag_id
>-	(@_) or return undef;
>+  (@_) or return;
> 	my $sth = C4::Context->dbh->prepare("$select_all WHERE tag_id = ?");
> 	$sth->execute(shift);
> 	return $sth->fetchrow_hashref;
>@@ -505,7 +505,7 @@ sub rectify_weights {
> 	} else {
> 		$sth->execute();
> 	}
>-	my $results = $sth->fetchall_arrayref({}) or return undef;
>+        my $results = $sth->fetchall_arrayref({}) or return;
> 	my %tally = ();
> 	foreach (@$results) {
> 		_set_weight($_->{count},$_->{term},$_->{biblionumber});
>@@ -557,12 +557,12 @@ sub _set_weight {
> }
> 
> sub add_tag {	# biblionumber,term,[borrowernumber,approvernumber]
>-	my $biblionumber = shift or return undef;
>-	my $term         = shift or return undef;
>+   my $biblionumber = shift or return;
>+    my $term         = shift or return;
> 	my $borrowernumber = (@_) ? shift : 0;		# the user, default to kohaadmin
> 	$term =~ s/^\s+//;
> 	$term =~ s/\s+$//;
>-	($term) or return undef;	# must be more than whitespace
>+  ($term) or return;	# must be more than whitespace
> 	my $rows = get_tag_rows({biblionumber=>$biblionumber, borrowernumber=>$borrowernumber, term=>$term, limit=>1});
> 	my $query = "INSERT INTO tags_all
> 	(borrowernumber,biblionumber,term,date_created)
>@@ -571,7 +571,7 @@ sub add_tag {	# biblionumber,term,[borrowernumber,approvernumber]
> 							"add_tag query args: ($borrowernumber,$biblionumber,$term)\n";
> 	if (scalar @$rows) {
> 		$debug and carp "Duplicate tag detected.  Tag not added.";	
>-		return undef;
>+              return;
> 	}
> 	# add to tags_all regardless of approaval
> 	my $sth = C4::Context->dbh->prepare($query);
>-- 
>1.7.2.5

Actions: View | Diff | Splinter Review

Attachments on bug 6679: 4883 | 4884 | 4885 | 4886 | 4887 | 4888 | 5445 | 5446 | 5483 | 5487 | 6121 | 6122 | 6123 | 6139 | 6140 | 6141 | 6142 | 6143 | 6144 | 6270 | 6271 | 6663 | 6670 | 6675 | 6676 | 6677 | 6678 | 8542 | 8897 | 8898 | 8899 | 8950 | 8951 | 9746 | 9747 | 9748 | 9749 | 9750 | 10050 | 10051 | 10052 | 10053 | 10054 | 10075 | 10076 | 10077 | 10078 | 10079 | 10484 | 10485 | 10486 | 10487 | 10488 | 10489 | 10491 | 10492 | 10493 | 10494 | 10495 | 10496 | 10497 | 10499 | 10500 | 10501 | 10502 | 10503 | 10504 | 10505 | 10506 | 10507 | 10508 | 10509 | 10510 | 10512 | 10513 | 10514 | 10515 | 10517 | 10521 | 10522 | 10523 | 10524 | 10525 | 10526 | 10527 | 10935 | 10936 | 10938 | 10940 | 10944 | 10945 | 10946 | 10947 | 10948 | 10949 | 10950 | 10952 | 10953 | 10955 | 10957 | 10958 | 10959 | 10960 | 10961 | 10962 | 10963 | 10964 | 10965 | 10966 | 10967 | 10972 | 10976 | 10979 | 10980 | 10981 | 12224 | 12363 | 12490 | 12491 | 12492 | 34191 | 34192 | 34193 | 34194 | 34195 | 34196 | 34197 | 39100 | 39101 | 39102 | 39392 | 39393 | 39394 | 39395 | 39396 | 39397 | 39398 | 42770 | 47086 | 47087 | 47088 | 47089 | 47090 | 47178 | 47179 | 47180 | 47181 | 47205 | 47206 | 47207 | 47208