448690 - Changing columns after editing a saved search forgets the edits

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[Jesse Clark]

Steps to reproduce:
1. Invoke a saved search (create one if necessary. "My Bugs" may not work)
2. Click the "Edit Search" link and modify the search so that it returns different results
3. Click the "Change Columns" link and make some changes

Expected results: the edited search shows the new column selection
Actual results: the saved search (as it was before editing) appears with the new column selection

Comment 1

[Teemu Mannermaa (:wicked)]

Attachment: Fix editing columns of edited saved searches, V1

This patch fixes the mentioned problem, that was probably caused by changes in bug 277466.

Template no longer passes remembered search name to colchange.cgi so that it no longer thinks user is changing a saved search. This way it correctly executes the search parameters and not run a saved search as is. Additionally a known name of the saved search is passed back to buglist so that it can suggest it as a saved search name. Now user can simply save the altered saved search with new column list.

Comment 2

[Frédéric Buclin]

Fun, I encountered this problem myself yesterday, and wondered if a bug had already been reported. Now I have my answer. :)

Comment 3

[Frédéric Buclin]

(18:58:25) LpSolit: wicked: create a saved search which has custom columns, i.e. different from what non-saved searches have
(18:58:38) LpSolit: wicked: then edit the saved search
(18:58:54) LpSolit: wicked: the buglist displays the edited saved search with default columns
(18:59:15) LpSolit: wicked: and if you try to save your changes, this will also store default columns
(18:59:20) LpSolit: which we clearly don't want

Could you investigate why this happens? This seems to happen with and without your patch. Maybe the root cause is the same, in which case we must fix it correctly. So I will leave this review request as is for now for further investigation.

Comment 4

[Frédéric Buclin]

Comment on attachment 356469 [details] [diff] [review]
Fix editing columns of edited saved searches, V1

Removing this patch from my review queue till the problem mentioned in my previous comment has been investigated.

Comment 5

[Kent Rogers [:mrbball]]

A slight variation on this, which may or may not be a separate bug:

- Run a saved search
- Click on "Change Columns" link
- Check a new column that was not part of the saved search's custom column list
- Uncheck "Save this column list only for search ..."
- Click the "Change Columns" button

I would have expected the above set of steps to display the saved search,
with the newly selected column included, but without the newly selected
column being remembered in the saved search's custom column list.  Does
that make sense?

That is not what happens, though.  After clicking the "Change Columns"
button on colchange.cgi form, the saved query displays the same as it
ever did, apparently completely ignoring my request to add another
column to the display.

I can see that what actually happens is that the "Change Columns" action
is applied to the default column list (and not the saved search's custom
column list), but that is certainly not intuitive and quite disfunctional.

I occasionally want to run my saved search and 'temporarily' add a column
or two (i.e. without altering the custom columns that are saved with the
search).  But, there does not appear to be any way to do that without
jumping through an unreasonable number of hoops.

I'm not familiar with this area of the code, but would it be possible to
have two checkboxes on the colchange form in this case:

  - Save this column list for the default search
  - Save this column list only for search ...

Then in the absence of check marks in either of these two checkboxes,
the changes to the column list would only be temporary (i.e. only apply
to the current query)?

Comment 6

[Teemu Mannermaa (:wicked)]

I tested this to affect at least 3.2 and 3.4 branches as well. I think we should consider fixing this in all three branches.

