Closed Bug 99203 (bz-alias) Opened 24 years ago Closed 23 years ago

alias for a bug

Categories

(Bugzilla :: Bugzilla-General, enhancement)

2.10
x86
Windows 2000
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: gagan, Assigned: myk)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 11 obsolete files)

17.80 KB, patch
myk
: review+
myk
: review+
Details | Diff | Splinter Review
Need a way to address a bug using a string moniker. More specifically we'd like to be able to add an alias to the |Depends on|, and the |Blocks| list.
Blocks: 99207
Some implementation and usability questions: 1. Can all bugs have an alias? 2 Is it first come, first serve on aliases? 3 What does the UI look like for adding and removing an alias? 4 Do aliases get removed or retired when bugs are resolved or verified? 5 When (in buglist, show_bug, dependency_tree, etc.) does it display the number, the alias and both. 6 Do we put a length limit on the alias? We probably need to think about this some more.
1. Can all bugs have an alias? Yes, why not? 2 Is it first come, first serve on aliases? Yes, I don't foresee great conflicts (this isn't domain name registration after all), and the ones that do arise (who gets "xxx"?) can be resolved by discussion in most cases, fiat in the rest. 3 What does the UI look like for adding and removing an alias? Since the field values are arbitrary strings, I imagine the UI being a text field on the show_bug form. 4 Do aliases get removed or retired when bugs are resolved or verified? I don't think so. People can retire them manually without too much trouble, and they should stay around on some bugs. 5 When (in buglist, show_bug, dependency_tree, etc.) does it display the number, the alias and both. That's the 64,000$ question. 6 Do we put a length limit on the alias? We have to put a limit on it, the question is how much. I recommend something small, like 20 characters. Aliases are supposed to be easy to remember, so I can't imagine the right limit being much more than that.
I'd like to propose that we keep the alias system simple. The association of an alias with a bug should be totally independent from the rest of the bug system. This way we start off with anyone being able to build an association of a bug with an alias and also be able to retire that association. So that we can reuse common aliases across time by applying them to different bugs-- so for example, what is 'nsbeta' today can be another bug tomorrow (that way the history is all preserved) As Myk said there may or may not be a reason to keep the alias around after the bug is fixed. Think of this alias as a Unix soft link :-) >5 When (in buglist, show_bug, dependency_tree, etc.) does it display the number, the alias and both. Just the numbers! Keep It Simple! Additionally there should be a reverse mapping page as well (which when fed a number would return an alias if one exists) More importantly this functionality can be added later on if I am wrong about it being needed right away... IMHO the biggest reason to keep it simple is to get it rolled out faster and have people start using such a system.
Target Milestone: --- → Future
Re-targeting to Bugzilla 2.18 milestone, since I plan to work on these bugs during that timeframe if not sooner.
Target Milestone: Future → Bugzilla 2.18
Attached patch patch v1: implements feature (obsolete) — Splinter Review
This patch implements this feature by modifying the ValidateBugID function (through which all user-submitted IDs should pass before being used by Bugzilla scripts) to check if the ID that was submitted is actually an alias. If it is, ValidateBugID modifies the *original* variable (not its local copy of it), converting it from the alias to the corresponding ID. This approach means we don't have to hack every script to recognize aliases independently, because every script that uses ValidateBugID will automatically get alias->id resolution. Scripts that don't use ValidateBugID, of course, will have to be updated, but this has to happen anyway and has already been done for a number of scripts. In addition to the changes to ValidateBugID, checksetup.pl has been modified to add the "alias" column to the "bugs" table, UI has been added to bug_form.pl (I put the field right under the bug ID row and moved the assignee field under the reporter field to make space for it), and the corresponding code has been added to process_bug.pl to update aliases in the database when users change them.
Comment on attachment 63807 [details] [diff] [review] patch v1: implements feature I haven't applied this yet, I'm just commenting on the code itsself. >Index: bug_form.pl >=================================================================== >RCS file: /cvsroot/mozilla/webtools/bugzilla/bug_form.pl,v >retrieving revision 1.85 >diff -u -r1.85 bug_form.pl You get to redo this after templatisation ;) >Index: process_bug.cgi >+foreach my $field ("dependson", "blocked") { >+ if (defined($::FORM{$field}) && $::FORM{$field} ne "") { >+ my @validvalues; >+ foreach my $id (split(/[\s,]+/, $::FORM{$field})) { >+ next unless $id; >+ ValidateBugID($id); I'm worried that this could be slow. Look at how dkl rewrote CanSeeBug on the groups branch. You could SELECT bug_id, alias from bugs where bug_id in (...), and nthen just remap those bugs which return a value. >+# If the bug has an alias, validate it for uniqueness. >+if (defined($::FORM{'alias'})) { >+ my $alias = trim($::FORM{'alias'}); >+ if ($alias ne "") { >+ my $sqlalias = SqlQuote($alias); >+ SendSQL("SELECT bug_id FROM bugs where alias = $sqlalias"); Why not use the helper func? Also, do we want to restrict creating aliases to people with editbugs? Those without can see the name, but can't create a new alias.
Blocks: 132181
Comment on attachment 63807 [details] [diff] [review] patch v1: implements feature This is interesting. I'd be glad to test this, but not before the patch gets pumped up to support the modern world (templates in this case).
Attachment #63807 - Flags: review-
Myk: do you still want this for the b.m.o upgrade? If so, could you please refresh and update the patch. If not, could you remove it from the dep list of bug 132181? Thanks :-) Gerv
Attached patch patch v2: updated to tip (obsolete) — Splinter Review
Attachment #63807 - Attachment is obsolete: true
Keywords: patch, review
Comment on attachment 87141 [details] [diff] [review] patch v3: here's a version with minimal impact on the "show bug" page >+ if ($id !~ /^\s*([1-9][0-9]*)\s*$/) { >+ $id = BugAliasToID($_[0]); >+ if ($id) { $_[0] = $id } Nit: can we line this up properly, please? :-) >>- <b>Bug#:</b> >+ <b>Bug&nbsp;#/Alias:</b> > </td> > <td> > <a href="[% Param('urlbase') %]show_bug.cgi?id=[% bug.bug_id %]"> > [% bug.bug_id %]</a> >+ <input name="alias" value="[% bug.alias FILTER html %]" length="10" maxlength="20"> > </td> I suggest making it look like: Bug#: 12345 (alias [<none> ]) where <none> is checked for and eliminated, much like "http://" in the URL field on bug entry. You could also say "a.k.a." instead of "alias". It depends which you think would be understood by more people. Other than that, r=gerv. Gerv
Attachment #87141 - Flags: review+
Attached patch patch v4: uses "a.k.a." (obsolete) — Splinter Review
This version uses "a.k.a." instead of alias, fixes a size problem (I was using the "length" attribute instead of the "size" attribute), and adds a label and tooltip for the form control. I didn't do "<none>", since it seems inconsistent with the rest of the "show bug" interface.
Attachment #87141 - Attachment is obsolete: true
testing
Alias: bugAlias
Does this allow people without editbugs to set aliases, if they're the reporter/qa/assignee? Should that be allowed? I thik we shouldn't, else we'll encourage lots of 'my-really-important-bug' aliases.
Alias: bugAlias → bz-alias
Comment on attachment 87164 [details] [diff] [review] patch v4: uses "a.k.a." The patch is also missing a call to AddFDef for the alias column - it should be mapping "alias" -> "Alias", ie with the capital. myk, you may have to update bmo manually, although the checksetup.pl code uses REPLACE, so maybe not. I don't know what the mailhead column in the fielddefs table is for, it seems to be working on bmo now with the default of 0, though.
Attachment #87164 - Flags: review-
IMHO, this field needs to be explained (especially at first, when it's new to people). Tooltip is good, but IMHO not good enough, mainly because tooltips do not currently wrap in Mozilla. What I would suggest is changin it back to "alias" and making the "alias" word a link to bug_status.html (or similar). P.S. The tooltip curently starts with "an name...", should probably be "A name..."
The text field should be wider, and I thunk 20 chars is not enough - it should be 30, or so.
Attached patch patch v5: misc fixes (obsolete) — Splinter Review
This patch fixes the "an name" typo and shortens the tooltip, fixes the taint error, and adds the FDef. >Nit: can we line this up properly, please? :-) The style is correct according to the Perl style guide upon which the Bugzilla style guide is based. >IMHO, this field needs to be explained (especially at first, when it's new to >people). I'll do that in my "state of b.m.o address" once the upgrade settles down. >The text field should be wider, and I thunk 20 chars is not enough - it should >be 30, or so. At that point it starts to become a mini-summary and is redundant with the summary field. This field is designed to hold a short name that is easier to remember and type than a number. Twenty character names are most likely too long already. Users should try to keep names at ten characters or less (like IRC nicks), hence the size of the form field.
Attachment #87164 - Attachment is obsolete: true
Hmm. I guess - we'll see how it goes. You need to enforce that limit in process_bug, though, and error out if someon tries an entry which is too big. I don't know if validatebugid needs the change - can I query for a varchar(20 column with a 21 character string?
> At that point it starts to become a mini-summary and is redundant > with the summary field. This field is designed to hold a short name > that is easier to remember and type than a number. So? Users are quite capable of doing this themselves without directives from up high. > Twenty character names are most likely too long already. Users should > try to keep names at ten characters or less (like IRC nicks), hence the > size of the form field. This is not true, I wanted to put bz-sanity-check-cleanup on a bug but I had to reduce it to bz-sancheckclean. It is smaller names that are harder to remember, because you have to use abbreviations. It shouldn't suprise anyone that bug numbers are the hardest to remember and they are also the shortest.
Depends on: 150831
No longer depends on: 150831
Depends on: 151018
Attached patch patch v6: adds validation (obsolete) — Splinter Review
Aliases shouldn't be able to contain commas or spaces or be all numbers.
Attachment #87187 - Attachment is obsolete: true
Comment on attachment 87309 [details] [diff] [review] patch v6: adds validation process_bug still doesn't check that the alias is the right length. You should also add teh alias to xml.cgi/Bug.pm and bugzilla.dtd
Attachment #87309 - Flags: review-
Blocks: 151413
Attached patch patch v7: review fixes (obsolete) — Splinter Review
>You need to enforce that limit in process_bug, though, and error out if someon >tries an entry which is too big. Done. > I don't know if validatebugid needs the change >- can I query for a varchar(20 column with a 21 character string? Yup. >So? Users are quite capable of doing this themselves without directives from up >high. By this logic we should have no restrictions on data entry in any field. Restrictions are logical ways to structure reports in a useful way. >It is smaller names that are harder to remember, >because you have to use abbreviations. Abbreviations, like names, are mnemonic, which is what's important about aliases, not that they are comprehensive. >It shouldn't suprise anyone that bug >numbers are the hardest to remember and they are also the shortest. Bug numbers are hard to remember because they are arbitrary, not because they are short. f.e. I haven't forgotten "bz-perf" since I added it to the Bugzilla performance meta-bug, even though it's much shorter (and less explanatory) than "bz-performance". >You should also add teh alias to xml.cgi/Bug.pm and bugzilla.dtd Done. I also increased the size of the field to 20 characters, since otherwise users will see only a portion of long aliases and may think the alias is the truncated version they see.
Attachment #87309 - Attachment is obsolete: true
Comment on attachment 87606 [details] [diff] [review] patch v7: review fixes >Index: Bug.pm >=================================================================== >RCS file: /cvsroot/mozilla/webtools/bugzilla/Bug.pm,v >retrieving revision 1.12 >diff -u -r1.12 Bug.pm >--- Bug.pm 27 Apr 2002 06:25:41 -0000 1.12 >+++ Bug.pm 14 Jun 2002 00:57:50 -0000 >@@ -33,7 +33,7 @@ > use CGI::Carp qw(fatalsToBrowser); > my %ok_field; > >-for my $key (qw (bug_id product version rep_platform op_sys bug_status >+for my $key (qw (bug_id alias product version rep_platform op_sys bug_status > resolution priority bug_severity component assigned_to > reporter bug_file_loc short_desc target_milestone > qa_contact status_whiteboard creation_ts groupset >@@ -108,7 +108,7 @@ > You added alias first here, but... >-<!ELEMENT bug (bug_id, (bug_status, product, priority, version, rep_platform, assigned_to, delta_ts, component, reporter, target_milestone?, bug_severity, creation_ts, qa_contact?, op_sys, resolution?, bug_file_loc?, short_desc?, keywords*, status_whiteboard?, dependson*, blocks*, cc*, long_desc*, attachment*)?)> >+<!ELEMENT bug (bug_id, (bug_status, product, priority, version, rep_platform, assigned_to, delta_ts, component, reporter, target_milestone?, bug_severity, creation_ts, qa_contact?, op_sys, resolution?, bug_file_loc?, short_desc?, keywords*, status_whiteboard?, dependson*, blocks*, cc*, long_desc*, attachment*, alias?)?)> added it to the end here (bugzilla.dtd) Thats invalid XML. The alias should probably be after the bug_id, for consistency Also, I'm beginning to wonder if the use of aliases shouldn't be controlled by a param - some installations may not want this.
Attachment #87606 - Flags: review-
Attached patch patch v8: review fixes (obsolete) — Splinter Review
>Thats invalid XML. The alias should probably be after the bug_id, for >consistency Done. >Also, I'm beginning to wonder if the use of aliases shouldn't be controlled by >a param - some installations may not want this. [echoes of mpt in my ears] Done. This patch also fixes the adding/removing dependencies problem.
Attachment #87606 - Attachment is obsolete: true
The latest patch also changes "a.k.a." to alias. The word "alias" is used in error messages and other places, since it's much easier to work into sentences than "a.k.a.", and it makes sense to be consistent even though the style of "a.k.a." is very nice.
Comment on attachment 87610 [details] [diff] [review] patch v8: review fixes aka is still there - in fact, this looks identical to the previous patch (the xml fu isn't fixed, there isn't a param, etc) Also, the reporter can still change the alias. Do we want to let that go for now, and deal with it in that other bug we're takling about this in?
Attachment #87610 - Flags: review-
Attached patch patch v9: review fixes (obsolete) — Splinter Review
Hmm, perhaps I submitted the wrong patch. This should be the right one.
Attachment #87610 - Attachment is obsolete: true
Let's deal with the reporter problem in the other bug.
Comment on attachment 87614 [details] [diff] [review] patch v9: review fixes You're going to hate this, but: >Index: checksetup.pl >=================================================================== >RCS file: /cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v >retrieving revision 1.155 >diff -u -r1.155 checksetup.pl >--- checksetup.pl 4 Jun 2002 05:47:28 -0000 1.155 >+++ checksetup.pl 14 Jun 2002 02:08:32 -0000 > index (delta_ts), >@@ -1348,7 +1349,9 @@ > index (resolution), > index (target_milestone), > index (qa_contact), >- index (votes)'; >+ index (votes), >+ >+ unique(alias)'; > You can't do this as-is. This creates a 17th index on the bugs table, and ISAM tables only allow 16. Before doing this, we'll have to 'fix' the bug for updating our mysql version to at least 3.23.6, and require myisam table formats. The first is ok (We probably want to push it up higher, later) and given that I don't see a problem with the second. I can't find the postgres limit in a quick glance through its manual. We may eventually remove the votes index (its only used for searching, and we have to handle the mutli-grouping fu differnetly for attachment statuses anyway). We should ask dkl what the postgres/oracle/etc limit is, though, first. >Index: xml.cgi >=================================================================== >RCS file: /cvsroot/mozilla/webtools/bugzilla/xml.cgi,v >retrieving revision 1.6 >diff -u -r1.6 xml.cgi >--- xml.cgi 27 Apr 2002 06:25:41 -0000 1.6 >+++ xml.cgi 14 Jun 2002 02:08:32 -0000 >@@ -49,7 +49,8 @@ > print Bug::XML_Header(Param("urlbase"), $::param{'version'}, > Param("maintainer"), $exporter); > foreach my $id (@ids) { >- my $bug = new Bug(trim($id), $::userid); >+ ValidateBugID($id); >+ my $bug = new Bug($id, $::userid); > print $bug->emitXML; > } You can't do this; xml.cgi must let Bug.pm handle validation so that the error messages are in xml.
Attachment #87614 - Flags: review-
dkl - please see the above, and comment on the max number of keys/table in postgres/oracle/sybase/etc. We also need to check the limits for the other mysql table types (innodb, in particular)
http://www.au.postgresql.org/users-lounge/limitations.html: "Maximum number of indexes on a table: unlimited" That answers that question...
Oh, and you need Param checks in process_bug, and bugaliastoid should return undef if not Param("...");
Attached patch patch v10: review fixes (obsolete) — Splinter Review
>Before doing this, we'll have to 'fix' the bug for >updating our mysql version to at least 3.23.6, and require myisam table >formats. Done and done. >You can't do this; xml.cgi must let Bug.pm handle validation so that the error >messages are in xml. Done. >Oh, and you need Param checks in process_bug, and bugaliastoid should return >undef if not Param("..."); Done.
Attachment #87614 - Attachment is obsolete: true
Attached patch patch v11: fixes bug 155460 (obsolete) — Splinter Review
Attachment #88561 - Attachment is obsolete: true
Comment on attachment 90004 [details] [diff] [review] patch v11: fixes bug 155460 This seems to have been working on bmo for a while, and the patch looks OK to me now. In checksetup.pl, move the 'alias' field addition to the bottom, and change the date to whenever you check it in. r=bbaetz
Attachment #90004 - Flags: review+
Comment on attachment 90004 [details] [diff] [review] patch v11: fixes bug 155460 > # no bug number given Add "or the alias didn't match a bug" (or something similar). >+sub BugAliasToID { >+ # Queries the database for the bug with a given alias, and returns >+ # the ID of the bug if it exists. ... "and undef if it doesn't." >+ The bug ID <em>$htmlid</em> is invalid (i.e. it is neither >+ a bug ID nor an alias to a bug ID). If you are trying to use Note that this part mentions bug aliases even if aliases are disabled. Consider whether or not this should be fixed. >+DefParam("use_bug_aliases", Params haven't traditionally had underscores in them. Unless there is a good reason, remove them? >+ # Since aliases are unique (like bug numbers), they can only be changed >+ # for one bug at a time, so ignore the alias change unless only a single >+ # bug is being changed. Unless the value is being set to empty, but maybe we don't need to care about this right now. With those addressed, r=jouni.
Attachment #90004 - Flags: review+
bbaetz: >In checksetup.pl, move the 'alias' field addition to the bottom, and change the >date to whenever you check it in. Done. jouni: >Add "or the alias didn't match a bug" (or something similar). Done. >... "and undef if it doesn't." Done. >Note that this part mentions bug aliases even if aliases are disabled. Consider >whether or not this should be fixed. Fixed. >Params haven't traditionally had underscores in them. Unless there is a good >reason, remove them? There are advantages to underscores, but we should probably switch en masse rather than only for new parameters. Removed underscores and created bug 155628 for this issue. >>+ # Since aliases are unique (like bug numbers), they can only be changed >>+ # for one bug at a time, so ignore the alias change unless only a single >>+ # bug is being changed. > >Unless the value is being set to empty, but maybe we don't need to care about >this right now. Right.
Attachment #90004 - Attachment is obsolete: true
Comment on attachment 90089 [details] [diff] [review] patch v12: review fixes Comments indicate this inherits reviews.
Attachment #90089 - Flags: review+
Checking in Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.13; previous revision: 1.12 done Checking in CGI.pl; /cvsroot/mozilla/webtools/bugzilla/CGI.pl,v <-- CGI.pl new revision: 1.160; previous revision: 1.159 done Checking in bug_form.pl; /cvsroot/mozilla/webtools/bugzilla/bug_form.pl,v <-- bug_form.pl new revision: 1.95; previous revision: 1.94 done Checking in bugzilla.dtd; /cvsroot/mozilla/webtools/bugzilla/bugzilla.dtd,v <-- bugzilla.dtd new revision: 1.6; previous revision: 1.5 done Checking in checksetup.pl; /cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl new revision: 1.158; previous revision: 1.157 done Checking in defparams.pl; /cvsroot/mozilla/webtools/bugzilla/defparams.pl,v <-- defparams.pl new revision: 1.76; previous revision: 1.75 done Checking in process_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v <-- process_bug.cgi new revision: 1.130; previous revision: 1.129 done Checking in template/en/default/bug/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.11; previous revision: 1.10 done
Resolving fixed.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Has the textbox been made wider than the b.m.o. version? That's my current gripe with the alias system :-) (Also the fact that it and QuickSearch don't yet play nicely together.) I still also think that "a.k.a." is the right styling for the show_bug page, even though it's not the same as the error usage. "a.k.a." and "alias" are interchangeable words, but a.k.a. has less other meanings. Gerv
>I still also think that "a.k.a." is the right styling for the show_bug page, >even though it's not the same as the error usage. "a.k.a." and "alias" are >interchangeable words, but a.k.a. has less other meanings. I'd go for alias nonetheless, since a.k.a. is pretty hard for non-native English speakers. The word alias is used as such in many languages and even when not, most people understand it because they've heard it before anyway.
this causes multiple taint errirs in show_bug.cgi and process_bug.cgi perl is : This is perl, version 5.005_03 built for i386-freebsd fisrt error is in CGI.pl line 305 SendSQL("SELECT bug_id FROM bugs WHERE bug_id = $id"); call to DB with tainted data $id used to be detaint_natural before the call changing that to : SendSQL("SELECT bug_id FROM bugs WHERE bug_id = " . SqlQuote($id)); fixed that error but next fails in global.pl line 800 Attempted to send tainted string 'SELECT bugs.bug_id FROM bugs LEFT JOIN cc selectVisible_cc ON bugs.bug_id = selectVisible_cc.bug_id AND selectVisible_cc.who = 1 WHERE ((bugs.groupset & 9223372036854775807) = bugs.groupset OR (bugs.reporter_accessible = 1 AND bugs.reporter = 1) OR (bugs.cclist_accessible = 1 AND selectVisible_cc.who = 1 AND not isnull(selectVisible_cc.who)) OR (bugs.assigned_to = 1)) AND bugs.bug_id = 2597' to the database at globals.pl line 260.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
daa@distributed.net: Is the problem you reported covered by bug 155700 or is it a separate issue? It would seem to me that these are the same issue.
I think it is... I'd also rather have this dealt with on a separate bug... trying to keep track of what patches are actually ready to check in is difficult when we have a reviewed and checked in patch on a reopened bug. The aliases feature is implimented. That fixes this bug. Other errors caused by it are new bugs.
Status: REOPENED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: