User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040804 Netscape/7.2 (ax) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040804 Netscape/7.2 (ax) I would like to be able to delete one or more series in New Charts. Reproducible: Always Steps to Reproduce: 1. Delete an already-existing product 2. Go to New Charts and attempt to delete the data sets associated with the product Actual Results: I could not find a way to delete a data set Expected Results: I should have been presented with a list of data sets to delete This can cause a number of problems. See bug 247936.
Created attachment 190906 [details] Example "Delete Data Sets" page This is what you will see when you click on the "Delete Data Sets" link in chart.cgi.
Created attachment 190909 [details] [diff] [review] Patch v1 This is my first attempt at a patch for this bug. Yay! chart.cgi is modified to include two new actions: delete and delete_set. In the case of 'delete', the user is shown a list of all data sets that they can delete. The user checks boxes next to each set to be deleted, and pushes a button that activates the next action, 'delete_set'. In the case of 'delete_set', the list of items to delete is checked, to make sure the user actually has permissions to delete the data sets. Then each data set is deleted and the initial New Charts page is displayed. When a data set is deleted, all of the entries in series_data (that are associated with the date set) are deleted. Then the data set entry in series is deleted. Finally, the catagory/subcategory that held the data set is checked. If the category/subcategory is now empty, it's also deleted. The list of data sets that can be deleted is displayed using it's own template (a new file). The list of filter exceptions is updated to include one directive that can't be filtered. There is also a new user-error and a new code-error. All of the new methods created for data set deletion are stored in the new module Bugzilla::Series::Util module. I created this module because all of the new operations do not require the existence of a Bugzilla::Series object (in C++ terms, they're static functions). A POD is included. I'm not sure who to ask for review on this one.
I have to remember to put the appropriate docs patches into a later attachment.
Created attachment 191018 [details] [diff] [review] Patch v2 Modification of attachment 190909 [details] [diff] [review]. First of all, this attachment includes all needed documentation patches. The first big change is the introduction of an "are you sure you want to delete?" page (see attachment 190915 [details]). The page will list all of the series that were selected (see attachment 190906 [details]) and will ask if the user really wants to delete them, noting that all data will be lost. The user is also given the opportunity to (a) view the query from which the set gets its data, and (b) view all data in the set in CSV (i.e. downloadable) format. The second big change comes in Bugzilla::Series::Util. The delete_series subroutine used to call can_mofify_series to ensure that the user doing the deleting actually had permissions to do so. This check has been removed; chart.cgi is now responsible for checking permissions before calling delete_series. This will be particularly useful when the permissions needed to delete a series varies depending on the action being performed. delete_series still checks to make sure that the series_id is valid. I'm asking mkanat for review, as he's listed as a reviewer for .pm & .cgi files and gerv seems to be unavailable. mkanat: If you think someone else may be better to review this, please don't hesitate to assign it to them! I'm also asking documentation@ to confirm that the docs patches don't break the documentation.
We just can't make major changes to charting when we want our next version to be the actual release version.
Comment on attachment 191018 [details] [diff] [review] Patch v2 Since Gerv is supposed to be back next week, I might as well ask him for review!
Max, can you please do some resolving on blocking2.20 here? This bug has blocking2.20- while at the same time blocks blocking2.20+ bug 247936.
(In reply to comment #8) > Max, can you please do some resolving on blocking2.20 here? This bug has > blocking2.20- while at the same time blocks blocking2.20+ bug 247936. OK. See bug 247936 comment 17.
Comment on attachment 191018 [details] [diff] [review] Patch v2 >Index: using.xml >=================================================================== >RCS file: /cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v >retrieving revision 126.96.36.199 >diff -u -r188.8.131.52 using.xml >--- using.xml 12 Jul 2005 10:55:56 -0000 184.108.40.206 >+++ using.xml 30 Jul 2005 02:26:28 -0000 >@@ -1118,6 +1118,15 @@ > and reduce the frequency of data collection to less than the default > seven days. > </para> >+ >+ <para> >+ If you no longer need the data set, you may remove it by >+ clicking on the "delete data sets" link on the Create Chart >+ page. You will be presented with the list of data sets that you >+ may delete. Select one or more and click on the "delete" >+ button. Before deleting, you will be given the opportinuty to >+ download the data set in CSV form. >+ </para> Although it shouldn't break the documentation, there is a small typo (opportinuty)...
Created attachment 191940 [details] [diff] [review] Patch v2.1 Modification of attachment 191018 [details] [diff] [review], taking into account comment 10 (among other things). It turns out that I was wrong about one thing. I had originally thought that, upon creation of a new product, all automagically-created data sets would have Bugzilla (ID 0) set as the creator. It turns out that the creator is being set to the ID of the person who created the Product. I'm changing it so all automagically-created data sets have Bugzilla set as the creator. Why do I do this? Two reasons. #1, I use "series.creator == 0" (among other things) to identify automagically-created series. Without this it becomes much harder to identify automatically-created series. In fact, without this it may not be possible to guarantee a perfect ID (relying on series/category/subcategory name on top of query content could result in false positives/negatives). #2, if the creator is a specific person, that (possibly non-admin) person will also be able to delete the automagically-created data sets at any time, which may not be a good idea. I also added a confirmation page, so users will have one last change to cancel their data set deletion. They are also given the opportunity to download the data set in CSV form for archving, and they can also take a look at the query used to gather said data. The typo in the docs has also been fixed. Requesting review from Gerv and documentation@.
Comment on attachment 191940 [details] [diff] [review] Patch v2.1 Docs bit looks OK...
This bug doesn't have any active review request on it... shouldn't it?
(In reply to comment #13) > This bug doesn't have any active review request on it... shouldn't it? > Not if there's a big chance of bitrotting. Gerv's unavailable for the rest of Nov. 2005. I might have one other option, though. If that option comes through I can post an up-to-date patch and target this for 2.22.
*** Bug 316704 has been marked as a duplicate of this bug. ***
*** Bug 311073 has been marked as a duplicate of this bug. ***
Is there a change that this will get in 3.2 ? Target milestone is "---", by purpose ?
this bug is really annoying, any chance of getting it fixed any time soon ?
(In reply to comment #18) > this bug is really annoying, any chance of getting it fixed any time soon ? Probably not unless you fix it yourself, no. There is nobody currently assigned to working on Charts, at all.
1.5 years later, has anything changed on this topic? This is really annoying, as my boss loves this feature. Not being able to clean up the mess I created in our charts really gives me a hard time.
(In reply to comment #21) > 1.5 years later, has anything changed on this topic? This is really annoying, > as my boss loves this feature. Not being able to clean up the mess I created in > our charts really gives me a hard time. Yeah, I wish something had changed. Unfortunately, we still don't have anybody working on charts, though LpSolit (one of the primary developers of Bugzilla) *has* fixed a few charting issues, so perhaps this will be addressed. If you'd like to take a stab at it yourself, feel free. Our development process is described here: http://wiki.mozilla.org/Bugzilla:Developers
Yeah, I first focused on fixing some annoying bugs before starting to implement new features. I may decide to fix bug 276230 before this one; we will see.
Created attachment 403038 [details] [diff] [review] patch, v3 I add a link besides the Edit link and use tokens to prevent unwanted deletions. You need to apply my patch from bug 519040 first due to conflicts in global/messages.html.tmpl.
Just in case you are wondering, the patch contains one unrelated cleanup, where I replaced Bugzilla->user by $user where appropriate, and one small bug fix, where the Apache error log was reporting an undefined value when you call chart.cgi with no parameter. And yes, that's expected. :)
Created attachment 403047 [details] [diff] [review] patch, v3.1 I refactored assertCanEdit() to avoid code duplication. I also added a missing |require Bugzilla::User| to Series->creator (I cannot |use| it, due to dependency loops).
Comment on attachment 403047 [details] [diff] [review] patch, v3.1 As far as I can see, you don't delete any of the data associated with the series. Is this intentional? Gerv
Comment on attachment 403047 [details] [diff] [review] patch, v3.1 r=gerv. Gerv
(In reply to comment #27) > As far as I can see, you don't delete any of the data associated with the > series. Is this intentional? For the record, the series_data table has a foreign key pointing to the series table, with ON DELETE CASCADE. So deleting an entry from the series table will automatically delete the corresponding entries from the series_data table. No need to delete stuff manually.
Checking in chart.cgi; /cvsroot/mozilla/webtools/bugzilla/chart.cgi,v <-- chart.cgi new revision: 1.31; previous revision: 1.30 done Checking in Bugzilla/Series.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Series.pm,v <-- Series.pm new revision: 1.20; previous revision: 1.19 done Checking in template/en/default/global/messages.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/messages.html.tmpl,v <-- messages.html.tmpl new revision: 1.92; previous revision: 1.91 done Checking in template/en/default/reports/create-chart.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/create-chart.html.tmpl,v <-- create-chart.html.tmpl new revision: 1.19; previous revision: 1.18 done RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/delete-series.html.tmpl,v done Checking in template/en/default/reports/delete-series.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/delete-series.html.tmpl,v <-- delete-series.html.tmpl initial revision: 1.1 done
Created attachment 404575 [details] [diff] [review] 2nd part of patch v3.1 I just realized that I forgot to include reports/edit-series.html.tmpl in the diff. Checking in template/en/default/reports/edit-series.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/edit-series.html.tmpl,v <-- edit-series.html.tmpl new revision: 1.8; previous revision: 1.7 done
Added to the release notes in bug 547466.