Closed Bug 163114 Opened 21 years ago Closed 21 years ago

Templatise all calls to DisplayError

Categories

(Bugzilla :: Bugzilla-General, defect, P2)

Product:
Bugzilla
Component:
Bugzilla-General
Version:
2.15
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Bugzilla 2.18

People

(Reporter: gerv, Assigned: gerv)

Details

Attachments

(4 files, 2 obsolete files)

Patch A v.2
14.54 KB, patch
Details | Diff | Splinter Review
Patch B v.2
13.46 KB, patch
Details | Diff | Splinter Review
Patch C v.1
12.36 KB, patch
Details | Diff | Splinter Review
Patch D v.1
20.84 KB, patch
Details | Diff | Splinter Review 
DisplayError is deprecated; all calls to it need to be replaced with localisable
calls to ThrowUserError(). There are approximately 115 of them; this will
therefore be a tracking bug, as they get done in batches.

Gerv
Priority: -- → P2
Target Milestone: --- → Bugzilla 2.18
Attached patch Patch A v.1 (obsolete) — — Splinter Review 
This patch does the first 19 calls to DisplayError, all in attachment.cgi.

Gerv
Attached patch Patch A v.2 — — Splinter Review 
Updated patch.

Gerv
Attachment #99951 - Attachment is obsolete: true
Attachment #100028 - Attachment description: Patch v.2 → Patch A v.2
Attachment #99951 - Attachment description: Patch v.1 → Patch A v.1
Attachment #99951 - Flags: review+
Logs:

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.22; previous revision: 1.21
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v
 <--  user-error.html.tmpl
new revision: 1.9; previous revision: 1.8
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v
 <--  code-error.html.tmpl
new revision: 1.10; previous revision: 1.9
done

Patch B coming up.

Gerv
Attached patch Patch B v.1 (obsolete) — — Splinter Review 
This patch does CGI.pl and buglist.cgi.

Gerv
+  [% ELSIF error == "invalid_column_name_cookie" %]
+     column name <em>[% qfragment FILTER html %]</em>. 

+  ThrowCodeError("invalid_column_name_cookie");
How about
 ThrowCodeError("invalid_column_name_cookie", {'qfragment' => $fragment});

Otherwise it looks ok, i.e. r=burnus
Attached patch Patch B v.2 — — Splinter Review 
Version with review comments, to be checked in.

Gerv
Attachment #100375 - Attachment is obsolete: true
Patch B checked in.

Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v
 <--  code-error.html.tmpl
new revision: 1.13; previous revision: 1.12
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v
 <--  user-error.html.tmpl
new revision: 1.11; previous revision: 1.10
done
Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.197; previous revision: 1.196
done
Checking in CGI.pl;
/cvsroot/mozilla/webtools/bugzilla/CGI.pl,v  <--  CGI.pl
new revision: 1.179; previous revision: 1.178
done

Gerv
Comment on attachment 100375 [details] [diff] [review]
Patch B v.1

(close request to obsolete attachment 100375 [details] [diff] [review], the review fixed version, which
has been checked in, is attachment 100447 [details] [diff] [review])
Better late than never...

Gerv
Assignee: justdave → gerv
Attached patch Patch C v.1 — — Splinter Review 
Patch C. I estimate that this and one more of a similar size should have the
job finished.

Gerv
> +  [% ELSIF error == "invalid_component" %]
> +    [% title = "Invalid Component" %]
> +    The [% component FILTER html %] component doesn't exist in the 
> +    [% product FILTER html %] product.

> + $comp_id || ThrowCodeError("invalid_component");
You need to add the product and the component as argument.

>      # Add the quip 
>      my $comment = $::FORM{"quip"};
> -    if (!$comment) {
> -        DisplayError("Please enter a quip in the text field.");
> -        exit();
> -    }
You don't check for this anymore. Please add something like
$comment || ThrowUserError(...)

r=burnus if you fix those.
CHANGE PROCESS_BUG.CGI:636.