(In reply to comment #3)
> Could you investigate why this happens? This seems to happen with and without

This is a different problem where custom column list is not preserved by query.cgi even though it's given to it. However, I did fix this at the same time in my patch.

(In reply to comment #5)
> A slight variation on this, which may or may not be a separate bug:

Yeah, this is same problem as this bug and is fixed by my patch.

> I occasionally want to run my saved search and 'temporarily' add a column
> or two (i.e. without altering the custom columns that are saved with the
> search).  But, there does not appear to be any way to do that without

I agree that this would be useful and currently there's no way to do this as either the saved search or the default column list (COLUMNLIST cookie) is changed. Could you file a new enhancement request bug asking for this feature? I'd want to keep that change separate from this bug fix.

Comment 7

[Teemu Mannermaa (:wicked)]

Attachment: Fix editing columns of edited saved searches, V2

My previous patch actually didn't fix all the problems but this one does and seems to give much better experience when editing saved searches and columns.

Most notably this makes colchange.cgi not to rerun saved search anymore but instead use the passed query parameters to run the query. This makes it more consistent with "Edit Search" and makes sure all the previous edits are not forgotten. Only time saved search could be run as is when columns are changed for an unmodified search AND the new columns are saved to the saved search. I decided to remove the running instead of to try to figure out when it can be safely done.

As mentioned it also makes query.cgi preserve custom column set in the URL.

Comment 8

[Kent Rogers [:mrbball]]

(In reply to comment #6)
> I agree that this would be useful and currently there's no way to do this as
> either the saved search or the default column list (COLUMNLIST cookie) is
> changed. Could you file a new enhancement request bug asking for this feature?
> I'd want to keep that change separate from this bug fix.
>
Just opened bug 493941.

Comment 9

[Frédéric Buclin]

Comment on attachment 376137 [details] [diff] [review]
Fix editing columns of edited saved searches, V2

As far as I can test, this is working fine. r=LpSolit

Did you test on the 3.2 and 3.4 branches too?

Comment 10

[Frédéric Buclin]

Waiting for wicked's answer to comment 9 before granting approval on branches.

Comment 11

[Frédéric Buclin]

Oh, the patch doesn't apply cleanly on 3.2.4.

Comment 12

[Teemu Mannermaa (:wicked)]

Attachment: Fix editing columns of edited saved searches - v3.2 backport, V2

Here's the backport to 3.2 branch. No major changes since there was only little cleanup/changes in recent branches to colchange.cgi and query.cgi scripts.

I tested previous patch on 3.4 branch and it seems to work just like on the tip.

Comment 13

[Frédéric Buclin]

Comment on attachment 388168 [details] [diff] [review]
Fix editing columns of edited saved searches - v3.2 backport, V2

r=LpSolit

Comment 14

[Teemu Mannermaa (:wicked)]

Trunk:
Checking in colchange.cgi;
/cvsroot/mozilla/webtools/bugzilla/colchange.cgi,v  <--  colchange.cgi
new revision: 1.67; previous revision: 1.66
done
Checking in query.cgi;
/cvsroot/mozilla/webtools/bugzilla/query.cgi,v  <--  query.cgi
new revision: 1.184; previous revision: 1.183
done
Checking in template/en/default/search/knob.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/search/knob.html.tmpl,v  <--  knob.html.tmpl
new revision: 1.22; previous revision: 1.21
done

3.4 branch:
Checking in colchange.cgi;
/cvsroot/mozilla/webtools/bugzilla/colchange.cgi,v  <--  colchange.cgi
new revision: 1.66.2.1; previous revision: 1.66
done
Checking in query.cgi;
/cvsroot/mozilla/webtools/bugzilla/query.cgi,v  <--  query.cgi
new revision: 1.183.2.1; previous revision: 1.183
done
Checking in template/en/default/search/knob.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/search/knob.html.tmpl,v  <--  knob.html.tmpl
new revision: 1.21.4.1; previous revision: 1.21
done

3.2 branch:
Checking in colchange.cgi;
/cvsroot/mozilla/webtools/bugzilla/colchange.cgi,v  <--  colchange.cgi
new revision: 1.62.2.3; previous revision: 1.62.2.2
done
Checking in query.cgi;
/cvsroot/mozilla/webtools/bugzilla/query.cgi,v  <--  query.cgi
new revision: 1.181.2.1; previous revision: 1.181
done
Checking in template/en/default/search/knob.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/search/knob.html.tmpl,v  <--  knob.html.tmpl
new revision: 1.21.2.1; previous revision: 1.21
done