Closed Bug 81642 Opened 24 years ago Closed 20 years ago

"Split bug / Clone bug": Enter new bug with prefilled fields

Categories

(Bugzilla :: Creating/Changing Bugs, enhancement, P3)

Product:
Bugzilla
Component:
Creating/Changing Bugs
Version:
2.13
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Bugzilla 2.20

People

(Reporter: afranke, Assigned: shane.h.w.travis)

References

Details

(Whiteboard: enter_bug)

Attachments

(1 file, 11 obsolete files)

Code patch for tip, take 7
11.21 KB, patch
jouni
: review+
Details | Diff | Splinter Review
For splitting bugs that contain multiple issues, a "Clone this bug" link in the "show_bug.cgi" generated bug view would be helpful. It should have all fields initialized to the original one, except the comments. If you want to get fancy, you can have an intermediate page where you can choose to _not_ copy certain fields. Maybe the description field should be initialized with something like "*** This bug has been split off bug 12345 ***".
I would like to see the parent bug have the children added as dependant bugs. ie: It would be required to fix all children before fixing the parent. Additionally, this mechanism could be used to enter bugs that span multiple versions. A form that allowed multiple version selections could be added, then if > 1 version(s) were checked, multiple dependent bugs would be created for each version, with a meta bug as a parent for all the dependent children.
Priority: -- → P4
Target Milestone: --- → Future
Mattty, can you give a short explanation of the meaning of "Future"? If it means "post Bugzilla 3.2" which is post-3.0 which in turn may be 2004, then I disagree with a lot of your target milestone assignments, including this one.
The main task here was to clear off my '---' list to make sure there wasn't anything really important on it and get the 2.16 list ready. 'Future' in Bugzilla currently basically means "no plans to implement in the other milestones". It doesn't mean it will have to wait for 3.0, as it might happen in 2.20, but it does mean it won't be happening in 2.16/2.18, nor in 3.0/3.2. That's a little confusing but it's a little hard to target otherwise because we don't know how many 2.X milestones we're going to have. If we clear as many bugs away as I hope we will in 2.16 we'll need to sit down afterwards and figure out where we're going for 2.18 and beyond and where 3.0 fits into the picture. Currently there aren't many bugs targetted for 2.18, 3.0 or 3.2 as most are at 2.16 or Future. And as always patches are welcome and should hopefully in future be given the express lane once we clear the patch backlog.
Ok, looking for the express lane then. Please comment whether this approach is the right or the wrong way to do it. The patch is mostly working, except for groupsets, dependencies, and keywords. (dependencies and keywords are not yet supported by enter_bug.cgi). It would be nice if someone who knows groupsets better could have a look at enter_bug.cgi and this new code and give me a hint how to debug the bits stuff.
Assignee: tara → afranke
Keywords: patch, review
Priority: P4 → P3
Target Milestone: Future → Bugzilla 2.16
-> Bugzilla product.
Component: Bugzilla → Creating/Changing Bugs
Product: Webtools → Bugzilla
Version: Bugzilla 2.13 → 2.13
Whiteboard: enter_bug
-> default owner. (Pulling myself out.)
Assignee: afranke → myk
Doesn't "remember values as a bookmarkable template" already accomplish this?
Only if you plan to file two similar bug reports before filing them. This is for after the first one already exists, and you decide later it needs to be split.
We definitely want the ability to clone bugs - Asa and Myk and I had a long discussion about it. However, we want to associate the clones in the DB, and I think it's a bit bigger of a project than filing a new bug with the same data. At any rate, this isn't 2.16 material :-) Gerv
Keywords: patch, review
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
This sounds like bloat to me. It's already possible to file a new bug and then cross-reference the two bugs. What makes this feature worth the extra complexity of the code and of bugzilla's user interface?
Gerv, can you clarify what you meant by "associate the clones in the DB"? I mean, the new bug won't be an exact clone, will it? In most cases, you will tweak the summary a bit, and maybe some other things. Maybe you mean "originated_from" or "part_of" (which could be expressed as a new dependency like bug 141175 calls for)? Can you explain your intended semantics?
The way I envisage this working is as follows: There is an operation you (or a sufficiently empowered user) can perform on a bug, known as "cloning" it. This files a new bug report, with all the fields set to exactly the same values as the old report, and makes a link between the bugs using some DB mechanism (I have ideas, but it's not relevant.) The UI would then be that bug reports which were part of a set of clones would have tabs at the top, giving the numbers, and perhaps summaries, of the clones. Once you have cloned a bug, you are responsible for changing the summary or fields to indicate why it's different, e.g. changing the Version field to "Mozilla 1.0 Branch" for a bug to track a fix which has been checked in on the trunk. So, what do we do about things which are associated with the intial bug: - attachments - comments - votes - dependencies? This would require discussion. Off the top of my head: - attachments. We could either try to clone all non-obsolete ones, or not clone any, on the basis that the patch for the clone is probably different. - comments. Don't clone these. There will be loads of dross, and the cloner can summarise the issues the new bug is about when they fiddle the fields. - votes. Again, not much point cloning these really. We are past the stage where votes matter - dependencies. Hmm. If the bug has been fixed, "depends on" is irrelevant. "blocks" might be relevant. Another thought: you may be asked to redefine the initial description of the clone, based on the description of the original. Gerv
Ok, I understand your reasoning, but that's different from what I had in mind when I filed this bug. I was just finding myself quite often in a situation where I needed to file a new bug, with almost the same settings as an existing one I was viewing at the same time. I wouldn't want to have them associated in the database, or displayed as tabs, or something. I just wanted to have the option to file a "similar" bug where I only needed to adjust a few fields instead of re-entering *all* the fields' data. In some cases, setting the (yet to be implemented) "related bugs" field (see bug 12286) may make sense, but that would fit under a generalization of bug 141175. Gerv: can you make a decision whether this bug is about mine or your proposal, and whether we need a third bug for the other one (besides bug 141175 which calls for a shortcut to create a dependent bug)?
> I just wanted to have the > option to file a "similar" bug where I only needed to adjust a few fields > instead of re-entering *all* the fields' data. Your idea and mine are almost exactly the same; all that would be required would be to have a checkbox, checked by default, marked "These bugs should be associated with each other" on the intermediate confirmation screen for cloning a bug (where the user defines those bits of the bug which aren't copied from the clone.) If we then define a way to create the associations independently of a "clone" operation, we have the "related bugs" feature. Maybe. But I'm not convinced by "related bugs" - the examples given in bug 12286 fit into dependency relationships, as far as I can see. Gerv
> checkbox [...] on the intermediate confirmation screen for cloning a bug nice, except that I've been happily using my "cloning" feature for almost half a year now, without an extra intermediate screen, and I think it's really not necessary. But if you agree that this checkbox can be displayed somehow on the same page with the ordinary enter_bug form, then this could be a solution. Of course, it would be nice if I could customize my template(s) so that this checkbox is hidden (and off) when my users hit my customized "clone" link (in case I haven't any use for the "associated in the database" feature). But maybe the database-association will be useful enough in the end that everybody wants to have this option, I don't know.
> nice, except that I've been happily using my "cloning" feature for almost half a > year now, without an extra intermediate screen, and I think it's really not > necessary. That must mean that your cloned bugs start off identical to the original, if there is no opportunity to modify the exact parameters of the cloning. I think that doing that would make the feature substantially less useful and general. Gerv
No, it just takes me directly to the enter_bug page where I can modify everything I want, just like a bookmarked enter_bug template, except that the prefill values are taken from the bug where I click the link on. See the patches attached to this bug. What I meant is: if you can agree to put that checkbox on the enter_bug page directly, then this is probably solved.
> No, it just takes me directly to the enter_bug page where I can modify > everything I want, Except you can't, because not all bug fields appear on the enter_bug page. I'm not quite convinced that going via the enter_bug page is the right approach... I'd have to think about that. Gerv
> Except you can't, because not all bug fields appear on the enter_bug page. which is a bug in the enter_bug page, IMO. Anyway, it worksforme, see attachment 45537 [details] [diff] [review] (clone_bug.cgi). The only shortcomings I am aware of: #XXX TODO: groupsets (enter_bug already has support for that), # dependencies (no support in enter_bug yet), # keywords (no support in enter_bug yet). # settable_resolution
Blocks: bz-zippy
This probably depends on combining process_bug and post_bug (and, ideally, making them use bug.pm). If it doesn't depend on it, it would probably make it _much_ easier.
*** Bug 141175 has been marked as a duplicate of this bug. ***
I would be happy with this being a dup of bug 141175 if clone_bug.cgi could also take a dependecy (dependson and blocked) argument to pre-fill with those fields on enter_bug with those values (and if not sent, clone the bug). This would allow me to easily make a link for "Enter blocking/dependant bug" in the template like 141175 requested. And as far as the TODOs: > #XXX TODO: groupsets (enter_bug already has support for that), > # dependencies (no support in enter_bug yet), Fixed in bug 112373. > # keywords (no support in enter_bug yet). Fixed in bug 25521.
*** Bug 175250 has been marked as a duplicate of this bug. ***
So comments are not shared between cloned bugs, Gerv?
> So comments are not shared between cloned bugs, Gerv? In my vision, no. When the screen comes up for filing the clone, the "Original Description" would be initialised with the original description from the first bug, but then it could be changed or added to if necessary, to say why this particular bug was split off (if it wasn't obvious from other fields.) Fitting with the idea of developing the fix in one place, and then splitting for the actual application to various places, most of the comments would be irrelevant anyway. This does raise the question of whether we want to clone attachments; perhaps we could clone non-obsolete ones. I don't think there's a problem having two bugs referencing the same attachment. Gerv
The Product to which the bug should be cloned should be selectable, instead of fixed. Let's say you start a new product from older code, but you still need to maintain the older code, you might want to clone the bug to both products.
clone_bug.cgi is perfect, but it would be more perfect :) if you could: 1. select the product to clone the bug to 2. clone the comments 3. clone the attachments I'd be willing to make the changes myself, if someone could point me in the right direction.
Line 125 of attachment 45537 [details] [diff] [review] is missing an = sign. . "&op_sys" . url_quote($bug{'op_sys'}) should be: . "&op_sys=" . url_quote($bug{'op_sys'})
The attachment 45539 [details] [diff] [review] is obsolete. It should replaced by patch for template bug/navigate.html.tmpl To fix this there should be added: "&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;<a href="clone_bug.cgi?id=[% bug.bug_id %]">Clone this bug</a>" at the end of this template.
*** Bug 226208 has been marked as a duplicate of this bug. ***
I vote for this bug! My company tends to use Collector bugs to track groups of similar bugs. We would greatly benefit from this functionality in that we could easily create new bugs to add to the Collector by editing the Collector, clicking on the new 'Create similar bug' link, and then changing the fields as needed, prior to submission.
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
sort of a different idea in Bug 252807
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.
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
Attached patch Patch against 2.18rc2, first try (obsolete) — — Splinter Review
Alright... I'm new at this, so be gentle. :) I hesitated to upload my patch because I didn't want to step on anyone's toes, but I can see that this has been on the books for a long time, and am told that Andreas is mostly inactive these days. I don't know if this will facilitate getting the feature into an upcoming version, but I figure it certainly can't hurt. I've come at this from a different direction from Andreas, I think. My version adds another field on enter_bug.cgi that allows you to name another bug whose contents you wish to clone. Information is sucked from that bug, and the page is redrawn with said info pre-filled in to the fields. These fields can then be edited if the user desires. Only the first comment from the original bug is copied, and an additional line of text is included at the top of the comment to note that the information was cloned from another bug. It is possible to clone a bug across products; a security check is done to ensure that the author has permissions to see the information on the bug he is trying to clone. If he does, it goes ahead; he will have to re-set the product and component before he can commit. Local decision was that attachments were not to be copied. I can see some arguments for why they should, but for the most part I agree that they should stay with the original. Since there's no way to edit inclusion/exclusion of attachments in enter_bug, I'm more comfortable leaving it out. patch includes an allow-bug-cloning parameter so sites can turn this feature off if they won't use it. I originally did the fix in 2.16.6, but had to hack some stuff to get the dependencies to work. The attached patch is was done against 2.18rc2 and is much more elegant, IMHO. I have (literally) never made a patch file before, so I am hoping that it works. :)
Attachment #162935 - Flags: review?
It would be best to avoid going to the database with specialized SQL to do this. Can you do this usign either the fields alredy provided to the template by the previous show_bug (to prepopulate the new bug template - if a bookmarklet can do it (bug 241443), template code should be able to) or by prefilling the fields using the bug object?
The reason it works for bookmarkable templates is because (up to 2.18 anyway, don't know about beyond) a template URL looks something like this: http://fwk-svr.sedsystems.ca/bugzilla-test/enter_bug.cgi?product=TestProduct&bug_status=NEW&version=other&component=TestComponent&rep_platform=Macintosh&op_sys=Windows%20Server%202003&priority=P5&bug_severity=blocker&assigned_to=travis&cc=test%20test2%20test3%20&bug_file_loc=http%3A%2F%2Fwww.sedsystems.ca&short_desc=Bug%20Bug%20Numbah%20One%21&comment=%2B%2B%2B%20This%20log%20item%20was%20initially%20created%20as%20a%20clone%20of%20Log%20Item%20%231%20%2B%2B%2B%0D%0AFirst%20bug%21%20Booyah%21&keywords=important%20gui%20&dependson=1&blocked=&maketemplate=Remember%20values%20as%20bookmarkable%20template&form_name=enter_bug Right there, that's all the information you need to re-create an enter_bug form. For my patch, that information does not exist. There *is* no 'previous show_bug' or existing bug object. As I said in comment #36, this is NOT an update of Andreas' patch; it is an entirely new way of coming at it. Andreas' implementation (as I understand it) was to have a 'Clone this bug' button on show_bug; mine is to have a 'clone bug # ___' on enter_bug. This is not an intentional slight on Andreas' work; I had already coded up my way of cloning bugs at the request of our local users, and it was only later, after several people on the developer's list and newsgroup showed similar interest in having these capabilities, that I learned of the existence of this bug, at which point I updated my modifications to work with 2.18rc2 and made them publicly available here. Andreas' method may be great; it may, in fact, be better than mine. I don't know, because I've never implemented his method. I'm just adding my voice to the chorus of people who'd like to see this functionality exist globally, and putting my money where my mouth is by making my patch available to all. If the decision is that mine is the wrong way of going about it, I won't be crushed irrevocably; I just won't be able to work to get the feature into Bugzilla any more, and will have to content myself with our local solution. Having said that, I found one minor bug, some commented code I left in, and a couple of tabs-instead-of-spaces, so I've made a second attempt.
Attachment #162935 - Attachment is obsolete: true
Attachment #163092 - Flags: review?(justdave46203)
Attachment #162935 - Flags: review?
Attachment #163092 - Flags: review?(justdave46203) → review?(justdave)
Hmm. I'd say the correct implementation is definitely to have the "clone bug" link on the original bug. Otherwise, you are looking at the original bug, and you have to go somewhere else, taking the bug number with you, to clone it... Gerv
Y'know... I had a big explanation written in response to Comment #39 as to why doing it this way was better -- mostly because Andreas' method didn't allow for bugs to be cloned from one project to another, whereas my patch did. (This was critical for local implementation.) Besides, even the original patch was going back to the database for all the information DESPITE being positioned already on the show_bug page. The more I went looking for specific reasons why I couldn't do it, though, the more I realized that it might be workable after all... especially when keeping in mind Joel's comments about bookmarks templates. So, long story short (I know, too late) I re-wrote the whole darn thing to make a new template clone.html.tmpl that hangs off show_bug, only appears if the parameter is true, allows for cloning across products, and doesn't ever have to go to the database. As the script-kiddies say, this one = teh win. :)
Attachment #163092 - Attachment is obsolete: true
Attachment #163469 - Flags: review?(justdave)
Attachment #163092 - Flags: review?(justdave)
Comment on attachment 163469 [details] [diff] [review] 3rd patch against 2.18rc2 - complete re-write Learning the process, realizing Dave's a busy man... maybe kiko can take a look at this instead. :)
Attachment #163469 - Flags: review?(justdave) → review?(kiko)
Attachment #163469 - Flags: review?(jouni)
Assignee: myk → travis
Status: NEW → ASSIGNED
Comment on attachment 163469 [details] [diff] [review] 3rd patch against 2.18rc2 - complete re-write First off: I like the approach here. However, the architecture fails in one critical spot: passing the first comment. Because of this, some of the following code comments may be less useful for the future versions, but I'll post them nevertheless to help (or frustrate, whatever ;-)) you in your Bugzilla career. In the future, please patch against the tip instead of 2.18 branch. Not a problem this time though; the patch applied just fine. Also, please run the testing suite against the patched version - at least this one fails the tests. >+ name => 'allow-bug-cloning', We don't use hyphens in parameter names (just "allowbugcloning" will do). >+ desc => 'If this option is on, users may make a \'clone\' of any existing bug they can access. Cloning pre-fills various fields of the copy with information pulled from the original; these fields can be edited before the new bug is committed.', Wrap this line to the 80 char limit. Also, "the cloning link appears on the show_bug screen" might be a worthy addition. >--- bugzilla-2.18rc2/enter_bug.cgi 2004-06-22 01:49:15.000000000 -0600 >+++ bugzilla-2.18rc2.clone/enter_bug.cgi 2004-10-26 15:33:46.000000000 -0600 >+ if ( Param('allow-bug-cloning') ) { >+ $vars->{'url'} = $::buffer; >+ } "url" isn't a particularly enlightening name for a variable. How about "enter_params_url" or whatever? (at least you might add a comment describing what's going on here) >--- bugzilla-2.18rc2/template/en/default/bug/create/clone.html.tmpl 1969-12-31 18:00:00.000000000 -0600 >+++ bugzilla-2.18rc2.clone/template/en/default/bug/create/clone.html.tmpl 2004-10-26 15:25:36.000000000 -0600 Since this template is just the clone link, it's perhaps better off in the bug directory where the show templates reside? Also, maybe named as "clone-link" or whatever? >+[%# Sort of a messy way to go about this, but we need to 'FILTER url_quote' >+ # some parts of the string, and not others. Besides, this scans easily, >+ # which is seldom a bad thing. >+ #%] I agree about the messy part :-) You don't need to explain it in the source file, though... That explanation is necessary for the reviewer, not for the code reader. Anyway, I have a replacement suggestion (which has its problems too, but it's IMO more readable): [% clone_url = BLOCK %]bug_status=NEW&version= [% bug.version FILTER url_quote %]&component= [% bug.component FILTER url_quote %]&rep_platform= [% bug.rep_platform FILTER url_quote %] [% END %] (several fields omitted, but you get the point) >+[% clone_comment = '+++ This log item was initially created as a clone of ' _ terms.Bug _ ' #' _ bug.bug_id _ '. +++' FILTER url_quote %] >+[% clone_comment = clone_comment _ '%0D%0A' %] >+[% next_param = bug.longdescs.0.body FILTER url_quote %] >+[% clone_url = clone_url _ '&comment=' _ clone_comment _ next_param %] This part is the main problem here. First, it causes a "414 Request URI too long" server error if your comment 0 is long enough. Second, it exposes the content of the first comment in even if it is marked as private (using the insider group). You could go around this by reformatting the link as a group of hidden fields and a submit button, but that's likely to be too dominant an UI element. Of course, you could do the extra db access to handle this, but it seems like quite some hassle. Hmm... I wonder if the first comment is really worth all this effort? Usually it isn't the best summarization of a problem, so the reporter is probably going to end up writing a renewed summary anyway. The clone comment prefix would better work as something like "This description was cloned from bug Xxx". >+[% clone_url = clone_url _ '&blocked=' %] Why this empty param? (the same for the empty assignee param passed)
Attachment #163469 - Flags: review?(jouni) → review-
Attachment #163469 - Flags: review?(kiko)
(In reply to comment #42) > >+ name => 'allow-bug-cloning', > We don't use hyphens in parameter names (just "allowbugcloning" will do). I beg to differ: cf: all the moved-* parameters. (Many newer parameters also have underlines in them.) I recall seeing a bug where someone requested that all parameters be broken up on word boundaries to make it easier for non-english speakers, and I think it's a very good idea. I don't mind switching from hyphens to underscores if that's the preferred method, but I do think that multi-word paramaters should be broken *somehow* on word boundaries. > >--- bugzilla-2.18rc2/template/en/default/bug/create/clone.html.tmpl > Since this template is just the clone link, it's perhaps better off in the bug > directory where the show templates reside? Well, you do end up creating a new bug, hence my placement in 'create' and not 'show'. If you're adamant about moving it, I could do so, but this seemed more natural to me. Rename makes sense, and will be in next version. > >+[%# Sort of a messy way to go about this, but we need to 'FILTER url_quote' > >+ # some parts of the string, and not others. Besides, this scans easily, > >+ # which is seldom a bad thing. > >+ #%] > I agree about the messy part :-) You don't need to explain it in the source > file, though... That explanation is necessary for the reviewer, not for the > code reader. Well, my personal opinion is that Bugzilla suffers from being significantly *under* commented, so there's seldom any explanation as to what's happening or why. One quote in our quips file (put there by me, admittedly) is "Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live. ~Martin Golding" For me, this means not only well- written code, but programmer comments where they make sense. Since this looked so kludgy, it made sense here. <shrug> > This part is the main problem here. First, it causes a "414 Request URI too > long" server error if your comment 0 is long enough. Hrm... kay, I'll have to look into that. > Second, it exposes the content of the first comment in even if it is marked > as private (using the insider group). Could this not be fixed by duplicating the first comment that the user can see? (And setting the privacy to be the same in the new bug as it was in the old bug...) After all, if someone can only see the third comment because the first two are insider comments, and they go to clone the bug, expected behaviour would be to bring over the first comment visible to them, no? > Hmm... I wonder if the first comment is really worth all this effort? I think it is, and several people in the thread of this bug have commented that they'd like the first comment preserved. > The clone comment prefix would better work as something like "This description > was cloned from bug Xxx". Why 'this description' and not 'this bug'? After all, you're cloning the entire bug (and linking them), not just the description... > >+[% clone_url = clone_url _ '&blocked=' %] > Why this empty param? (the same for the empty assignee param passed) Probably unnecessary. I'll check. Thanks very much for the feedback, Jouni; I look forward to your response, and will have an updated patch some time in the near future.
(In reply to comment #43) > I beg to differ: cf: all the moved-* parameters. (Many newer parameters also > have underlines in them.) Very well. > I recall seeing a bug where someone requested that all parameters be > broken up on word boundaries to make it easier for non-english speakers, > and I think it's a very good idea. I agree 100%; it's just that I believe in doing it uniformly one way and then switching in one go. But if we've already begun sliding, I'm not going to oppose it here. > Well, you do end up creating a new bug, hence my placement in 'create' and > not 'show'. If you're adamant about moving it, I could do so, but this seemed > more natural to me. Yes, but we're only creating a _link_ to bug creation here. As a really bad analogy, we don't have the footer's New link templatized in the create directory. > Well, my personal opinion is that Bugzilla suffers from being significantly > *under* commented, so there's seldom any explanation as to what's happening or > why. True, though I feel this isn't the commentary Bugzilla needs most direly. It's all about signal/noise ratio. But I'm not going to hold review over this comment, of course :-) If you like my replacement suggestion, this may of course become irrelevant anyway. > Could this not be fixed by duplicating the first comment that the user > can see? Perhaps, but it could practically produce a very strange basis for a bug report - the first comment the user can see could be anything, including something totally worthless. I'd almost say we could leave the first comment empty if we can't get comment 0. > > The clone comment prefix would better work as something like "This > > description was cloned from bug Xxx". > Why 'this description' and not 'this bug'? After all, you're cloning the > entire bug (and linking them), not just the description... All right, you've got me turned.
Attached patch Code patch for tip, take 2 (obsolete) — — Splinter Review
* Patched against tip, not 2.18 * Changed parameter name to have underscores instead of hyphens * Wrapped parameter info properly * $vars->{url} is now $vars->{clone_link_url} * Renamed clone.html.tmpl to clone_link.html.tmpl (but left it in the same directory; if you're adamant, I guess I can still move it) * Used Jouni's replacement code suggestion * Removed blank parameters * Removed any attempt to get the first comment into the clone Of these, only the last point really bothers me. I put code in place so that the first comment was not copied if the user couldn't see it, and that was simple enough... but when I tried to deal with the too-long-URI issue, it really smacked me down hard. Empirical testing showed that I was usually safe with a URL up to at least 4000 characters on several different browsers, but Jouni you said that the standard indicates that browsers only HAVE to accept URLS up to 512 characters, which doesn't leave much space for the initial comment at all. :( Even with 4000 characters, I was having problems deciding where to stop (because [% FILTER url_quote %] was adding a lot of characters to the URL), and what to do if I had to truncate, but those were mere technical issues that I could eventually have gotten around. My honest opinion is that without the ability to copy the first comment, this becomes a much less useful feature... but maybe that's just me and my needs. (Although even Gerv seemed to think that the first comment was useful too -- see comment #27). Can anyone help me brainstorm a way to make this happen that gets around the URI limit? Anyway... on the assumption that it cannot be done and/or it's still a useful feature without cloning the first comment, I'll still finish it up and get it on the trunk. One last quesiton: Andreas had his clone link in the navigation bar, I put mine in the knob. Other than style/preference, any good reasons to go with one over the other?
Attachment #163469 - Attachment is obsolete: true
Attachment #168881 - Flags: review?(jouni)
Comment on attachment 168881 [details] [diff] [review] Code patch for tip, take 2 (In reply to comment #45) > really smacked me down hard. Empirical testing showed that I was usually safe > with a URL up to at least 4000 characters on several different browsers, but > Jouni you said that the standard indicates that browsers only HAVE to accept > URLS up to 512 characters, which doesn't leave much space for the initial > comment at all. :( I seem to have remembered wrong. The HTTP spec doesn't limit nor require anything on the URI length. 512 chars must've been the limitation from some package I was using a long time ago. My apologies for the false information. Some quick googling shows that the most dominant limitation here is 2048 (or 2083, depends on how you count) by IE 5.5 and earlier; see <http://support.microsoft.com/kb/q208427/>. Apparently my testbed Apache rejects URIs of 4k+ - but as you noted, whatever the limitations are, we will have a problem as the uri simply cannot contain all this information, and that's it. Let's focus on the alternatives. > My honest opinion is that without the ability to copy the first comment, this > becomes a much less useful feature... but maybe that's just me and my needs. > (Although even Gerv seemed to think that the first comment was useful too -- > see comment #27). What's this "even Gerv" stuff? ;-) I'm not saying the first comment wouldn't be useful at times - it might be a reasonable default. Even though you're probably right at least for some usage scenarios, the lack of first comment copying doesn't IMO make the feature useless - cloning a bug is still easier with this stuff than without it. > Can anyone help me brainstorm a way to make this happen that > gets around the URI limit? As far as I can see, there are two reasonable options. Either you make Clone link a button and use HTTP post to convey the information OR you pass a bug ID and retrieve the comment from the DB at enter_bug. I strongly prefer the first one, since I don't think it's necessarily worthy to bloat the show_bug HTML by including the basic bug information and comment 0 twice. If you go down the DB access line, then passing any of the other fields in the URI seems illogical as well. > One last quesiton: Andreas had his clone link in the navigation bar, I put mine > in the knob. Other than style/preference, any good reasons to go with one over > the other? Your choice is IMO better; it fits the grouping of links to list-level/bug-level better than the placement in nav bar. I also noted that security group and time tracking information doesn't get cloned. It's an interesting question whether or not they should (then again, why not?), but as the amount of fields to clone grows, I'm more and more starting to think about the DB-read approach. --- >--- tip/template/en/default/bug/create/clone_link.html.tmpl 1969-12-31 18:00:00.000000000 -0600 >+++ tip.clone/template/en/default/bug/create/clone_link.html.tmpl 2004-12-16 11:24:26.000000000 -0600 We use hyphens for template names ("clone-link"). >+[%# Sort of a messy way to go about this, but we need to 'FILTER url_quote' >+ # some parts of the string, and not others. Besides, this scans easily, >+ # which is seldom a bad thing. >+ #%] Hey, I don't think it's messy anymore :-) IMO you can remove this comment totally. >+[% clone_comment = '+++ This log item was initially created as a clone of ' >+ _ terms.Bug _ ' #' _ bug.bug_id _ '. +++' FILTER url_quote %] How about "This [% terms.bug %] was initially created"? "log item" doesn't sound familiar to me. > &nbsp; | &nbsp; > <a href="long_list.cgi?buglist=[% bug.bug_id %]">Format For Printing</a> >+ [% IF Param('allow_bug_cloning') %] >+ [% PROCESS bug/create/clone_link.html.tmpl %] >+ [% END %] > </b> Nit: Indent the IF block by two spaces.
Attachment #168881 - Flags: review?(jouni) → review-
(In reply to comment #46) > What's this "even Gerv" stuff? ;-) Honestly, just a reference to the earlier discussion. By my reading, Gerv didn't think that it was worth copying *all* the comments, but he did think it was worth taking the first one. I would tend to agree with his logic, and was basically using his support as justification. > the lack of first comment copying doesn't IMO make the feature > useless - cloning a bug is still easier with this stuff than without it. So, by my reading of your comments, this patch is basically ready to go (ready to go, barring a couple of nits)... and we're going to get rid of it because we can't use it to get the first comment. That right? (I'm not complaining: I'm just trying to see if I've correctly divined what you are saying.) > Either you make Clone > link a button and use HTTP post to convey the information OR you pass a bug ID > and retrieve the comment from the DB at enter_bug. I strongly prefer the first > one, since I don't think it's necessarily worthy to bloat the show_bug HTML by > including the basic bug information and comment 0 twice. Some confusion here: did you mean to say, "I strongly prefer the second one?" I don't see how passing the bug_id would cause bloat, but perhaps I'm missing something. > If you go down the DB access line, > then passing any of the other fields in the URI seems illogical as well. I guess so. The whole reason we were trying to pass in all the info in the URI was to make use of the bookmark-like features and avoid any database access whatsoever. One access is infinitely more than no accesses... and two is only twice as much as one. ;-) > I also noted that security group and time tracking information doesn't get > cloned. It's an interesting question whether or not they should (then again, > why not?), but as the amount of fields to clone grows, I'm more and more > starting to think about the DB-read approach. The security group is just a non-product group, isn't it? Nothing inherently special about it... although it does beg the question as to whether or not a cloned bug should start off with all the same bug groups as its parent. What if the clone is in a different product altogether? That's allowed... and some of the bug groups may no longer apply if that's the case. My thought is just to leave them to whatever the defaults are, or whatever can normally be set for a new bug of this type on the enter_bug screen. Time tracking should definitely *not* be copied, for two reasons: * First, it would mean that you've got hours for one bug recorded in two places. Time spent on the original and time spent on the clone are tracked in each individual bug. * Secondly, time tracking is stored in longdescs, and it's not possible to copy a longdesc without taking the comment - can't have a blank longdesc:thetext entry or many things break. Since we're not taking the comments, we *can't* take the time entries. > >+++ tip.clone/template/en/default/bug/create/clone_link.html.tmpl 2004-12- 16 11:24:26.000000000 -0600 > We use hyphens for template names ("clone-link"). I tried that; I thought that it choked because it was finding a '-' character where it wasn't expecting one, which is why I renamed it. I'll check it again both ways and report back. > Hey, I don't think it's messy anymore :-) IMO you can remove this comment > totally. Oops. :) Oversight. It should have been removed. > How about "This [% terms.bug %] was initially created"? "log item" doesn't > sound familiar to me. Absolutely. 'log item' is local usage. (I thought I fixed that...) So... where do we go from here, boss? Fix up these nits and call it a day? Or another complete re-write to eliminate clone-link altogether and instead pass a bug_id into enter_bug? IF that's the case, we're back to updating attachment #163092 [details] [diff] [review] (and the objections to it in comment #37 and comment #39). That patch assumed that you'd be entering the bug_id on the enter_bug page, but most of the code would be the same (I think) if the bug_id was instead getting passed up the chain. Please take a look at that patch in light of these later discussions and tell me if you think I'm on the right track.
> So, by my reading of your comments, this patch is basically ready to go > (ready to go, barring a couple of nits)... and we're going to get rid of > it because we can't use it to get the first comment. That right? I'm not sure what you refer to with "it" here - get rid of what? The patch? ;-) > > Either you make Clone link a button and use HTTP post to convey the > > information OR you pass a bug ID and retrieve the comment from the DB at > > enter_bug. I strongly prefer the first one, since I don't think it's > > necessarily worthy to bloat the show_bug HTML by including the > > basic bug information and comment 0 twice. > Some confusion here: did you mean to say, "I strongly prefer the second > one?" Yes, of course. Sorry :-( Using an additional HTML form would be bloat. > > I also noted that security group and time tracking information doesn't get > > cloned. It's an interesting question whether or not they should (then again, > > why not?), but as the amount of fields to clone grows, I'm more and more > > starting to think about the DB-read approach. > The security group is just a non-product group, isn't it? Nothing inherently > special about it... although it does beg the question as to whether or not a > cloned bug should start off with all the same bug groups as its parent. If (and when) we're approaching the cloning as pre-filling enter_bug, I believe the correct approach would be to pass all group information. If (some of) the groups are not allowed in the new product, they are of course then discarded - but otherwise, their checkboxes should be checked by default in the enter_bug form. Naturally, all mandatory groups for the new product need to be automatically added for the clone, but post_bug takes care of this for us. So, to simplify: Of the checkboxes normally available in enter_bug, we should check those which are also checked on the bug we're cloning from. The other group stuff we can ignore. From a practical standpoint, many organizations have a "security vulnerability" -style group in addition to normal product-wise grouping. A clone of security bug should by default be a security bug - I believe this is what the user expects as well. > Time tracking should definitely *not* be copied, for two reasons: > * First, it would mean that you've got hours for one bug recorded in two > places. Time spent on the original and time spent on the clone are > tracked in each individual bug. Yeah, copying the whole time tracking information would probably lead to undesired results. However, copying the "estimated hours" field isn't probably such a bad idea: we have that field in enter_bug already, and copying it wouldn't probably hurt. In fact, it is probably even desirable: If I want the same fix done in three products, I just want to file it in one place and then use "clone" to create the two copies. Copying the time estimate is probably the best we can do. However, _if_ the "clone" is actually used to fork an already worked-on bug into two different issues, the result would truly be somewhat confusing. I still think the best way to do this is copying the estimate and leaving it up to the user to fix it correctly. > * Secondly, time tracking is stored in longdescs, and it's not possible > to copy a longdesc without taking the comment - can't have a blank > longdesc:thetext entry or many things break. Since we're not taking > the comments, we *can't* take the time entries. estimated_time is recorded in bug table, so we won't encounter this issue if we copy only that. > I tried that; I thought that it choked because it was finding a '-' character > where it wasn't expecting one, which is why I renamed it. I'll check it again > both ways and report back. Probably in the PROCESS directive? Quote the template path. > So... where do we go from here, boss? Fix up these nits and call it a > day? Or another complete re-write to eliminate clone-link altogether > and instead pass a bug_id into enter_bug? What do you think? Either approach is fine with me. I believe we should clone the security groups and the estimated_time field, but other than those and the couple of nits, I think your patch is fine. The bug_id solution might be more elegant and enable comment passing, but as I already said, I believe this approach is useful and worth getting checked in. FWIW, I'm not at all concerned about the additional DB retrieval that the bug_id approach involves, but the code changes for that will be much more complicated. This one is simple enough that I'd even be ready to ask Dave for a 2.20 approval if you get the patch done and working soonish.
Two thoughts from a quick review of the patch: - IMO there's no need for a controlling param. Why would anyone ever want to turn this off? - IMO we should force the new bug to be in the same product as the old by adding &product=<foo> to clone_url. Given that cloned bugs are likely to be very similar to the original, this is the 99.9% case, and it prevents _everyone_ having to go through the Select Product screen. The 0.1% of people can change the product afterwards in a separate step. Both these suggestions have the pleasant side-effect of making the patch quite a bit simpler. Other than that, nice one :-) Gerv
(In reply to comment #49) > - IMO there's no need for a controlling param. Why would anyone ever want to > turn this off? There's always somebody who wants to disable it. But I think you're still right - it's cleaner to make the few people do this by customizing the templates. Let's forget about the param. > - IMO we should force the new bug to be in the same product as the old by > adding &product=<foo> to clone_url. Given that cloned bugs are likely to be > very similar to the original, this is the 99.9% case, and it prevents > _everyone_ having to go through the Select Product screen. > The 0.1% of people can change the product afterwards in a separate step. This is more controversial. For bmo, your statement might hold. For other installations, not at all necessarily. The concept of product can f.e. be "a client site" - a typical bug might then be "Upgrade library xxx to y.yy". In this case, the typical cloning scenario is making one bug for each of the products. This is, of course, customizable through the templates. However, I'm inclined towards leaving it as it is now. It is probably easier for an admin to realize and use the possibility to _limit_ the functionality. If we limit clones to the same product by default, it requires quite complex understanding of many things to realize that the option could actually also be used for cloning between products.
(In reply to comment #49) As usual, Jouni seems to speak my mind for me. I have no issues with removing the parameter. At the time I thought it was a good idea, but I've sort of wondered myself if it was *really* needed as I progressed. (Remember, this bug represents my very first attempt at a code patch to Bugzilla, and I'm still learning.) I'll get rid of it in the next re-write. I will fight you tooth and nail on the product change, though, as it is an integral part of why we need it locally, and I can't believe that we're unique in this regard. I will concede that cloning across products may not be very useful on bmo... but honestly I cant see bmo really using cloning at all. (In fact, now that I'm thinking about it, bmo was one of the reasons I put in the parameter in the first place - I figured that cloning bugs would be disabled here. Don't we have enough duplication already without allowing the user one-click access to more? Hrm... maybe the parameter should stay in after all...) > Given that cloned bugs are likely to be very > similar to the original, this is the 99.9% case, and it prevents _everyone_ > having to go through the Select Product screen. The 0.1% of people can change > the product afterwards in a separate step. If I agreed with your numbers, I might also agree with your conclusion... but even then I think I'd argue in favour of flexibility over restriction. It is intuitive to have to pick a bug's product at bug creation; you already have to do it with every new bug that you make, so this adds nothing to that process. The alternative is to be forced into picking the 'wrong' product then going back to fix it later; this option would make no sense to a lot of people, and creates a lot more bugmail. The real kicker is that your scheme could make it so that someone *cannot* clone a bug. It's not difficult to imagine this scenario: Product X and Product Y have their own, mutually exclusive groups. Fred (who runs product X) gets cc:'d on a bug in Product Y because it seems to be related to his area. Normally, Fred can't see Product Y, but since he's in the CC field, he can see this particular bug. He agrees that it affects Product X, and goes to clone the bug. If the product is set at the time of the cloning, Fred will be told that he doesn't have the permissions to clone the bug, since he can't create a bug in the new product. Furthermore, while Jim (the head opf Project Y) can clone the bug, he can't change the product to Product X because he doesn't have access to that product group. If the product is *not* set at the time of cloning, Fred hits the 'Clone this bug' link, and is taken to the product choice screen... where only Product X shows up for him. He clones the bug into Product X and continues on his way. Finally: It's easier for a customizer to make something *more* restrictive than it is to make it *less* restrictive. If a given site wants to fix the product, they certainly can... and without having to understand too much of the code or patch either. > Both these suggestions have the pleasant side-effect of making the patch > quite a bit simpler. Gotta say, I don't consider the removal of one line from choose- product.html.tmpl to make things *that* much simpler. ;) If you'd like, I can even include the following in clone-link.html.tmpl: [%# Uncomment the following if you do not want to allow cloning into different products %] [%# product = bug.product FILTER url_quote %] [%# clone_url = clone_url _ '&product=' _ next_param %] That should make it trivially easy for people to alter it to fit their specifications... although I confess I don't recall seeing anything like this anywhere else. Would this make it easier to swallow? > Other than that, nice one :-) Thanks!
OK, I'm sold. Remove the param, leave the product stuff as-is, and add the comment as you suggest. :-) Gerv
Depends on: 276446
Attached patch Code patch for tip, take 3 (obsolete) — — Splinter Review
We're back to getting the information from the database, but in a much more elegant way. I've tested this in as many ways as I can think of testing, and cleaned up the code so it's as pretty as I can make it; hoping that this is an easy review, Jouni. :) Obsoleting Andreas' initial patches now, since a) I'm able to, and b) I'm confident I'll be able to see this through to the end.
Attachment #45537 - Attachment is obsolete: true
Attachment #45539 - Attachment is obsolete: true
Attachment #168881 - Attachment is obsolete: true
Attachment #171770 - Flags: review?(jouni)
I seem to have a problem with posting 3rd revisions... this isn't the first time I screwed it up. (The deleted one was unedited and contained some passwords, which is why Dave deleted it for me. Thanks dave!)
Attachment #171770 - Attachment is obsolete: true
Attachment #171774 - Flags: review?(jouni)
Attachment #171770 - Flags: review?(jouni)
Comment on attachment 171774 [details] [diff] [review] Code patch for tip, take 3 (for real) Looking pretty good... >+# If a user is trying to clone a bug >+# Check that the user has authorization to view the parent bug >+# Create an instance of Bug that holds the info from the parent >+$cloned_bug_id = $cgi->param('cloned_bug_id') || 0 ; >+ >+if ($cloned_bug_id > 0) { You don't need the "|| 0" or "> 0". >+if ($cloned_bug_id > 0) { >+ >+ $default{'component_'} = $cloned_bug->{'component'}; >+ $default{'priority'} = $cloned_bug->{'priority'}; >+ $default{'bug_severity'} = $cloned_bug->{'bug_severity'}; >+ $default{'rep_platform'} = $cloned_bug->{'rep_platform'}; >+ $default{'op_sys'} = $cloned_bug->{'op_sys'}; All these make me think it might be time to change the interface to the template. In other words, simply do: $vars->{'default'} = $cloned_bug; and then have the template do [% default.component %], [% default.op_sys %] etc. It seems to me to make semantic sense for the defaults for a bug entry form to be contained in something that is, or looks like, a Bug object. >+# We need to ensure that we respect the 'insider' status of >+# the first comment, if it has one. Either way, make a note >+# that this bug was cloned from another bug. Nit: leading spaces. >+ >+ $cloned_bug->longdescs(); >+ my $desc = "+++ This bug was initially created " . >+ "as a clone of bug #$cloned_bug_id +++\n\n"; This is an l10n problem. I know we do it elsewhere, but we shouldn't compound the problem. Can you please execute a template and get the returned string? >+ my $isprivate = $cloned_bug->{'longdescs'}->[0]->{'isprivate'}; >+ >+ $vars->{'comment'} = $desc; >+ $vars->{'commentprivacy'} = 0; >+ >+ if ( ( $isprivate <= 0 ) || >+ ( >+ ( Param("insidergroup") ) && >+ ( UserInGroup(Param("insidergroup")) ) >+ ) >+ ) { I don't know about anyone else, but this brace style has me completely flummoxed. :-) Can we use something a bit more like existing examples? Gerv
Attachment #171774 - Flags: review-
Attached patch Code patch for tip, take 4 (obsolete) — — Splinter Review
(In reply to comment #55) > You don't need the "|| 0" or "> 0". Stripped. > All these make me think it might be time to change the interface to the > template. Perhaps it is; I can even see your logic. I do hope, however, that you're not proposing it be done as part of *this* bug, are you? Seems to me like that's a whole 'nother issue... > Nit: leading spaces. Local style - reverse-indent comment paragraphs. I sort of like it, (and did it without thinking), but it's not a gamebreaker for me. Removed. > >+ > >+ $cloned_bug->longdescs(); > >+ my $desc = "+++ This bug was initially created " . > >+ "as a clone of bug #$cloned_bug_id +++\n\n"; > This is an l10n problem. I know we do it elsewhere, but we shouldn't compound > the problem. Can you please execute a template and get the returned string? mutter grumble... fix the world's problems on the back of my bug... ;) Alright, I moved the string down into create.html.tmpl; it is prepended to the comment if cloned_bug_id exists. Someone still has to replace the string if they want to localize, but now they do it in a template instead of a .cgi file. Does that satisfy the requirement? > I don't know about anyone else, but this brace style has me completely > flummoxed. :-) Indented by clause... perfectly readable to me, but then again I wrote it. Changed somewhat for the hard-of-deciphering. ;-) I also fixed a couple of other things that I found I hadn't tested - you can't take the defauly bug_status from the clone, for example, because then you could end up with a new bug that's trying to start out with a status of RESOLVED (which breaks all sorts of things). Fixed now.
Attachment #171774 - Attachment is obsolete: true
Attachment #171822 - Flags: review?(jouni)
Attachment #171774 - Flags: review?(jouni)
Comment on attachment 171822 [details] [diff] [review] Code patch for tip, take 4 This crashes with "Can't use an undefined value as an ARRAY reference at /var/www/bugzilla/tip/enter_bug.cgi line 357." if you have no CC list. The first line of the actual comment is strangely indented. If my first comment is: -- Test content, line 1 Test content, line 2 Test content, line 3 -- The cloned result becomes: -- +++ This bug was initially created as a clone of Bug #22. +++ Test content, line 1 Test content, line 2 Test content, line 3 -- >Index: enter_bug.cgi >=================================================================== >+ $vars->{'dependson'} = $cloned_bug_id; WHAT? This causes pretty interesting results. >+ $cloned_bug->longdescs(); >+ my $isprivate = $cloned_bug->{'longdescs'}->[0]->{'isprivate'}; Nit: Indent the first line properly. >+ $vars->{'comment'} .= $cloned_bug->{'longdescs'}->[0]->{'body'}; >+ $vars->{'commentprivacy'} = $isprivate; Nit: This would look much more cool if the =s were aligned ;-) >+ # Use the version specified in the URL, if one is supplied. If not, >+ # then use the cookie-specified value. (Posting a bug sets a cookie >+ # for the current version.) If no URL or cookie version, the default >+ # version is the last one in the list (hopefully the latest one). >+ # Eventually maybe each product should have a "current version" >+ # parameter. I don't think this code works for me. If the version set in the originating product is not available in the new product, nothing is set and an ugly error is shown (unless I manually pick something). Starts looking good though :-)
Attachment #171822 - Flags: review?(jouni) → review-
Attached patch Code patch for tip, take 5 (obsolete) — — Splinter Review
(In reply to comment #57) > (From update of attachment 171822 [details] [diff] [review] [edit]) > This crashes with "Can't use an undefined value as an ARRAY reference at > /var/www/bugzilla/tip/enter_bug.cgi line 357." if you have no CC list. argh. good catch. Fixed. > The first line of the actual comment is strangely indented. No indication in the .tmpl file as to why it was doing this. I re-wrote this chunk of text, and now it doesn't add any extra spaces. (Looks a lot nicer too! :) > Nit: This would look much more cool if the =s were aligned ;-) Done deliberately as I did not want people to mistake the ".=" for an "=". It's unnecessary now that I no longer pre-fill the comment with the "+++ This bug..." string (as of previous revision), so I changed it to an '=' and lined it all up real pretty. :) > >+ $vars->{'dependson'} = $cloned_bug_id; > > WHAT? This causes pretty interesting results. Alright, long conversation in IRC about this between Jouni and I, here are the results. As per comment #14, attachments/comments/votes/dependencies are handled differently than everything else (which, as near as possible, is just copied directly from the parent into the child). attachments: aren't taken at all. votes: aren't taken at all. comments: first comment only is taken, and prepended with a string to indicate that it is a copy of a comment in another bug. If the cloning user can't see the first comment, only the prepended string exists. Dependencies... aye, there's the rub. Jouni was of the opinion that a cloned bug should have the same dependencies as its parent; it is, after all, supposed to be a clone/copy. I do not think that will always (or even usually) be useful information, and I also think that there should be a direct relationship between a cloned bug and parent; as such, I have made the cloned bug depend on the parent itself with no other dependencies or blockages. Whichever way is chosen it is going to be useful for some situations and not for others... so Jouni has agreed not to deny review based on this criteria, and to let Dave decide if he feels strongly about it one way or the other when giving approval. > I don't think this code works for me. If the version set in the originating > product is not available in the new product, nothing is set and an ugly error > is shown (unless I manually pick something). Right. That was the intention -- to ensure that you manually picked something. I am aware, however, that this is different behaviour from the default entry form. This section is modified now so that it will always pick a version again -- either the same version as the parent (if the product is the same) or a version based on the rules that were here before. Onward!
Attachment #171822 - Attachment is obsolete: true
Attachment #171917 - Flags: review?(jouni)
Comment on attachment 171917 [details] [diff] [review] Code patch for tip, take 5 Your patch to bug 266579 made this bitrot (hunk 6 of enter_bug doesn't apply), and applying this by hand seems non-trivial, so I can't test this reliably enough to r+ anyway. By inspection it looks fine apart from the single issue mentioned below. I'll test the next patch, then. BugMail.pm: ----------- > # disable email flag for offline debugging work >-my $enableSendMail = 1; >+my $enableSendMail = 0; Let's not check this in, please. :-)
Attachment #171917 - Flags: review?(jouni) → review-
(In reply to comment #56) > Perhaps it is; I can even see your logic. I do hope, however, that you're not > proposing it be done as part of *this* bug, are you? Seems to me like that's > a whole 'nother issue... Well, it would mean you made a different set of changes to the template interface instead of the set you are making at the moment. But I suppose it can wait. > mutter grumble... fix the world's problems on the back of my bug... ;) Not at all - I'm just asking you not to make things any worse :-) I didn't ask you to fix all the other occurrences of this, after all... ;-) > Does that satisfy the requirement? Yes - the text to change has to be in a template. That's all it is. Gerv
Attached patch Code patch for tip, take 6 (obsolete) — — Splinter Review
- Updated to HEAD (as of the time of creation) - Removed the BugMail oopsie from the patch.
Attachment #171917 - Attachment is obsolete: true
Attachment #172236 - Flags: review?(jouni)
Attachment #172236 - Flags: review?(jouni)
As above, plus: - add a line to filterexceptions so that the test suite runs successfully.
Attachment #172236 - Attachment is obsolete: true
Attachment #172237 - Flags: review?(jouni)
Comment on attachment 172237 [details] [diff] [review] Code patch for tip, take 7 r=jouni
Attachment #172237 - Flags: review?(jouni) → review+
Dave/Myk; Before approving, please take into account the 'dependencies' issue raised in comment #58. If you want it addressed differently than this patch handles it, speak now or forever hold your peace. ;)
Flags: approval?
Target Milestone: Bugzilla 2.22 → Bugzilla 2.20
I like the way you did dependencies... this is all just preloading a new bug form for you, correct? So the user can still change things before the new bug is actually created? If that's how it works, consider it a+'d. We can hash out the little stuff in individual bugs if people don't like specific things about the way it works.
Flags: approval? → approval+
Thank you to everyone who helped me shepherd my first significant enhancement attempt through the process. When I started, I had never even made a patch file before... and now I'm checking in the bug myself. :) Short time, long way, lot of learning taken place. Onward and upward!! Checking in enter_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v <-- enter_bug.cgi new revision: 1.101; previous revision: 1.100 done Checking in template/en/default/filterexceptions.pl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v <- - filterexceptions.pl new revision: 1.32; previous revision: 1.31 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.14; previous revision: 1.13 done Checking in template/en/default/bug/create/create.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tm pl,v <-- create.html.tmpl new revision: 1.41; previous revision: 1.40 done Checking in template/en/default/global/choose-product.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/choose- product.html.tmpl,v <-- choose-product.html.tmpl new revision: 1.11; previous revision: 1.10 done
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
*** Bug 252807 has been marked as a duplicate of this bug. ***
I have found what appears to be a (very small) bug in the patch. there is a trailing space in the following line: + [% IF cloned_bug_id %]&amp;cloned_bug_id=[% cloned_bug_id FILTER url_quote %][% END %] This causes breakage if you pass a format into create_bug.cgi. I'm not re-opening, since I can't verify that the bug made it into CVS, but I'd recommend that it be checked for. Besides that little quirk, bug-cloning is working great, thanks.
Dave; thanks for pointing this out, but I believe it has already been identified and fixed as bug 283424. I have marked that bug (and one more that pointed out an error in implementation) as being blocked by this one so it's easier to spot them.
Blocks: 282983, 283424
Dependecies inheritance declared in comment 58 (cloned bug depends on his parent) is not useful. Bug cloning extremely useful when we, working on some bug, create some subtask, cloned from general task. In this case, parent bug must depends from subtask (see "timetracking summary" reports and so on). In our company we use (enter_bug.cgi) $vars->{'dependson'} = ""; $vars->{'blocked'} = $cloned_bug_id; instead of # $vars->{'dependson'} = $cloned_bug_id; # $vars->{'blocked'} = ""; and we propose this scheme (or may be make it customizeable by Bugzilla parameters).
(In reply to comment #70) > Dependecies inheritance declared in comment 58 ... is not useful. As was also declared in comment #58: > Whichever way is chosen it is going to be useful for some situations and not > for others... Dave decided he liked it this way, so this is how it is, and I doubt you'll get it changed now. If you'd like to open up an enhancement request for customizing it via parameter, by all means feel free! Personally, I'm not sure there's a need... and it looks like you already know how to fix the issue for your site, though, (your changes look to me like they should do the trick). If you do open one, though, please cc: me on it.
Blocks: 355302
Making the dependency behaviour of cloned bugs configurable is bug 428102.
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

Creator:
Created:
Updated:
Size: