Misused Throw*Error tags in code and templates

RESOLVED FIXED in Bugzilla 2.20

Status

()

RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: dennis.melentyev, Assigned: dennis.melentyev)

Tracking

2.20
Bugzilla 2.20
Dependency tree / graph
Bug Flags:
approval +
approval2.20 +

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

13 years ago
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
(Assignee)

Comment 1

13 years ago
Created attachment 199411 [details] [diff] [review]
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.
Attachment #199411 - Flags: review?(jouni)

Updated

13 years ago
Assignee: zach → dennis.melentyev
Status: UNCONFIRMED → NEW
Component: Testing Suite → Bugzilla-General
Ever confirmed: true
Target Milestone: --- → Bugzilla 2.20
(Assignee)

Updated

13 years ago
Blocks: 312042

Updated

13 years ago
Attachment #199411 - Flags: review?(LpSolit)

Comment 2

13 years ago
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.
Attachment #199411 - Flags: review?(jouni)
Attachment #199411 - Flags: review?(LpSolit)
Attachment #199411 - Flags: review-
(Assignee)

Comment 3

13 years ago
Created attachment 199945 [details] [diff] [review]
Updated patch

Updated patch according to Fred's comments, plus one more issue in relogin.cgi
Attachment #199411 - Attachment is obsolete: true
Attachment #199945 - Flags: review?(LpSolit)

Comment 4

13 years ago
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
Attachment #199945 - Flags: review?(LpSolit) → review+

Comment 5

13 years ago
justdave, do you want it for 2.20 too?
Status: NEW → ASSIGNED
Flags: approval?
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? ;)
Whiteboard: needs patch for 2.20
(Assignee)

Comment 7

13 years ago
(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.
(Assignee)

Comment 8

13 years ago
Created attachment 200405 [details] [diff] [review]
The same patch for v2.20

As was asked, the same patch for 2.20
Attachment #200405 - Flags: review?(LpSolit)
Whiteboard: needs patch for 2.20 → patch waiting review

Comment 9

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

r=LpSolit
Attachment #200405 - Flags: review?(LpSolit) → review+

Updated

13 years ago
Flags: approval2.20?
Whiteboard: patch waiting review
Flags: approval?
Flags: approval2.20?
Flags: approval2.20+
Flags: approval+

Comment 10

13 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Version: unspecified → 2.20

Updated

13 years ago
Blocks: 290574
You need to log in before you can comment on or make changes to this bug.