> You don't check for this anymore.

The copy of the patch I'm looking at has
+    $comment || ThrowUserError("need_quip");
on line 322. :-) Good call on the other one, though - I completely missed that.

Fixed.

Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v
 <--  user-error.html.tmpl
new revision: 1.13; previous revision: 1.12
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v
 <--  code-error.html.tmpl
new revision: 1.15; previous revision: 1.14
done
Checking in createaccount.cgi;
/cvsroot/mozilla/webtools/bugzilla/createaccount.cgi,v  <--  createaccount.cgi
new revision: 1.26; previous revision: 1.25
done
Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.75; previous revision: 1.74
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.68; previous revision: 1.67
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.152; previous revision: 1.151
done
Checking in quips.cgi;
/cvsroot/mozilla/webtools/bugzilla/quips.cgi,v  <--  quips.cgi
new revision: 1.14; previous revision: 1.13
done

Gerv
Attached patch Patch D v.1 — — Splinter Review 
This is it - the last patch, the final removal of DisplayError.

Gerv
> -    DisplayError("Unknown action: " . html_quote($action));
> +    ThrowCodeError("unknown_action");
You may want to change this to
  ThrowCodeError("unknown_action", { 'action' => $action});
to include more information.

> -            if ($prodcount{$prod} > $::prodmaxvotes{$prod}) {
> -                DisplayError("You may only use at most $::prodmaxvotes{$prod}
> +            ($prodcount{$prod} > $::prodmaxvotes{$prod})
> +              || ThrowUserError("too_many_votes_for_bug",
Are you sure that you don't revise the logic? Either use '&&' or '<='.
Additionally this should be "too_many_votes_for_product" not
"too_many_votes_for_bug".

> +     <tt>[% prod %]</tt> product, but you are trying to use [% votes %].
> +    Can't use [% field %] as a field name.
You should use a FILTER html here. If you fix those: r=burnus
All fixed! DisplayError() is no more.

Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v
 <--  user-error.html.tmpl
new revision: 1.14; previous revision: 1.13
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v
 <--  code-error.html.tmpl
new revision: 1.16; previous revision: 1.15
done
Checking in votes.cgi;
/cvsroot/mozilla/webtools/bugzilla/votes.cgi,v  <--  votes.cgi
new revision: 1.8; previous revision: 1.7
done
Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.45; previous revision: 1.44
done
Checking in reports.cgi;
/cvsroot/mozilla/webtools/bugzilla/reports.cgi,v  <--  reports.cgi
new revision: 1.60; previous revision: 1.59
done
Checking in sanitycheck.cgi;
/cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v  <--  sanitycheck.cgi
new revision: 1.61; previous revision: 1.60
done
Checking in showdependencygraph.cgi;
/cvsroot/mozilla/webtools/bugzilla/showdependencygraph.cgi,v  <-- 
showdependencygraph.cgi
new revision: 1.23; previous revision: 1.22
done
Checking in sidebar.cgi;
/cvsroot/mozilla/webtools/bugzilla/sidebar.cgi,v  <--  sidebar.cgi
new revision: 1.10; previous revision: 1.9
done
Checking in token.cgi;
/cvsroot/mozilla/webtools/bugzilla/token.cgi,v  <--  token.cgi
new revision: 1.13; previous revision: 1.12
done
Checking in describecomponents.cgi;
/cvsroot/mozilla/webtools/bugzilla/describecomponents.cgi,v  <-- 
describecomponents.cgi
new revision: 1.18; previous revision: 1.17
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.209; previous revision: 1.208
done
Checking in CGI.pl;
/cvsroot/mozilla/webtools/bugzilla/CGI.pl,v  <--  CGI.pl
new revision: 1.183; previous revision: 1.182
done
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.20; previous revision: 1.19
done

Gerv
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment on attachment 101380 [details] [diff] [review]
Patch D v.1

(Close request. This has been checked in.)
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.