We need to support custom resolutions, as well as the ability to make resolutions inactive. I'm currently working on this. My patch has enter_bug, query, show_bug, checksetup, etc all working ok, other parts not so good. When I get a few more cycles I can probably get the patch to a stage where I will post it. Basically in my implementation FIXED, MOVED and DUPLICATE are hardcoded (but renameable) because they're all special, everything else is up for grabs.
Is there a way I can remove VERIFIED? I think CLOSED is good enough.
There is good discussion about customisable statuses over at bug #37613, this is purely for resolutions.
-> New product Bugzilla
Created attachment 48260 [details] [diff] [review] Here's an initial version for people to look at. It isn't ready yet, but I'm letting people see the method..
Gerv, I need your input about converting over charts to use customised resolutions. The first problem is that new resolutions can be added. This means that I need to maintain a mapping from resolution to column, and add any new resolution detected to the list. There seems to be one of these in the file already which I could use. However the file is not currently being read by collectstats.pl. In any case, this mapping needs to be from resolution id to column, because resolutions can be renamed. As is, I'd have to convert the resolution list to id rather than name. The second problem is deletions. I either need to prevent deleting resolutions that are logged (difficult, impractical if collectstats.pl continues to collect all resolutions), or make deletions be automatically accounted for, by making that column 0 forever more. However then, I'd have to make sure ids never get recycled, which they can be now. Any input you have on these issues would be appreciated. I'm fast approaching the end of development on this feature.
The mapping from resolution to column is that the column is headed with the resolution. (Note that the current columns contain statuses as well as resolutions.) The solution to this is blatently to move the graphing data into the DB. I should have done this long ago but have not. It would probably be acceptable for you to finish this feature and say that only resolutions with the following names: FIXED WONTFIX etc. will be chartable until I fix the charting... A problem might arise if the custom resolutions were not a superset... Hmm. You might need a backwards compatibility hack until the info moves into the DB. I think it's fair to say that if a resolution is deleted, its charting data can be deleted. However, I think you should also implement deprecation for resolutions; in several other areas of Bugzilla (e.g. groups) it has been found that deprecation is very useful - allow the thing to continue to exist, but don't allow the problem to be compounded, e.g. don't allow more NS Confidential bugs to be filed. If you need more thoughts, let me know :-) Gerv
> The mapping from resolution to column is that the column is headed with the > resolution. Yes, I figured that. It's just that I would have to parse this information in collectstats.pl, whereas at the moment I believe it is a simple append to that file. > The solution to this is blatently to move the graphing data into the DB. > I should have done this long ago but have not. It would probably be > acceptable for you to finish this feature and say that only resolutions > with the following names: FIXED WONTFIX etc. will be chartable until I > fix the charting... A problem might arise if the custom resolutions were > not a superset... Hmm. You might need a backwards compatibility hack until > the info moves into the DB. I could do a temporary hack, however I have changed the default set of resolutions for new installations as a part of this work, so it would not work out of the box. If I do do a hack here I'll want a bug # on your side to hang my full fix bug off. > I think it's fair to say that if a resolution is deleted, its charting data > can be deleted. I don't. The purpose of charting is to get a historical perspective and that implies keeping deleted resolutions. Of course, if we had a horizon on chart data beyond which it gets deleted (do we?), we could delete it after that. One problem is that with deletion, the name will need to be stored for the resolution rather than the ID. > However, I think you should also implement deprecation for resolutions Inactive resolutions have been present in the implementation since the very beginning, as is stated on the first line of this bug.
Matthew: I think the easiest way to make this happen is if you check in customisable resolutions (and break graphing) and then I implement the back end for general bug charts, which would do away with any link between resolutions and graphing. I'd implement a generic back end but, because no-one has yet worked out how to do a decent UI for configuring and selecting generic charting, I'd hide it behind the current UI and not allow you to edit the possible charts. That would make it much easier and less controversial to implement. Feel free to file a bug on me for this work, or we can tie it into one of the current "generic charting" bugs. BTW, is Bugzilla going to continue to ship by default with the same resolutions that it has at the moment? I assume so. Gerv
My current patch will keep the existing resolutions for existing installations, but will slightly change them for new ones. Basically WONTFIX is split off into NOTWORTHIT (for closed projects) and FEATURENOTBUG (for the way we currently use it now), and the latter is what I will recommend mozilla.org change to to avoid the confusion there currently is over WONTFIX. There is also the addition of CAREFACTORZERO (much requested feature) and MISSING (bug #58552). REMIND and LATER have been removed.
Do resolutions have to be all caps and not separated? How about CARE_FACTOR_ZERO, for a start? :-) And I'd go for NOT_A_BUG rather than FEATURENOTBUG. Anyhow, the sensible order for this is probably you, then me. I'll get straight to work as soon as you check in. This will mean charting is broken for a bit; but that's nothing new :-) Gerv
Also, if I may be permitted to stick my oar in _even_ further, if we are adding new screens to Bugzilla, shouldn't we be adding them templatised? Or have you done that? ;-) Gerv
I'm pretty sure underscores are allowed for resolutions, basically its the same policy as keywords uses since that's where the code came from originally. The resolutions were chosen to be those names with the fact in mind that the bug list shows only the first 4 characters. Hence "CARE" and "FEAT" would be OK, but I'd hesitate introducing "NOT_". Yes, editresolutions.cgi is templatised. Yes, editresolutions.cgi runs in taint mode. Any more questions? =)
Well, we could change the way the columns work ( ;-) ). My misgiving is over the fact that "it's a bug, not a feature" is a term commonly used among hackers but not well understood elsewhere. It's also less general than "NOT_A_BUG", or some other similar name. We can still do CARE_FACTOR_ZERO, but why do we need that resolution at all? mozilla.org practice is that much-requested feature bugs are left open, so people can find them in queries, and assigned to firstname.lastname@example.org . The current assignee may have CARE_FACTOR_ZERO, but it doesn't mean that anyone else has. Introducing this resolution will encourage engineers to close feature bugs and make it harder to find owners for them. Gerv
Just curious (I have yet to look at the patch) but why are we interested in naming resolutions here? I thought the point of customization was so that the end users could decide on names themselves. If users want to use "NOT_A_BUG" and have it show up as "NOT_" why should we prohibit that? It was my impression that this bug would allow adding and removing and renaming resolutions, not offering a stock set that they can choose from....
There still have to be defaults. The canonical rule of Bugzilla development is "don't break bugzilla.mozilla.org". So (AIUI) the defaults will closely match b.m.o, apart from the changes Matthew mentions. Gerv
My earlier point came off wrong: I meant to ask why we are interested in changing the defaults. I know we need defaults still. What is wrong with the current set of defaults? If we include documentation and note that you can now change resolutions, I think that people will figure out how to change them and decide on names on their own. If we really feel like we want to offer some suggestions, why not just include documentation with examples and maybe even a walkthrough containing the examples? 'To change resolutions, click here and do yadayada, and if you type in "NOT_A_BUG" for the resolution it will display as "NOT_" in the bug lists, etc....' I know of a few people who install copies of Bugzilla every now and then on intranets as a way of tracking issues during contract work (I am one of them). I have gotten used to (and like) the current FIXED, INVALID, DUPLICATE, WONTFIX, etc. resolutions. Why should we make users go into Bugzilla on any new installs they perform and "change them back" to the way things are now? We aren't going to please everyone with the defaults, so let's keep them the way they are and the ones that already are not pleased can change them (I am sure they will find out how to change resolutions if they really want to). We don't need to alienate those who are content with the current defaults too, do we?
As for CARE_FACTOR_ZERO, it's kind of a joke, it'll probably get removed. I certainly wouldn't suggest using it for an open source software product. Our defaults should encourage proper practice and demonstrate possibilities, not necessarily be a useful set out of the box. There is already a precedent for this - new installations get a TestProduct and TestComponent. If people have gotten used to the defaults, they can get used to the new ones. I don't appreciate people citing inertia when there are good reasons to change them. None of this will affect mozilla.org as it only applies to new installations.
This is now on a branch called CUST_RES_BRANCH. This is because Gerv needs to do bug #105491 at the same time to prevent graphing from breaking if someone renames an existing or adds a new resolution. There will probably be a couple of other (small) breakages when this lands but nothing major. The latest code is running on landfill. There are still a couple of issues I'm aware of that I should be fixing shortly. Summary: Resolutions are customised with the editresolutions.cgi script, which uses the admineditor.pl, a reusable file that is intended to be used in future by many of the edit*.cgi files. These files are templatised and run in taint mode including DB taint. Resolutions can be added, removed, renamed and deactivated. Bugs can only be resolved with resolutions that are active. You can only query for resolutions that are active or on at least one bug. Resolutions must be unique on the first four characters because only the first four characters of a bug's resolution is shown on bug lists.
This is now on the "we really want this for 2.16, but won't hold the release for it if it's not done by then" list.
In response to your comment on bug 106589: Regardless of whether or not other parts of Bugzilla do it that way, using the number zero instead of the null value to indicate the lack of a resolution will cause us problems later when we implement referential integrity and all values in the "bugs.resolutionid" column are required to match a corresponding value in the "resolutions.id" column. Tricking referential integrity rules by adding the empty resolution to the "resolutions" table is an even worse solution because all code that accesses that table has to distinguish the empty resolution from the real ones. Consistency with similar implementations in other parts of the code is no excuse for perpetuating a design flaw, especially when implementing a new feature. The null value exists for the express purpose of indicating the lack of a value for a certain field and is the logically consistent value for the resolution of an unresolved bug.
Yes, I'd forgotten about referential integrity. My main point with that statement though, is to indicate this is not the only place it occurs.
s/occurs/needs to be fixed/
Not making 2.16, too blocked.
What still needs to be done on the branch: Bring it up to date with HEAD (again). Provide an option as to whether you want to use sortkeys for resolutions (probably in editparams.cgi). Add a short name field that will appear on buglist, rather than just pulling the first 4 characters. What this is blocked on: Bug #105491 (customisable charts) needs to be redone to go at the same time so changing resolutions doesn't wreak havoc with charting. Bug #70907 (converting QuickSearch to Perl) needs to be done beforehand so changing resolutions properly updates QuickSearch searching.
I know my bug blocks this one; but I'm afraid what with 2.16, rewriting the documentation, Mozilla 1.0, request tracker and other stuff, I don't know when I'll get to fixing the charts system. I will do my best, but it might be a while. Gerv
I will probably attempt to get in the administration architecture (currently on this branch) into the tree in another CGI (probably editkeywords), which will make this bug an easier review, especially if generic charting goes in at the same time.
*** Bug 150894 has been marked as a duplicate of this bug. ***
*** Bug 163635 has been marked as a duplicate of this bug. ***
*** Bug 208096 has been marked as a duplicate of this bug. ***
Okay can any up to date patches be posted? even if not working...
So is anyone looking at this? We have an old patch. Anyone like/dislike the path this was taking? Comments/ ideas? I see a lot of people on the CC list for this bug. I know I was working on my own version of this back in 2.14
Okay, So, I see that this was working on landfill. Is it still? How much needs to be done on this? Just a side ?, When I was working on this long... Time .... Ago... we needed to rename the resolutions/states for each product. I had worked out an aliasing system. Does the new system have support for this? Can it be added? Thanks
Taking Jake's bugs... his Army Reserve unit has been deployed.
The landfill version is really old. I'm currently working on the administration rearchitecture that spawned off this, over at bug #69267. It's almost ready for review, and once that's in, I can revive the custres part that will require it.
Is there anything new with this bug? As it blocks a lot of others (e.g. DB independece), I would really like to see some progress here.
Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004. Anything flagged enhancement that hasn't already landed is being pushed out. If this bug is otherwise ready to land, we'll handle it on a case-by-case basis, please set the blocking2.20 flag to '?' if you think it qualifies.
Some Addions to the comments
Let's see what I can do with this one... :)
I won't have time to play with it before the 2.22 freeze
This feature is wanted for the next release, per our new roadmap.
Created attachment 221385 [details] [diff] [review] untested patch, v0.5 This is mostly still work in progress. It seems more or less complete, it passes runtests.pl and collectstats.pl runs correctly. But it seems that it might be necessary to regenerate statistics when adding a new resolution or renaming one (I have to investigate this point a bit further). Also, editvalues.cgi now correctly manages fields which have no default value (which is the case of the resolution). Requesting review to put this patch in our radar.
Created attachment 228036 [details] [diff] [review] patch, v1 Per our discussion on IRC with timeless and justdave, we don't want to update saved searches and series to reflect changes made in resolutions. The main argument is that if I rename a resolution, that's maybe because I want to give it a new and separate meaning than its previous meaning, and so stats and/or queries shouldn't assume (by default) that they are still the same "object". Another argument is that editvalues.cgi doesn't update saved searches and series when updating a platform name, an OS, a priority or a severity. Same thing when renaming products, components, etc... I had to fix collectstats.pl to not break when finding a non-standard resolution. Basically, it looks at the field names already known, and if there is nothing new, simply add new stats at the end of the data file, else it "reformats" the data file to take new fields into account (which is *much* faster than calling --regenerate).
I have got 2 questions: What do I have to do if I want to 1.) delete 2.) add new resolutions (or stati - as in https://bugzilla.mozilla.org/show_bug.cgi?id=101179)? Enabling those links in the GUI via adding 'bug_status' and 'resolution' to 'our @valid_fields' in editvalues.cgi isn't enough. So which steps are need to be done for both intentions? Wherefore is the parameter 'isactive' in the database e.g. in table 'resolution' (and also in table 'bug_status')? Is it enough to set this field to '0' if I want do delete (or deactivate) a resolution?
> Per our discussion on IRC with timeless and justdave, we don't want to update > saved searches and series to reflect changes made in resolutions. I think it would cause a great deal of confusion to have saved searches and series update on some changes but not on others. For example, they might update with changes of product name but not with changes of resolution. That would be bad. IMO, if you want a resolution with a new and separate meaning to another resolution, you should not be renaming one, you should be deleting one and creating another. Gerv
(In reply to comment #46) > IMO, if you want a resolution with a new and separate meaning to another > resolution, you should not be renaming one, you should be deleting one and > creating another. That's why my patch doesn't update saved queries.
(In reply to comment #47) > (In reply to comment #46) > > IMO, if you want a resolution with a new and separate meaning to another > > resolution, you should not be renaming one, you should be deleting one and > > creating another. > > That's why my patch doesn't update saved queries. There are two requirements here. One is to simple rename something that was misspelled or to clarify its naming. In this case you want your references be updated automatically. The other case is when you want to replace it with a new state. In this case you create a new state, "deprecate" the old state and keep it around for display/history purposes or delete it later. In this case you won't existing queries to be updated automatically. But this won't happen because you are actually not renaming it.
Comment on attachment 228036 [details] [diff] [review] patch, v1 The code looks basically good. Seems like we added a lot of complexity to editvalues.cgi, though--is there any way to put a lot of that code into subroutines and re-use it, or anything? >Index: collectstats.pl > [snip] >+ AND resolution.id IS NULL >+ UNION >+ SELECT bugs_activity.removed Nit: Add an extra newline before and after UNION--it makes it easier to read. >+# Actually, the list of statuses is predefined. This will change in the near future. >+local our @statuses = qw(NEW ASSIGNED REOPENED UNCONFIRMED RESOLVED VERIFIED CLOSED); Nit: This script never gets run as a CGI, so you don't need to "local our" if you don't want. >@@ -143,56 +176,101 @@ > [snip] There's a *lot* of code in this section. Could we move some of it into subroutines just to make things more readable? I didn't look over the templates at all, yet.
(In reply to comment #49) > The code looks basically good. Seems like we added a lot of complexity to > editvalues.cgi, though--is there any way to put a lot of that code into > subroutines and re-use it, or anything? I don't see how. We never have twice the same piece of code.
Created attachment 228947 [details] [diff] [review] patch, v1.1 I put some code into a separate get_old_data() routine to make the code a bit easier to read in collectstats.pl. No other changes.
Comment on attachment 228947 [details] [diff] [review] patch, v1.1 I'm pretty sure the empty resolution needs to be static. Also, when I rename a resolution, I would expect it to be renamed in the stats, but I guess we don't want to modify the bugs_activity table, or something, right? So that's fine. Also: Let's pretend that I: 1) Add a resolution: IGNORED 2) Run collectstats.pl 3) I wait a week, I set several bugs to IGNORED 4)I add another resolution: DESTROYED 5) I run collectstats.pl Now, IGNORED will say "0" in every column in the past, right? Does that matter? Everything else seems to be good.
Created attachment 229093 [details] [diff] [review] patch, v1.2 Add "" to the list of static values in editvalues.cgi
Comment on attachment 229093 [details] [diff] [review] patch, v1.2 Hrm...it's also impossible to edit the empty value, but that's always been a problem for editvalues.cgi, so we should file a bug for that.
Checkin fix: Non-deletable (with hyphen)
Checking in collectstats.pl; /cvsroot/mozilla/webtools/bugzilla/collectstats.pl,v <-- collectstats.pl new revision: 1.54; previous revision: 1.53 done Checking in editvalues.cgi; /cvsroot/mozilla/webtools/bugzilla/editvalues.cgi,v <-- editvalues.cgi new revision: 1.14; previous revision: 1.13 done Checking in reports.cgi; /cvsroot/mozilla/webtools/bugzilla/reports.cgi,v <-- reports.cgi new revision: 1.87; previous revision: 1.86 done Checking in t/008filter.t; /cvsroot/mozilla/webtools/bugzilla/t/008filter.t,v <-- 008filter.t new revision: 1.24; previous revision: 1.23 done Checking in template/en/default/admin/fieldvalues/confirm-delete.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/fieldvalues/confirm-delete.html.tmpl,v <-- confirm-delete.html.tmpl new revision: 1.6; previous revision: 1.5 done Checking in template/en/default/admin/fieldvalues/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/fieldvalues/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.5; previous revision: 1.4 done Checking in template/en/default/admin/fieldvalues/list.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/fieldvalues/list.html.tmpl,v <-- list.html.tmpl new revision: 1.4; previous revision: 1.3 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.77; previous revision: 1.76 done Checking in template/en/default/bug/knob.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/knob.html.tmpl,v <-- knob.html.tmpl new revision: 1.22; previous revision: 1.21 done Checking in template/en/default/bug/show-multiple.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show-multiple.html.tmpl,v <-- show-multiple.html.tmpl new revision: 1.29; previous revision: 1.28 done Checking in template/en/default/bug/activity/table.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/activity/table.html.tmpl,v <-- table.html.tmpl new revision: 1.13; previous revision: 1.12 done Checking in template/en/default/email/whine.txt.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/email/whine.txt.tmpl,v <-- whine.txt.tmpl new revision: 1.2; previous revision: 1.1 done Checking in template/en/default/global/field-descs.none.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/field-descs.none.tmpl,v <-- field-descs.none.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.175; previous revision: 1.174 done Checking in template/en/default/list/edit-multiple.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/list/edit-multiple.html.tmpl,v <-- edit-multiple.html.tmpl new revision: 1.35; previous revision: 1.34 done Checking in template/en/default/list/table.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/list/table.html.tmpl,v <-- table.html.tmpl new revision: 1.29; previous revision: 1.28 done Checking in template/en/default/pages/fields.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/fields.html.tmpl,v <-- fields.html.tmpl new revision: 1.8; previous revision: 1.7 done Checking in template/en/default/reports/report-bar.png.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/report-bar.png.tmpl,v <-- report-bar.png.tmpl new revision: 1.6; previous revision: 1.5 done Checking in template/en/default/reports/report-line.png.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/report-line.png.tmpl,v <-- report-line.png.tmpl new revision: 1.7; previous revision: 1.6 done Checking in template/en/default/reports/report-pie.png.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/report-pie.png.tmpl,v <-- report-pie.png.tmpl new revision: 1.5; previous revision: 1.4 done Checking in template/en/default/reports/report-table.csv.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/report-table.csv.tmpl,v <-- report-table.csv.tmpl new revision: 1.9; previous revision: 1.8 done Checking in template/en/default/reports/report-table.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/report-table.html.tmpl,v <-- report-table.html.tmpl new revision: 1.12; previous revision: 1.11 done Checking in template/en/default/search/form.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/search/form.html.tmpl,v <-- form.html.tmpl new revision: 1.41; previous revision: 1.40 done Checking in template/en/default/whine/mail.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/whine/mail.html.tmpl,v <-- mail.html.tmpl new revision: 1.4; previous revision: 1.3 done Checking in template/en/default/whine/mail.txt.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/whine/mail.txt.tmpl,v <-- mail.txt.tmpl new revision: 1.4; previous revision: 1.3 done
Added to relnotes in bug 349423. Please let me know if I missed any critical information about this bug in the release notes.
Docs updated as part of bug 281876.
Selenium script added to bug 101179. Committing to: bzr+ssh://email@example.com/bugzilla/qa/4.0/ modified t/test_custom_fields.t Committed revision 176. Committing to: bzr+ssh://firstname.lastname@example.org/bugzilla/qa/3.6/ modified t/test_custom_fields.t Committed revision 147. Committing to: bzr+ssh://email@example.com/bugzilla/qa/3.4/ modified t/test_custom_fields.t Committed revision 119.