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.
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
I could not find a way to delete a data set
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
Created attachment 190909 [details] [diff] [review]
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
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 191015 [details]
Example "Confirm Data Set Deletion" page
Created attachment 191018 [details] [diff] [review]
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
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]
Since Gerv is supposed to be back next week, I might as well ask him for
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]
>RCS file: /cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v
>retrieving revision 220.127.116.11
>diff -u -r18.104.22.168 using.xml
>--- using.xml 12 Jul 2005 10:55:56 -0000 22.214.171.124
>+++ 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.
>+ 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.
Although it shouldn't break the documentation, there is a small typo
Created attachment 191940 [details] [diff] [review]
Modification of attachment 191018 [details] [diff] [review], taking into account comment 10 (among other
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]
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.
*** Bug 518778 has been marked as a duplicate of this bug. ***
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:
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]
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]
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]
As far as I can see, you don't delete any of the data associated with the series. Is this intentional?
Comment on attachment 403047 [details] [diff] [review]
(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
Checking in Bugzilla/Series.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Series.pm,v <-- Series.pm
new revision: 1.20; previous revision: 1.19
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
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
RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/delete-series.html.tmpl,v
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
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
Added to the release notes in bug 547466.
*** Bug 597223 has been marked as a duplicate of this bug. ***