From b173be41f6e36f7abd0134090b9906db3f2ffd30 Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Thu, 8 Sep 2005 23:58:56 +0000 Subject: Bug 306242: Templatize the 'update product' bit of editproducts.cgi - Patch by GavinS r=LpSolit a=justdave --- editproducts.cgi | 166 ++++++++++------ .../en/default/admin/products/footer.html.tmpl | 15 +- .../en/default/admin/products/updated.html.tmpl | 220 +++++++++++++++++++++ template/en/default/global/user-error.html.tmpl | 54 ++++- 4 files changed, 385 insertions(+), 70 deletions(-) create mode 100644 template/en/default/admin/products/updated.html.tmpl diff --git a/editproducts.cgi b/editproducts.cgi index ba8337189..46a2a2bad 100755 --- a/editproducts.cgi +++ b/editproducts.cgi @@ -23,7 +23,7 @@ # Dawn Endico # Joe Robins # Gavin Shelley -# Frédéric Buclin +# Fr��ic Buclin # Greg Hendricks # # Direct any questions on this source code to @@ -69,13 +69,33 @@ sub CheckProduct exit; } - unless (TestProduct $prod) { + unless (TestProduct($prod)) { print "Sorry, product '$prod' does not exist."; PutTrailer(); exit; } } +# For the transition period, as this file is templatised bit by bit, +# we need this routine, which does things properly, and will +# eventually be the only version. (The older versions assume a +# $template->put_header() call has been made) +sub CheckProductNew +{ + my $prod = shift; + + # do we have a product? + unless ($prod) { + ThrowUserError('product_not_specified'); + } + + unless (TestProduct($prod)) { + ThrowUserError('product_doesnt_exist', + {'product' => $prod}); + } +} + + # TestClassification: just returns if the specified classification does exists # CheckClassification: same check, optionally emit an error text @@ -121,7 +141,7 @@ sub CheckClassificationNew ThrowUserError('classification_not_specified'); } - unless (TestClassification $cl) { + unless (TestClassification($cl)) { ThrowUserError('classification_doesnt_exist', {'name' => $cl}); } @@ -342,7 +362,7 @@ if (!$action && !$product) { if ($action eq 'add') { if (Param('useclassification')) { - CheckClassification($classification); + CheckClassificationNew($classification); } $vars->{'classification'} = $classification; $template->process("admin/products/create.html.tmpl", $vars) @@ -689,7 +709,7 @@ if ($action eq 'delete') { # if ($action eq 'edit' || (!$action && $product)) { - CheckProduct($product); + CheckProductNew($product); trick_taint($product); my $product_id = get_product_id($product); my $classification_id=1; @@ -697,7 +717,7 @@ if ($action eq 'edit' || (!$action && $product)) { # If a product has been given with no classification associated # with it, take this information from the DB if ($classification) { - CheckClassificationProduct($classification, $product); + CheckClassificationProductNew($classification, $product); } else { $classification = $dbh->selectrow_array("SELECT classifications.name @@ -1012,9 +1032,9 @@ if ($action eq 'updategroupcontrols') { # # action='update' -> update the product # - if ($action eq 'update') { - $template->put_header("Update product"); + + $vars->{'classification'} = $classification; my $productold = trim($cgi->param('productold') || ''); my $description = trim($cgi->param('description') || ''); @@ -1034,25 +1054,25 @@ if ($action eq 'update') { my $checkvotes = 0; - CheckProduct($productold); + CheckProductNew($productold); my $product_id = get_product_id($productold); + my $stored_maxvotesperbug = $maxvotesperbug; if (!detaint_natural($maxvotesperbug)) { - print "Sorry, the max votes per bug must be an integer >= 0."; - PutTrailer($localtrailer); - exit; + ThrowUserError('prod_votes_per_bug_must_be_nonnegative', + {maxvotesperbug => $stored_maxvotesperbug}); } + my $stored_votesperuser = $votesperuser; if (!detaint_natural($votesperuser)) { - print "Sorry, the votes per user must be an integer >= 0."; - PutTrailer($localtrailer); - exit; + ThrowUserError('prod_votes_per_user_must_be_nonnegative', + {votesperuser => $stored_votesperuser}); } + my $stored_votestoconfirm = $votestoconfirm; if (!detaint_natural($votestoconfirm)) { - print "Sorry, the votes to confirm must be an integer >= 0."; - PutTrailer($localtrailer); - exit; + ThrowUserError('prod_votes_to_confirm_must_be_nonnegative', + {votestoconfirm => $stored_votestoconfirm}); } # Note that we got the $product_id using $productold above so it will @@ -1070,27 +1090,31 @@ if ($action eq 'update') { SendSQL("UPDATE products SET disallownew=$disallownew WHERE id=$product_id"); - print "Updated bug submit status.
\n"; + $vars->{'updated_bugsubmitstatus'} = 1; + $vars->{'new_bugsubmitstatus'} = $disallownew; } if ($description ne $descriptionold) { unless ($description) { - print "Sorry, I can't delete the description."; - $dbh->bz_unlock_tables(UNLOCK_ABORT); - PutTrailer($localtrailer); - exit; + ThrowUserError('prod_cant_delete_description', + {product => $productold}); } SendSQL("UPDATE products SET description=" . SqlQuote($description) . " WHERE id=$product_id"); - print "Updated description.
\n"; + $vars->{'updated_description'} = 1; + $vars->{'old_description'} = $descriptionold; + $vars->{'new_description'} = $description; } - if (Param('usetargetmilestone') && $milestoneurl ne $milestoneurlold) { + if (Param('usetargetmilestone') + && ($milestoneurl ne $milestoneurlold)) { SendSQL("UPDATE products SET milestoneurl=" . SqlQuote($milestoneurl) . " WHERE id=$product_id"); - print "Updated mile stone URL.
\n"; + $vars->{'updated_milestoneurl'} = 1; + $vars->{'old_milestoneurl'} = $milestoneurlold; + $vars->{'new_milestoneurl'} = $milestoneurl; } @@ -1098,7 +1122,10 @@ if ($action eq 'update') { SendSQL("UPDATE products SET votesperuser=$votesperuser WHERE id=$product_id"); - print "Updated votes per user.
\n"; + $vars->{'updated_votesperuser'} = 1; + $vars->{'old_votesperuser'} = $votesperuserold; + $vars->{'new_votesperuser'} = $votesperuser; + $checkvotes = 1; } @@ -1107,7 +1134,10 @@ if ($action eq 'update') { SendSQL("UPDATE products SET maxvotesperbug=$maxvotesperbug WHERE id=$product_id"); - print "Updated max votes per bug.
\n"; + $vars->{'updated_maxvotesperbug'} = 1; + $vars->{'old_maxvotesperbug'} = $maxvotesperbugold; + $vars->{'new_maxvotesperbug'} = $maxvotesperbug; + $checkvotes = 1; } @@ -1116,7 +1146,11 @@ if ($action eq 'update') { SendSQL("UPDATE products SET votestoconfirm=$votestoconfirm WHERE id=$product_id"); - print "Updated votes to confirm.
\n"; + + $vars->{'updated_votestoconfirm'} = 1; + $vars->{'old_votestoconfirm'} = $votestoconfirmold; + $vars->{'new_votestoconfirm'} = $votestoconfirm; + $checkvotes = 1; } @@ -1126,15 +1160,18 @@ if ($action eq 'update') { "WHERE value = " . SqlQuote($defaultmilestone) . " AND product_id = $product_id"); if (!FetchOneColumn()) { - print "Sorry, the milestone $defaultmilestone must be defined first."; - $dbh->bz_unlock_tables(UNLOCK_ABORT); - PutTrailer($localtrailer); - exit; + ThrowUserError('prod_must_define_defaultmilestone', + {product => $productold, + defaultmilestone => $defaultmilestone, + classification => $classification}); } SendSQL("UPDATE products " . "SET defaultmilestone = " . SqlQuote($defaultmilestone) . "WHERE id=$product_id"); - print "Updated default milestone.
\n"; + + $vars->{'updated_defaultmilestone'} = 1; + $vars->{'old_defaultmilestone'} = $defaultmilestoneold; + $vars->{'new_defaultmilestone'} = $defaultmilestone; } my $qp = SqlQuote($product); @@ -1142,30 +1179,33 @@ if ($action eq 'update') { if ($product ne $productold) { unless ($product) { - print "Sorry, I can't delete the product name."; - $dbh->bz_unlock_tables(UNLOCK_ABORT); - PutTrailer($localtrailer); - exit; + ThrowUserError('prod_cant_delete_name', + {product => $productold}); } if (lc($product) ne lc($productold) && TestProduct($product)) { - print "Sorry, product name '$product' is already in use."; - $dbh->bz_unlock_tables(UNLOCK_ABORT); - PutTrailer($localtrailer); - exit; + ThrowUserError('prod_name_already_in_use', + {product => $product}); } SendSQL("UPDATE products SET name=$qp WHERE id=$product_id"); - print "Updated product name.
\n"; + + $vars->{'updated_product'} = 1; + $vars->{'old_product'} = $productold; + $vars->{'new_product'} = $product; + } $dbh->bz_unlock_tables(); unlink "$datadir/versioncache"; if ($checkvotes) { + $vars->{'checkvotes'} = 1; + # 1. too many votes for a single user on a single bug. + my @toomanyvotes_list = (); if ($maxvotesperbug < $votesperuser) { - print "
Checking existing votes in this product for anybody who now has too many votes for a single bug."; + SendSQL("SELECT votes.who, votes.bug_id " . "FROM votes, bugs " . "WHERE bugs.bug_id = votes.bug_id " . @@ -1176,19 +1216,27 @@ if ($action eq 'update') { my ($who, $id) = (FetchSQLData()); push(@list, [$who, $id]); } + + foreach my $ref (@list) { my ($who, $id) = (@$ref); RemoveVotes($id, $who, "The rules for voting on this product has changed;\nyou had too many votes for a single bug."); my $name = DBID_to_name($who); - print qq{
Removed votes for bug $id from $name\n}; + + push(@toomanyvotes_list, + {id => $id, name => $name}); } + } + $vars->{'toomanyvotes'} = \@toomanyvotes_list; + + # 2. too many total votes for a single user. # This part doesn't work in the general case because RemoveVotes # doesn't enforce votesperuser (except per-bug when it's less # than maxvotesperbug). See RemoveVotes in globals.pl. - print "
Checking existing votes in this product for anybody who now has too many total votes."; + SendSQL("SELECT votes.who, votes.vote_count FROM votes, bugs " . "WHERE bugs.bug_id = votes.bug_id " . " AND bugs.product_id = $product_id"); @@ -1201,6 +1249,7 @@ if ($action eq 'update') { $counts{$who} += $count; } } + my @toomanytotalvotes_list = (); foreach my $who (keys(%counts)) { if ($counts{$who} > $votesperuser) { SendSQL("SELECT votes.bug_id FROM votes, bugs " . @@ -1212,37 +1261,36 @@ if ($action eq 'update') { RemoveVotes($id, $who, "The rules for voting on this product has changed; you had too many\ntotal votes, so all votes have been removed."); my $name = DBID_to_name($who); - print qq{
Removed votes for bug $id from $name\n}; + + push(@toomanytotalvotes_list, + {id => $id, name => $name}); } } } + $vars->{'toomanytotalvotes'} = \@toomanytotalvotes_list; + # 3. enough votes to confirm my $bug_list = $dbh->selectcol_arrayref("SELECT bug_id FROM bugs WHERE product_id = ? AND bug_status = 'UNCONFIRMED' AND votes >= ?", undef, ($product_id, $votestoconfirm)); - if (scalar(@$bug_list)) { - print "
Checking unconfirmed bugs in this product for any which now have sufficient votes."; - } + my @updated_bugs = (); foreach my $bug_id (@$bug_list) { my $confirmed = CheckIfVotedConfirmed($bug_id, $whoid); push (@updated_bugs, $bug_id) if $confirmed; } - $vars->{'type'} = "votes"; - $vars->{'mailrecipients'} = { 'changer' => $whoid }; - $vars->{'header_done'} = 1; + $vars->{'confirmedbugs'} = \@updated_bugs; + $vars->{'changer'} = $whoid; - foreach my $bug_id (@updated_bugs) { - $vars->{'id'} = $bug_id; - $template->process("bug/process/results.html.tmpl", $vars) - || ThrowTemplateError($template->error()); - } } - PutTrailer($localtrailer); + $vars->{'name'} = $product; + $template->process("admin/products/updated.html.tmpl", $vars) + || ThrowTemplateError($template->error()); + exit; } diff --git a/template/en/default/admin/products/footer.html.tmpl b/template/en/default/admin/products/footer.html.tmpl index 2244c69a1..157e71582 100644 --- a/template/en/default/admin/products/footer.html.tmpl +++ b/template/en/default/admin/products/footer.html.tmpl @@ -32,6 +32,9 @@ [% classification_url_part = BLOCK %]&classification= [%- classification FILTER url_quote %] [% END %] + [% classification_url_part_start = BLOCK %]classification= + [%- classification FILTER url_quote %] + [% END %] [% classification_text = BLOCK %] of classification '[% classification FILTER html %]' [% END %] @@ -61,16 +64,16 @@ [% classification_text %]" href="editproducts.cgi?action=edit&product= [%- name FILTER url_quote %][% classification_url_part %]"> - '[% name FILTER html %]' + '[% name FILTER html %]'. [% END %] +[%# Edit other products (in a classification if specified): %] [% UNLESS no_edit_other_products_link %] - Edit other products [% classification_text %]' - [%- classification FILTER html %]' + Edit other products + [% classification_text %]. [% END %] diff --git a/template/en/default/admin/products/updated.html.tmpl b/template/en/default/admin/products/updated.html.tmpl new file mode 100644 index 000000000..7c00c4ae5 --- /dev/null +++ b/template/en/default/admin/products/updated.html.tmpl @@ -0,0 +1,220 @@ +[%# 1.0@bugzilla.org %] +[%# The contents of this file are subject to the Mozilla Public + # License Version 1.1 (the "License"); you may not use this file + # except in compliance with the License. You may obtain a copy of + # the License at http://www.mozilla.org/MPL/ + # + # Software distributed under the License is distributed on an "AS + # IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or + # implied. See the License for the specific language governing + # rights and limitations under the License. + # + # The Original Code is the Bugzilla Bug Tracking System. + # + # The Initial Developer of the Original Code is Netscape Communications + # Corporation. Portions created by Netscape are + # Copyright (C) 1998 Netscape Communications Corporation. All + # Rights Reserved. + # + # Contributor(s): Gavin Shelley + #%] + +[%# INTERFACE: + # + # updated_XXX : boolean; is true when the 'XXX' field has been updated. + # old_XXX : ... string; old value of the field 'XXX'. + # new_XXX : ... string; new value of the field 'XXX'. + # + # updated_product: boolean; the name of the product was updated + # + # updated_description: boolean; the product description was updated + # + # updated_milestoneurl: boolean; the product milestone URL was updated + # + # updated_votesperuser: boolean; the votes per user was updated + # + # updated_maxvotesperbug: boolean; the max votes per bug was updated + # + # updated_votestoconfirm: boolean; the votes to confirm a bug was updated + # + # updated_defaultmilestone: boolean; the default milestone was updated + # + # updated_bugsubmitstatus: boolean; the open/closed for new bugs status + # was updated (no 'old_XXX' value) + # + # classification: string; The product classification (may be empty or missing) + # + # changer: string; user id of the user making the changes, used for mailing + # bug changes if necessary + # + # name: string; the product name + # + # checkvotes: boolean; is true if vote related fields have changed. If so, + # then the following parameters will be specified: + # + # toomanyvotes: list of hashes, each one with an 'id' and a 'name' hash key + # detailing the bug id and the username of users who had too + # many votes for a bug + # + # toomanytotalvotes: list of hashes, each one with an 'id' and a 'name' hash key + # detailing the bug id and the username of users who had + # too many total votes + # + # confirmedbugs: list of bug ids, which were confirmed by votes + # + #%] + +[% IF classification %] + [% classification_url_part = BLOCK %]&classification= + [%- classification FILTER url_quote %] + [% END %] + [% classification_text = BLOCK %] + of classification '[% classification FILTER html %]' + [% END %] +[% END %] + +[% title = BLOCK %]Updating Product '[% name FILTER html %]' + [% classification_text FILTER none %][% END %] +[% PROCESS global/header.html.tmpl + title = title + style_urls = ['skins/standard/admin.css'] +%] + +[% IF updated_product %] +

+ Updated product name from '[% old_product FILTER html %]' to + [% new_product FILTER html %]. +[% END %] + + +[% IF updated_description %] +

+ Updated description to:

+

+

[% new_description FILTER html %]

+[% END %] + +[% IF updated_bugsubmitstatus %] +

+ Product is now + [% IF new_bugsubmitstatus %] + closed to + [% ELSE %] + open for + [% END %] + new [% terms.bugs %]. +[% END %] + +[% IF updated_milestoneurl %] +

+ Updated milestone URL + [% IF old_milestoneurl != '' %] + from
' + [%- old_milestoneurl FILTER html %]' + [% END %] + to + [% IF new_milestoneurl != '' %] +
' + [%- new_milestoneurl FILTER html %]'. + [% ELSE %] + be empty. + [% END %] +

+[% END %] + +[% IF updated_defaultmilestone %] +

+ Updated default milestone from '[% old_defaultmilestone FILTER html %]' to + '[% new_defaultmilestone FILTER html %]'. +

+[% END %] + +[% IF updated_votesperuser %] +

+ Updated votes per user from + [%+ old_votesperuser FILTER html %] to + [%+ new_votesperuser FILTER html %]. +[% END %] + +[% IF updated_maxvotesperbug %] +

+ Updated maximum votes per [% terms.bug %] from + [%+ old_maxvotesperbug FILTER html %] to + [%+ new_maxvotesperbug FILTER html %]. +[% END %] + +[% IF updated_votestoconfirm %] +

+ Updated number of votes needed to confirm a [% terms.bug %] from + [%+ old_votestoconfirm FILTER html %] to + [%+ new_votestoconfirm FILTER html %]. +[% END %] + +[% UNLESS updated_bugsubmitstatus || + updated_description || + updated_milestoneurl || + updated_votesperuser || + updated_maxvotesperbug || + updated_votestoconfirm || + updated_defaultmilestone || + updated_product %] +

Nothing changed for product '[% name FILTER html %]'. +[% END %] + +[%# Note that this display of changed votes and/or confirmed bugs is + not very scalable. We could have a _lot_, and we just list them all. + One day we should limit this perhaps, or have a more scalable display %] + + +[% IF checkvotes %] +

+ +

Checking existing votes in this product for anybody who now + has too many votes for [% terms.abug %]...
+ [% IF toomanyvotes.size > 0 %] + [% FOREACH detail = toomanyvotes %] + →removed votes for [% terms.bug %] + [%- detail.id FILTER html %] from [% detail.name FILTER html %]
+ [% END %] + [% ELSE %] + →there were none. + [% END %] + +

Checking existing votes in this product for anybody + who now has too many total votes...
+ [% IF toomanytotalvotes.size > 0 %] + [% FOREACH detail = toomanytotalvotes %] + →removed votes for [% terms.bug %] + [%- detail.id FILTER html %] from [% detail.name FILTER html %]
+ [% END %] + [% ELSE %] + →there were none. + [% END %] + +

Checking unconfirmed [% terms.bugs %] in this product for any which now have + sufficient votes...
+ [% IF confirmedbugs.size > 0 %] + [% FOREACH id = confirmedbugs %] + + [%# This is INCLUDED instead of PROCESSED to avoid variables getting + overwritten, which happens otherwise %] + [% INCLUDE bug/process/results.html.tmpl + type = 'votes' + mailrecipients = { 'changer' => changer } + header_done = 1 + id = id + %] + [% END %] + [% ELSE %] + →there were none. + [% END %] + +[% END %] + +[% PROCESS admin/products/footer.html.tmpl %] + +[% PROCESS global/footer.html.tmpl %] diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 34340fc41..be6108e81 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -288,11 +288,6 @@ [% title = "Component Requires Default Assignee" %] You must enter a default assignee for component '[% name FILTER html %]'. - [% ELSIF error == "product_not_specified" %] - [% title = "No Product Specified" %] - No product specified when trying to edit components, milestones or - versions. - [% ELSIF error == "component_not_specified" %] [% title = "No Component Specified" %] No component specified when trying to edit components. @@ -951,6 +946,50 @@ Patches cannot be more than [% Param('maxpatchsize') %] KB in size. Try breaking your patch into several pieces. + [% ELSIF error == "prod_votes_per_bug_must_be_nonnegative" %] + [% title = "Maximum Votes Must Be Non-negative" %] + '[% maxvotesperbug FILTER html %]' is an invalid value for the + 'Maximum Votes Per [% terms.Bug %]' field, which should + contain a non-negative number. + + [% ELSIF error == "prod_votes_per_user_must_be_nonnegative" %] + [% title = "Votes Per User Must Be Non-negative" %] + '[% votesperuser FILTER html %]' is an invalid value for the + 'Votes Per User' field, which should contain a + non-negative number. + + [% ELSIF error == "prod_votes_to_confirm_must_be_nonnegative" %] + [% title = "Votes To Confirm Must Be Non-negative" %] + '[% votestoconfirm FILTER html %]' is an invalid value for the + 'Votes To Confirm' field, which should contain a + non-negative number. + + [% ELSIF error == "prod_cant_delete_description" %] + [% title = "Cannot delete product description" %] + Cannot delete the description for product + '[% product FILTER html %]'. + + [% ELSIF error == "prod_cant_delete_name" %] + [% title = "Cannot delete product name" %] + Cannot delete the product name for product '[% product FILTER html %]'. + + [% ELSIF error == "prod_name_already_in_use" %] + [% title = "Product name already in use" %] + The product name '[% product FILTER html %]' is already in use. + + [% ELSIF error == "prod_must_define_defaultmilestone" %] + [% title = "Must define new default milestone" %] + [% IF classification %] + [% classification_url_part = BLOCK %]&classification= + [%- classification FILTER url_quote %] + [% END %] + [% END %] + You must + create the milestone '[% defaultmilestone FILTER html %]' before + it can be made the default milestone for product '[% product FILTER html %]'. + [% ELSIF error == "product_disabled" %] [% title = BLOCK %]Product closed for [% terms.Bugs %] Entry[% END %] Sorry, entering [% terms.bugs %] into the @@ -967,6 +1006,11 @@ You must reassign those [% terms.bugs %] to another product before you can delete this one. + [% ELSIF error == "product_not_specified" %] + [% title = "No Product Specified" %] + No product specified when trying to edit components, milestones, versions + or product. + [% ELSIF error == "query_name_missing" %] [% title = "No Search Name Specified" %] You must enter a name for your search. -- cgit v1.2.1