312307 - Misused Throw*Error tags in code and templates

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[u197037]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050217
Build Identifier: 

Some of our CGI's and .pm's are throwing errors newer defined in
code/user-error.html.tmpl files. Also, there are a set of errors in templates,
which aren't ever used in code.
First case results in cryptic error messages for users and second one is just a
garbage.

Reproducible: Always



Expected Results:  
Correct error messages (well localized) and not littered templates

Comment 1

[u197037]

Attachment: Fix for current misused Throw*Error tags

This should fix current tags either not covered by code/user-error.html.tmpl or
defined in templates but never used from code.

Comment 2

[Frédéric Buclin]

Comment on attachment 199411 [details] [diff] [review]
Fix for current misused Throw*Error tags

>Index: template/en/default/global/code-error.html.tmpl

>+  [% ELSIF error == "group_id_invalid" %]
>+    Invalid group ID specified.

user-error.html.tmpl has a 'invalid_group_ID' already. You should use it.



>Index: template/en/default/global/user-error.html.tmpl

>+  [% ELSIF error == "no_components" %]
>+    [% title = "No components for product" %]
>+    Product '[% product FILTER html %]' has not components.

in enter_bug.cgi:

if (0 == @{$::components{$product}}) {	      
    ThrowUserError("no_components", {product => $product});   
} 

This is deprecated code as enter_bug.cgi already calls
Bugzilla->user->can_enter_product($product, 1);
a few lines before. Remove this deprecated code from enter_bug.cgi.


>+  [% ELSIF error == "extern_id_conflict" %]
>+    [% title = "Extern ID Conflict" %]
>+    Someone with a different external ID has that address!.

No "!" please.


>+  [% ELSIF error == "token_invalid" %]
>+    [% title = "Invalid Token" %]
>+    That token cannot be used.


in token.cgi:

  my $validationerror = ValidatePassword($::token);
  if ($validationerror) {
      ThrowUserError("token_invalid");
  }

ValidatePassword() never returns anything, and throws an error itself when the
token is invalid. So we never enter the IF block. Remove this deprecated code
here too.

Comment 3

[u197037]

Attachment: Updated patch

Updated patch according to Fred's comments, plus one more issue in relogin.cgi

Comment 4

[Frédéric Buclin]

Comment on attachment 199945 [details] [diff] [review]
Updated patch

>Index: template/en/default/global/code-error.html.tmpl

>+  [% ELSIF error == "must_be_patch" %]
>+    [% title = "Attachment Must Be Patch" %]
>+    Attachment #[% attach_id FILTER html %] must be a patch.

attachment.cgi calls this error with no parameter:
ThrowCodeError("must_be_patch");

This should be: ThrowCodeError("must_be_patch", $vars);

This can be fixed on checkin. r=LpSolit for tip only

Comment 5

[Frédéric Buclin]

justdave, do you want it for 2.20 too?

Comment 6

[Dave Miller [:justdave]]

Yeah, we should do this for 2.20 as well.  error tags without text to go with
them is bad.  (don't we have a test for this? ;)

Comment 7

[u197037]

(In reply to comment #6)
> Yeah, we should do this for 2.20 as well.  error tags without text to go with
> them is bad.  (don't we have a test for this? ;)
Well, then I'll need to grab 2.20 from CVS (got only tip).
see bug 312042 - test is awaiting review from Jouni.

Comment 8

[u197037]

Attachment: The same patch for v2.20

As was asked, the same patch for 2.20

Comment 9

[Frédéric Buclin]

Comment on attachment 200405 [details] [diff] [review]
The same patch for v2.20

r=LpSolit

Comment 10

[Frédéric Buclin]

tip:

Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.123; previous revision: 1.122
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.129; previous revision: 1.128
done
Checking in relogin.cgi;
/cvsroot/mozilla/webtools/bugzilla/relogin.cgi,v  <--  relogin.cgi
new revision: 1.29; previous revision: 1.28
done
Checking in report.cgi;
/cvsroot/mozilla/webtools/bugzilla/report.cgi,v  <--  report.cgi
new revision: 1.33; previous revision: 1.32
done
Checking in token.cgi;
/cvsroot/mozilla/webtools/bugzilla/token.cgi,v  <--  token.cgi
new revision: 1.38; previous revision: 1.37
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.59; previous revision: 1.58
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.138; previous revision: 1.137
done


2.20:

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.89.2.2; previous revision: 1.89.2.1
done
Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.114.4.3; previous revision: 1.114.4.2
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.118.2.3; previous revision: 1.118.2.2
done
Checking in report.cgi;
/cvsroot/mozilla/webtools/bugzilla/report.cgi,v  <--  report.cgi
new revision: 1.29.4.1; previous revision: 1.29
done
Checking in token.cgi;
/cvsroot/mozilla/webtools/bugzilla/token.cgi,v  <--  token.cgi
new revision: 1.31.2.2; previous revision: 1.31.2.1
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.52.2.2; previous revision: 1.52.2.1
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.115.2.8; previous revision: 1.115.2.7
done