From 41dfcefaa82b7da417d4718e128e08093b2bdeda Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Tue, 7 Oct 2014 19:32:19 +0400 Subject: [PATCH] quips.cgi: code style, simplify show, add admin_table style --- quips.cgi | 137 +++++++++++------------ template/en/default/list/quips.html.tmpl | 41 ++++--- 2 files changed, 87 insertions(+), 91 deletions(-) diff --git a/quips.cgi b/quips.cgi index 9c2daa21c..50336e0a8 100755 --- a/quips.cgi +++ b/quips.cgi @@ -42,108 +42,107 @@ my $vars = {}; my $action = $ARGS->{action} || ""; my $token = $ARGS->{token}; -if ($action eq "show") { +if ($action eq "show") +{ # Read in the entire quip list - my $quipsref = $dbh->selectall_arrayref( - "SELECT quipid, userid, quip, approved FROM quips"); - - my $quips; - my @quipids; - foreach my $quipref (@$quipsref) { - my ($quipid, $userid, $quip, $approved) = @$quipref; - $quips->{$quipid} = {'userid' => $userid, 'quip' => $quip, - 'approved' => $approved}; - push(@quipids, $quipid); - } - - my $users; - my $sth = $dbh->prepare("SELECT login_name FROM profiles WHERE userid = ?"); - foreach my $quipid (@quipids) { - my $userid = $quips->{$quipid}{'userid'}; - if ($userid && not defined $users->{$userid}) { - ($users->{$userid}) = $dbh->selectrow_array($sth, undef, $userid); - } - } - $vars->{'quipids'} = \@quipids; - $vars->{'quips'} = $quips; - $vars->{'users'} = $users; - $vars->{'show_quips'} = 1; + my $quips = $dbh->selectall_hashref( + "SELECT q.quipid, q.userid, q.quip, q.approved, p.login_name". + " FROM quips q LEFT JOIN profiles p ON p.userid=q.userid", 'quipid' + ); + $vars->{quips} = $quips; + $vars->{show_quips} = 1; } -if ($action eq "add") { - (Bugzilla->params->{'quip_list_entry_control'} eq "closed") && - ThrowUserError("no_new_quips"); +if ($action eq "add") +{ + if (Bugzilla->params->{quip_list_entry_control} eq "closed") + { + ThrowUserError("no_new_quips"); + } - check_hash_token($token, ['create-quips']); - # Add the quip - my $approved = (Bugzilla->params->{'quip_list_entry_control'} eq "open") - || Bugzilla->user->in_group('admin') || 0; + check_hash_token($token, [ 'create-quips' ]); + + # Add the quip + my $approved = (Bugzilla->params->{quip_list_entry_control} eq "open") + || Bugzilla->user->in_group('admin') || 0; my $comment = $ARGS->{quip}; $comment || ThrowUserError("need_quip"); trick_taint($comment); # Used in a placeholder below - $dbh->do("INSERT INTO quips (userid, quip, approved) VALUES (?, ?, ?)", - undef, ($user->id, $comment, $approved)); + $dbh->do( + "INSERT INTO quips (userid, quip, approved) VALUES (?, ?, ?)", + undef, $user->id, $comment, $approved + ); - $vars->{'added_quip'} = $comment; + $vars->{added_quip} = $comment; } -if ($action eq 'approve') { - $user->in_group('admin') - || ThrowUserError("auth_failure", {group => "admin", - action => "approve", - object => "quips"}); +if ($action eq 'approve') +{ + $user->in_group('admin') || ThrowUserError("auth_failure", { + group => "admin", + action => "approve", + object => "quips", + }); + + check_hash_token($token, [ 'approve-quips' ]); - check_hash_token($token, ['approve-quips']); # Read in the entire quip list my $quipsref = $dbh->selectall_arrayref("SELECT quipid, approved FROM quips"); - my %quips; - foreach my $quipref (@$quipsref) { + foreach my $quipref (@$quipsref) + { my ($quipid, $approved) = @$quipref; $quips{$quipid} = $approved; } my @approved; my @unapproved; - foreach my $quipid (keys %quips) { + foreach my $quipid (keys %quips) + { # Must check for each quipid being defined for concurrency and # automated usage where only one quipid might be defined. my $quip = $ARGS->{"quipid_$quipid"} ? 1 : 0; - if (defined $ARGS->{"defined_quipid_$quipid"}) { - if($quips{$quipid} != $quip) { - if($quip) { - push(@approved, $quipid); - } else { - push(@unapproved, $quipid); + if (defined $ARGS->{"defined_quipid_$quipid"}) + { + if ($quips{$quipid} != $quip) + { + if ($quip) + { + push @approved, $quipid; + } + else + { + push @unapproved, $quipid; } } } } - $dbh->do("UPDATE quips SET approved = 1 WHERE quipid IN (" . - join(",", @approved) . ")") if($#approved > -1); - $dbh->do("UPDATE quips SET approved = 0 WHERE quipid IN (" . - join(",", @unapproved) . ")") if($#unapproved > -1); - $vars->{ 'approved' } = \@approved; - $vars->{ 'unapproved' } = \@unapproved; + $dbh->do("UPDATE quips SET approved = 1 WHERE quipid IN (" . join(",", @approved) . ")") if @approved; + $dbh->do("UPDATE quips SET approved = 0 WHERE quipid IN (" . join(",", @unapproved) . ")") if @unapproved; + $vars->{approved} = \@approved; + $vars->{unapproved} = \@unapproved; } -if ($action eq "delete") { - Bugzilla->user->in_group("admin") - || ThrowUserError("auth_failure", {group => "admin", - action => "delete", - object => "quips"}); - my $quipid = $ARGS->{quipid}; - ThrowCodeError("need_quipid") unless $quipid =~ /(\d+)/; - $quipid = $1; - check_hash_token($token, ['quips', $quipid]); +if ($action eq "delete") +{ + Bugzilla->user->in_group("admin") || ThrowUserError("auth_failure", { + group => "admin", + action => "delete", + object => "quips", + }); - ($vars->{'deleted_quip'}) = $dbh->selectrow_array( - "SELECT quip FROM quips WHERE quipid = ?", - undef, $quipid); + my $quipid = $ARGS->{quipid}; + ThrowCodeError("need_quipid") unless $quipid =~ /(\d+)/; + $quipid = $1; + check_hash_token($token, [ 'quips', $quipid ]); + + ($vars->{deleted_quip}) = $dbh->selectrow_array( + "SELECT quip FROM quips WHERE quipid = ?", undef, $quipid + ); $dbh->do("DELETE FROM quips WHERE quipid = ?", undef, $quipid); } $template->process("list/quips.html.tmpl", $vars) - || ThrowTemplateError($template->error()); + || ThrowTemplateError($template->error()); exit; diff --git a/template/en/default/list/quips.html.tmpl b/template/en/default/list/quips.html.tmpl index 186af591a..a6bf67447 100644 --- a/template/en/default/list/quips.html.tmpl +++ b/template/en/default/list/quips.html.tmpl @@ -34,7 +34,7 @@ [% IF added_quip %]

- Your quip '[% added_quip FILTER html %]' has been added. + Your quip '[% added_quip | html %]' has been added. [% IF Param("quip_list_entry_control") == "moderated" AND !user.in_group('admin') %] It will be used as soon as it gets approved. [% END %] @@ -45,7 +45,7 @@ [% IF deleted_quip %]

- The quip '[% deleted_quip FILTER html %]' has been deleted. + The quip '[% deleted_quip | html %]' has been deleted.

[% END %] @@ -54,7 +54,6 @@

[% approved.size %] quips approved and [% unapproved.size %] quips unapproved

[% END %] -

[% terms.Bugzilla %] will pick a random quip for the headline on each [% terms.bug %] list. @@ -72,7 +71,7 @@

+ value="[% issue_hash_token(['create-quips']) | html %]">

@@ -91,7 +90,7 @@

    [% FOREACH quipid = quipids %] [% NEXT IF NOT quips.$quipid.approved %] -
  • [% quips.$quipid.quip FILTER html %]
    • +
  • [% quips.$quipid.quip | html %]
    • [% END %]
[% ELSE %] @@ -104,40 +103,37 @@ - + value="[% issue_hash_token(['approve-quips']) | html %]"> +
- [% FOREACH quipid = quipids %] - - + [% FOREACH quipid = quips.keys.nsort %] + + + - [% END %]
Quip Author Action Approved
[% quips.$quipid.quip FILTER html %]
[% quips.$quipid.quip | html %][% quips.$quipid.login_name || "Unknown" | html %] - [% userid = quips.$quipid.userid %] - [% users.$userid FILTER html %] - [% "Unknown" IF NOT users.$userid %] - - + Delete - -
+

+

-
+
[% END %] [% ELSE %]