|
Description
Eugene Kuleshov
This is needed to prevent too many duplicate bug reports coming in once we have submission via error log (bug 135605). Note that we did a partial implementation of this waaaay back, the result of which is in org.eclipse.mylar.sandbox.bridge.bugs. Here is what you should do to start: 1) Add a page to the new bug creation wizard called "Find Related Reports" 2) In addition to Eugene's summary/description search suggestion, add an option for "Find similar stack traces" button, and a table which uses the same infrastructure as our Search results view but shows a multiline Description field so that the reports can be previewed without opening (see "Custom draw Tree" in http://download3.eclipse.org/eclipse/downloads/drops/R-3.2-200606291905/new_noteworthy/eclipse-news-part3.html) 3) Implement the search by using the stack trace matching code either from the sandbox or by extracting it from our JavaStrackTraceHyperlinkDetector 4) If a duplicate is found the user should be prompted to open and comment on the corresponding report in addition to being able to file a new one Hmm. Whouldn't it make sense to add to the submission of the newly entered task in task editor and not wizard? So, it would work not only for issues from the error log... Yes, that's what I was thinking too. The stuff in comment#1 is one extra page in the comment selection page called "Automatic Duplicate Detection". Clicking Finish from that page has the effect of either opening an existing bug editor if a duplicate was found, or a new bug editor if not (possibly auto-populated with values from bug 135605). Bumping the priority since we need this to avoid getting spammed with bug reports via the functionality from bug 135605 ;) Jeff: do you think you can have a core version of this working for Friday's dev build? It would be good to announce it for people to try out. So whats the final consensus on the UI, add a page to the wizard as in comment 1 and then open an existing editor if a dup is found like in comment 3, or show search results like point 3 of comment 1? What if the dup search returns multiple hits (for the auto-open exisiting editor idea)? Yes, we can't be opening editors indiscriminately. The problem here is the modal wizard. Perhaps on the 'duplicates found' wizard page the results could include the description in addition to the summary reducing the need to open an editor? At this point the user will more than likely have a good idea if the report is in fact a duplicate. From that wizard we can give them the option to open selected tasks in editors (which will result in terminating the wizard). Rob: see comment#1 point (2) for how this table will summarize reports without the need to open them. Jeff: Both comment#1 and comment#3 are referring to the same thing, just not being very clear about it. Multiple search results should get shown, and then the user can select one of them to open if they believe it is a duplicate, causing the existing editor to open. If they don't see a match in the list that looks like a duplicate they can create a new bug report. Perhaps the best way to integrate this is with a check box at the bottom of the "New Repository Task" page: [check] Search for related stack traces before creating [hyperlink] Close wizard and search for related reports before submitting If selected the "Find Related Reports" page shows up. The checkbox should only be enabled if invoked from the Error Log action, or if there is a stack trace in the clipboard. The page should look something like this: [check] Use stack trace [text with stack trace, they can either paste one in, or Error Log action will paste in automatically] Note that this punts on the summary/descrption search for now, since our Search page already provides those facilties and they will be tricky to integrate into the wizard. Recap (with a few questions)
1. We add a new (optional?) page to the New Task Wizard.
2. This page allows the user to
2.1 enter some summary/description terms (separate text fields?)
2.2 Select a check box to search for similar stack traces (auto selected by the create from PDE log) and
enter a stack trace into a text box (auto-filled by the create from PDE log)
3. Results are shown
3.1 The user clicks a button and the results are shown in a table in that same wizard page.
-- or --
3.2 The user clicks "next" and another wizard page displays the results
4. User checks off which reports they think might be dups and clicks "finish", the selected reports are
opened.
4.1 If no reports are selected or there are no search results, a new report is created.
I'm not sure I understand what your suggesting with the hyperlink, also, it sounds like point 2.1 should not be included from your last comment.
Mik, I'm still unsure about when the actual search takes place. Does it happen in the wizard with results displayed in the wizard page, or after the wizard is finished with results in the search view/new dialog, or is the duplicate detection just flagged by the wizard and the search happens when the user tries to submit the bug? Bjorn Freeman-Benson and Ward Cunnigham visited yesterday and we chatted about this. All were psyched about this feature because it will help with a major problem on eclipse.org: all the duplicate bug filings. Regarding comment#8: 1: yes, the page is optional. Call it "Automatic Duplicate Detection" 2.1: skip this for now, just do the stack trace thing 2.2: yes, but note that the check box comes on the parevious page. From comment#7: bottom of the "New Repository Task" page: [check] Search for related stack traces before creating [hyperlink] Close wizard and search for related reports before submitting 3: the user clicks "Next" and a page with the duplicate matches table is shown. Text on page is: "If you see a duplicate candidate select it to open and comment. If not press finish to create new one." 4: yup! How is it going Jeff? (In reply to comment #11) > How is it going Jeff? > Struggling a little with the refactored code base (I've got some bugs to file once I pinpoint the problem). I'm getting NULL pointers and no connectors showing up. Is this still happening? If so please include the NPEs since AllTests pass on my end and connectors show up. Let me know how this is coming along and if you have any other problems. Mik, all the issues I was having before have been resolved now and everything is comming along. My first implementation was becoming a bit of a hack so I scrapped it and started over on tuesday. I now have the UI in place and a framework in the tasks plug-in to allow connectors to provide duplicate detection (they essentially provide the functionality to submit the query). The wizard implemenation ended up being much more complicated than I thought since the actual new bug wizard doesn't get created when the new task wizard does (it's not created until a repository is selected and the user clicks next). I had to create a class to queue the duplicate detecting wizard pages so they could be added by wizards which support duplicate detecting (they do so by extending an abstract new report wizard class). I can summarize all the details of how this works on the mailing list so people writing connectors can add the support. Its actually quite simple, I think my babbling is making it sound more complicated than it acutally is :) I am working on the bugzilla implementation now, once thats done I'll write some test cases and post up a patch. Yes, I'm getting your babble because I implemented the wizards to work on-demand this way, so it makes sense that you had to do extra work to queue in the extra pages. Looking forward to the patch... Consider posting a screenshot or two before it's completed in case feedback at this stage would be useful, and remember to get the simplest thing going and tested before making this fancy, because there is no shortage of interesting ways to make this fancy (which we should explore once the core thing works). (In reply to comment #15) > and remember to > get the simplest thing going and tested before making this fancy, because there > is no shortage of interesting ways to make this fancy (which we should explore > once the core thing works). > Yeah, thats what I've done. I have a rather simple implementation for the bugzilla connector, once I'll post a patch a little later once I've written some tests and fixed a couple minor bugs. I agree there are many very interesting ways to persue this. As for the UI, I've essentially just picked one simple UI but its easy enough to adapt it to other ideas. I'll post a screen shot and explain some of the limitations I've encountered. Created attachment 46617 [details]
Screen shot - Related reports page
This shows the related reports page. I've nested the first bit of the description as child elements so the list doesn't get too long with many reports.
One of the problems with this type of table/tree, is that every element is sized to the size of the largest element. So if we use a 10 line description as a table/tree element, everything becomes huge.
other info also comes up in the tooltip (id, date, creator, assigned to). *not shown in screen shot.
clicking finish would cause the selected report to open.
Suggestions * Limit the number of 'extra' lines (i.e. those below the title) to two, and make those contain the first n characters of the description. For an example of this UI see the attached screenshot. * Make this RepositoryTaskSummaryTable and Viewer it's own class, it could be useful outside of the wizard. * Consider extending or reusing and improving the viewer used by our search results because these two have a very similar role and should show columns, etc. Created attachment 46618 [details]
screenshot example of row details in Outlook
Created attachment 46620 [details]
Add duplicate detection
Mik, here's a patch so you can experiment with what's there. I still have to extract the tree into a separate class as you've suggested and move some classes out of the internal tasks namespace so connectors can legitamately use them.
I'm not sure its possible to implement the type of table/tree in your screen shot (at least with the tree I'm using). If I make a child node 3 lines, all nodes will share that height (thats the way it currently is), the one in your screen shot has 1 line nodes on top, and larger ones as child nodes. Also, I don't know if a child node can span accross columns as in your screen shot.
I initially listed 3 lines worth of description by character, I've updated it to word wrap 3 lines (calculating fit by average character width) and add "..." if the description is being cut.
I also left a (currently disabled) text field for "Search terms" so the find related reports can be extended beyond just stack traces. That can be removed or implemented as needed.
And lastly, wizard descriptions wouldn't show up without and image, so I just stuck the bugzilla one in their for now :)
Created attachment 46621 [details]
mylar/context/zip
Oh, one other interesting point. You can generalize your stack trace search by trimming it. For example you could trim all details except for "java.lang.NullPointerException" to get all reports (of that product) with that exception... or you could trim it to: Java.lang.NullPointerException at org.eclipse.mylar.internal.tasks.ui.editors.AbstractRepositoryTaskEditor.setDescriptionText to get all null pointers in that method, or trim the method name too to get all null pointers in that class, etc.... I am not very big fan of using forms in dialogs, though in this case it may worth to try. Created attachment 46646 [details]
list prototype without custom rendering
I am not a big fan of using forms in dialogs and wizards, but it seems they give better rendering options. Here is the list implemented using expandable composite and form text. Though I didn't manage to fix the vertical scrolling.
Good start and test Jeff, patch applied. Let's get this refined and into Friday's 0.6.1 release. I used bug#41 on mylar.eclipse.org for testing. Things to fix: 1) The check boxes wierd. How about you just make the tree one level deep so that the first line shows the Summary, and the additional 2-3 lines show the description. This will get rid of the collapse ability, but I think the overall result will be better and more simple. Haven't taken a look at Eugene's prototype list, that might help too. 2) Show the icon for the bugt in the tree to make it recognizable. You can get it from one of the existing label providers. 2) Move the "Search for related" check box to the top of the wizard page--it's too easy to miss. 3) Remove "Search terms" until we support it. 4) I got this stack trace when re-invoking: java.lang.NullPointerException at org.eclipse.mylar.internal.tasks.ui.wizards.DisplayRelatedReportsPage.setRelatedTasks(DisplayRelatedReportsPage.java:231) at org.eclipse.mylar.internal.tasks.ui.wizards.FindRelatedReportsPage.getNextPage(FindRelatedReportsPage.java:103) at org.eclipse.jface.wizard.WizardDialog.nextPressed(WizardDialog.java:751) at org.eclipse.jface.wizard.WizardDialog.buttonPressed(WizardDialog.java:351) Created attachment 46850 [details]
Screen Shot - Collapseable form based table
I had to add a listener to explicitly layout the scroll composite in order to get the collapsing to work, I think this will look better than the current custom render table though (will post a screen shot of that next). The only issue is that the check box will have to be part of the description so the user has to expand in order to select it.
Created attachment 46851 [details]
Screen Shote - Custom render table
I removed the description as a child node and added it to the same item, the result was a little messy so I indented the description. I wanted to make the summary bold or something but since its all one table item I can't really render parts of it differently AFAIK.
Which UI approach should I go with?
By the way, I don't think we should have "search duplicates" check box on the first wizard page. Since it is repositoryspecific it should be on a secong page... or at least connector should declare "capacity" for this, so wizard could disable check box for repositories that does not support this feature. BTW, what is that hyperlin for? It does nothing when I tried it. Hmm. I tought TableWrapLayout would handle that. At least it dos on my test. Also, it is possible to add a control to the expandable composite itself, so checkbox could be added there. PS: SWT designer is really grreat tool. I basically sketched all my patch in it... It is possible to test a repository connector to see if it is capable of duplicate detection but it involves instantiating it's new report wizard. I was also thinking we could leave this check box out entirely and just allow the user to click "next" for duplicate detection (iff supported) or click finish to create a new report. The link functionality was broken, I have a fix for it that I can include in my next patch... however I was also thinking we should either leave the link out (because its currently redundant functionality of the "Cancel" button) or have it cancel the wizard and open the bugzilla search pane. >Hmm. I tought TableWrapLayout would handle that. At least it dos on my test. Could be a bug on OSX I suppose. >Also, it is possible to add a control to the expandable composite itself, so checkbox could be added there. ah, good. Ill try that. I'll have to take a look at SWT designer as well. Duplicate detection relies on one capability on the connector: querying task data such as description and comments. Since Trac needs it too, I'm going to make this capability explicit via bug 142783. For now just leave this visible and make the hyperlink open the search page to the Bugzilla Tab, which will become a generic Task search tab. We may still want to consider improving the wizard flow as you described above but that can wait for now. I like the approach of the first screenshot, with the following changes: 1) Get rid of the checkboxes, I think they're more harm than good. If a submitter spots one duplicate that's enough, the bug triager/developer is the one responsible for managing mutliple duplicates. The selection in the table will indicate what the duplicate is. 2) The collapsing is nice, so keep it, but make the first line look as much as possible like the entry in the task list (i.e. give it the icon). Maybe the icon can show up in the column that the check boxes are in now. 3) The search should run with progress because it can be very slow. As an example of how to run with progress open the properties of an existing Bugzilla repository and click Validate Settings, noting that you need to call setNeedsProgressMonitor(true) on the wizard in its constructor. I just thinking, maybe instead of another page on new task wizard it should go into the new task editor. We could have "Search Duplicates" button next to "Create" and it will run search that will be shown in a standard search view. So, user will be able to use common facilities, e.g. open issues in separate editors to review them. This way it could use entered description or issue details to complete search criteria, so it does not have to be a stack trace only. I think such interaction fits better into Eclipse UI and require less time to spend in the modal wizard. I really like this idea, thanks for putting thought into this Eugene. It reuses existing facilities and as you indicate avoid all of the problems that comes from having users be stuck in the modal dialog (e.g. if the first 3 lines of the description are not enough to figure out if it is a duplicate. One problem is that the facility won't be available to connectors that can search but that can't create new bug reports, but we may be able to address that by creating a custom editor for those that swallows the web browser and provides its own field for the stack trace. Jeff: let me know what you think and how hard it is for you to make this change--hopefully it actually saves you some time in not having to fiddle with the custom tree (which could still be useful for other places where we might want the preview facility). Here's what it seems you need to do, and it would need to happen by today or early tomorrow to go out with 0.6.1. 1) Add a "Find Duplicates" button before "Create" on the NewBugEditor (this UI will be so nice and obvious :) 2) Clicking "Find Duplicates" should kick off your current search, showing results as any Bugzilla Search view does, using any stack that's in the description. If there is no stack in the description you bring up a warning dialog. 3) Create a new bug report for finding duplicates based on summary, since it will have to do some textual matching. 4) Leave the hyperlink in the wizard, but pull out the check box and extra page that you're adding now. I like the idea, I'll start working on this. If I don't have a patch in time for 0.6.1 we can go with the wizard version for that release or just remove it entirely (I'll post a patch to fix the null pointer and link functionality) What about connectors that don't support finding duplicates (since the button will be part of the AbstractRepositoryEditor)? I suppose we could initialize the button as disabled and have supporting connectors explicilty enable the button and override a stub implementation of the search. hmm, I just noticed the abstract performQuery in the AbstractConnector class. I guess any connector can query, its just a matter of hooking that into the search results page. Is querying from the integrated search something bugzilla specific or is this something done at the abstract tasklist level? It would be great if we could perform the query using the abstract connector API so connectors don't need to do any additional work to have duplicate detection. I'll keep diggin through it, just thought I'd post the question in case anyone can point me to it to save some time :) Note from my comment#33 that the for now the button will only appear bugzilla.ui.editor.NewBugEditor. Just do it all Bugzilla-specific for now, and we'll pull this up tom make it available to any connector that can query, as you indicate. Note from my comment#31 that I'll be implementing that soon (bug 142783). If at all possible we should try to put 0.6.1 out with this UI, especially since getting the wizard-based one usable would take more work them implementing this one. We can delay the final build until Saturday if having tomorrow gives you enough time. Ah, ok. I'll do it bugzilla specific then. I think I should be able to have a patch by early tomorrow. Great. We'll make this nice and prominent in the New & Noteworthy and encourage everyone to submit Mylar and Eclipse bugs this way. Created attachment 46907 [details]
Move dup detection to editor from wizard
This patch reverses most of the previous patch (wizard stuff) and adds duplicate detection to the editor.
I removed the "cancel and search" link because we don't know what type of repository the user will want to search at that point in the wizard. Opening to the bugzilla search page assumes they are intending to file a bugzilla bug. (The code is still present, just commented)
Very nice! I just went through the "automatic submit, create, try to submit again, detect duplicates" loop and the interaction is very clear and quick. I think you have just the following two fixes left before we can close this? * Provide a test case, I noticed that you removed the previous one which was good because it exercised the stack trace search. * Ensure that you can handle a stack trace pasted in by the user, currently it can't. Try pasting the following in, it should find bug 48 as a match on bugs222. If this is too tricky when the start is an arbitrary line just trim the spaces and newlines off the start of the description and only support having it be at the start of the description (update the warning message accordingly so that it teaches people how to use it). java.lang.NullPointerException at org.eclipse.mylar.internal.tasks.ui.wizards.DisplayRelatedReportsPage.setRelatedTasks(DisplayRelatedReportsPage.java:232) at org.eclipse.mylar.internal.tasks.ui.wizards.FindRelatedReportsPage.getNextPage(FindRelatedReportsPage.java:103) at org.eclipse.jface.wizard.WizardDialog.nextPressed(WizardDialog.java:751) The old test case was based on the wizard classes, so it got removed with them. I'll create one for this. The stack trace detection currently looks for the identifier "Stack Trace:\n" which is inserted by the generated report from the error log. I'm not sure how we can identify the start or end of a stacktrace otherwise. Maybe a regex, I'll take a look at it. Don't bother with a regex yet--just assume that it is the start of the description (after trimming as described above), and fail with the warning if it doesn't parse. If you wanted to be fancier you could try starting to parse it at every line and going until it works and failing if you get to the last line. Btw, if you don't want the wizard classes to be stuck on the report patch you can make them a patch to org.eclipse.mylar/developer/src-old where we dump stuff we figure might be useful again at some point. Created attachment 46929 [details]
Improves stack trace identification, adds test case
By the way, should this new button also appear in the bugzilla issue editor too? That would allow to search duplicated after issue submitted (e.g. if submitter didn't run the search or not using Mylar). Also, I think that search should not create new search page if there are 0 hits found. Currently it create new page every time you hit the search buton. Patch applied. Good work getting this solid for 0.6.1 Jeff. Yes, let's add this action to the editor for existing tasks as well. There isn't a great place for it now, so we'll put it as a button after "Submit to Repository". Rob: please add it after you have committed your task editor changes. Eugene: we should leave the search page with 0 hits for now because this is consistent with the way that other Eclipse searches work, and because changing this interaction would require rewiring the way that this feature connects to search to delay showing the page and suppressing the "Searching..." message which indicates the duplicate detection is running. Jeff: one more thing, remember to put an @author tag for yourself for classes that you create or make substantial changes to, since we like to attribute credit where it is due. I added a tag for you to DuplicateDetectionTest. Duplicate detection tests are failing, potentially due to Rob's recent editor changes, could you investigate? java.lang.ClassCastException: org.eclipse.mylar.internal.bugzilla.ui.editor.NewBugzillaTaskEditor at org.eclipse.mylar.bugzilla.tests.DuplicateDetetionTest.testDuplicateDetection(DuplicateDetetionTest.java:54) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) Sorry about that beakage, fix should be in head now. Fix verified, AllTests are now happy. Hmm. I just tried to search duplicate from error log entry I just created posted error from (bug 152255) and search retrned 0 hits for it. I did changed description and added a comment before stack trace, but it should not affect the search results. Jeff: I just noticed the same thing. Here's what I noticed: * If stack trace is pasted in manually the search works * If stack trace comes from the auto-generated description from error event search fails to find matches Looks like a regression from your changes to support manually created stack traces. Please make sure to add a failing test case before fixing this. Let me know when you can get to this, we can hold putting 0.6.1 off until early Monday morning but please verify any changes made at this point very carefully. All of the mentioned situations work for me. Copy/paste or generate from error log... It always returns the correct search results. ?? The stack trace detection is done the same way independant of how the description is formed (in comment or auto-gen). I've been testing with 2.22, I wonder if its a problem with how the search string is treated with a different bugzilla versions. I'll look into that. I just tried to reproduce with bug 152255 but don't have that stack trace anymore. I'm wondering if I made a mistake when trying to reproduce (comment#51). Let me know what you find. Searches work for me in 2.22 and 2.20 I did however find a bug where text added directly after a stack trace caused the search to fail (the stack trace parsing grabbed the extra line). I'll post a fix for that, but otherwise I'm unable to reproduce the problem. Eugene, did you have text added after the stack trace? I know your stack trace had the "...20 more" but that doesn't cause a problem since its part of the original stack trace anyway. Created attachment 47016 [details]
Fixes text after stack trace, adds regression test
This patch fixes the issue of having comment text directly after the stack trace and adds a regression test to test it.
Hopefully this addresses the issue Eugene was seeing.
Created attachment 47017 [details]
mylar/context/zip
(In reply to comment #54) > Eugene, did you have text added after the stack trace? I know your stack trace > had the "...20 more" but that doesn't cause a problem since its part of the > original stack trace anyway. I've added text before stack trace. See bug 152255 I am running on windows, so there could be potantial difference in line breaks. Created attachment 47021 [details] error log with stack trace that has duplicated issues I just tried it with latest changes from CVS and it still don't find duplicates for tose issues. However if I delete text before stack trace (making "org.eclipse.swt.SWTException: ..." line first in the text area) it does find bug 152255 You can import attached log (Error Log view / Import Log) and try to create issues for any of the "Failed to execute runnable (java.lang.NullPointerException)" rows. When I use that error log the search finds 18 potential matches including 152255. I've tried with both the current head and with my latest posted patch. I've also tried with and without adding text before and after the stack trace. It works for me. Eugene, maybe you could try adding a breakpoint or a println() in org.eclipse.mylar.internal.editor.NewBugEditor::getStackTraceFromDescritpion(), to see if the stack trace being parsed is correctly. Maybe this is related to something platform specific. (In reply to comment #59) > When I use that error log the search finds 18 potential matches including > 152255. I've tried with both the current head and with my latest posted patch. > I've also tried with and without adding text before and after the stack trace. > It works for me. Weird. After I remove text it returns exactly 1 hit for bug 152255 ok, i've found the bug... a simple logic error in the parsing. I still don't know why I saw 18 results and you 0... but both are wrong either way. I'll have a patch up in a minute to fix this. Created attachment 47025 [details]
Fixes stack parsing, adds test to excersize bug
OK, This patch fixes the bug Eugene was seeing and adds a test case to excerise this issue with parsing.
Patch applied, good test. This is exactly the way we try to do things: write a few tests while creating the feature to ensure that it is testable (sometimes that can turn out to be more work than creating the feature), then for every bug that's found add a failing test and fix. I did a quick manual test with Eugene's and with another exception and the behavior was consistent. In my testing I noticed that NPE's tend to show too many matches, so I created bug 152279 |