Three sets of changes:

  1) Make sure that a user doesn't create a query with the same name
     as an existing query that the user owns.

  2) When submitting a new named query, display the name of the query
     on the index page.

  3) Allow optional arguments to indexargs_url by setting their value
     to None. This will show the argument only if there is a valid
     value.

To match these, changed the template for all search templates so if an
error is thrown due to #1 the user stays on the search page so they
can fix the issue.

Note that I did not add automated tests for these because I couldn't
find existing tests for these code paths that I could adapt. I don't
understand how the existing Action tests work and there is no doc for
them.

Author: Unknown

CHANGES.txt

@@ -378,6 +378,25 @@ Fixed:
 - issue2550932 - html_calendar produces templating errors for bad date
   strings. Fixed to ignore bad date and highlight todays date in the
   calendar popup.
+- Query handling requires that query names for a user are unique.
+  Different users are allowed to use the same query name. Under some
+  circumstances a user could generate a second query with the same
+  name. The SearchAction function has been corrected to report this
+  error. Also the index.search.html template in the classic tracker
+  and corresponding templates in the other example trackers
+  has been modified to include:
+    <input type="hidden" name="@template" value="index|search"/>
+  so an error from SearchAction will display an error message and keep
+  the user on the search page so they can correct the error. See
+  ``doc/upgrading.txt``. (John Rouillard)
+- When a new named search is created, the index page that is displayed
+  doesn't show the name. This has been fixed by setting the @dispname
+  to the query's name. (John Rouillard)
+- Passing args into indexargs_url(..,{'@queryname': request/dispname
+  or None, 'Title': 'some' }) where the value of the arg is None
+  will not add the arg to the url. In the example above @queryname
+  will only be in the url if dispname is set in the request.
+  (John Rouillard)
 
 2016-01-11: 1.5.1
 

doc/upgrading.txt

@@ -396,6 +396,24 @@ parameter. This is expected to be the preferred form in the future.
 You do not need to use the ``ctx`` parameter in the function if you do
 not need it.
 
+Improve query editing
+---------------------
+
+The query editing interface will now report an error if the user
+creates a new query that they are already using. By default the query
+editing page (issue.search.html) displays the index page when the
+search is triggered. Since the user expects to see the results of the
+query, this is usually the right thing. But now that the code properly
+checks for duplicate search names, the user should stay on the search
+page if there is an error. To add this to your existing
+index.search.html page, add the following line after the hidden field
+@old-queryname:
+
+  <input type="hidden" name="@template" value="index|search"/>
+
+With this addition, the index template is displayed if there is no
+error, and the user stays on the search template if there is an error.
+
 Migrating from 1.5.0 to 1.5.1
 =============================
 

roundup/cgi/actions.py

@@ -299,6 +299,8 @@ def handle(self):
             key = self.db.query.getkey()
             if key:
                 # edit the old way, only one query per name
+                # Note that use of queryname as key will automatically
+                # raise an error if there are duplicate names.
                 try:
                     qid = self.db.query.lookup(old_queryname)
                     if not self.hasPermission('Edit', 'query', itemid=qid):
@@ -313,9 +315,29 @@ def handle(self):
                     qid = self.db.query.create(name=queryname,
                         klass=self.classname, url=url)
             else:
+                uid = self.db.getuid()
+
+                # if the queryname is being changed from the old
+                # (original) value, make sure new queryname is not
+                # already in use by user.
+                # if in use, return to edit/search screen and let
+                # user change it.
+
+                if old_queryname != queryname:
+                    # we have a name change
+                    qids = self.db.query.filter(None, {'name': queryname,
+                                                       'creator': uid})
+                    for qid in qids:
+                        # require an exact name match
+                        if queryname != self.db.query.get(qid, 'name'):
+                            continue
+                        # whoops we found a duplicate; report error and return
+                        message=_("You already own a query named '%s'. Please choose another name.")%(queryname)
+                        self.client.add_error_message(message)
+                        return
+
                 # edit the new way, query name not a key any more
                 # see if we match an existing private query
-                uid = self.db.getuid()
                 qids = self.db.query.filter(None, {'name': old_queryname,
                         'private_for': uid})
                 if not qids:
@@ -351,6 +373,16 @@ def handle(self):
             # commit the query change to the database
             self.db.commit()
 
+            # This redirects to the index page. Add the @dispname
+            # url param to the request so that the query name
+            # is displayed.
+            req.form.list.append(
+                cgi.MiniFieldStorage(
+                    "@dispname", queryname
+                )
+            )
+
+
     def fakeFilterVars(self):
         """Add a faked :filter form variable for each filtering prop."""
         cls = self.db.classes[self.classname]

roundup/cgi/templating.py

@@ -2780,12 +2780,21 @@ def add(k, v):
 
     def indexargs_url(self, url, args):
         """ Embed the current index args in a URL
+
+            If the value of an arg (in args dict) is None,
+            the argument is excluded from the url. If you want
+            an empty value use an empty string '' as the value.
+            Use this in templates to conditionally
+            include an arg if it is set to a value. E.G.
+            {..., '@queryname': request.dispname or None, ...}
+            will include @queryname in the url if there is a
+            dispname otherwise the parameter will be omitted
+            from the url.
         """
         q = urllib.quote
         sc = self.special_char
         l = ['%s=%s'%(k,isinstance(v, basestring) and q(v) or v)
-                for k,v in args.items()]
-
+             for k,v in args.items() if v != None ]
         # pull out the special values (prefixed by @ or :)
         specials = {}
         for key in args.keys():

share/roundup/templates/classic/html/issue.search.html

@@ -203,6 +203,7 @@
  <td tal:define="value request/form/@queryname/value | nothing">
   <input name="@queryname" tal:attributes="value value">
   <input type="hidden" name="@old-queryname" tal:attributes="value value">
+  <input type="hidden" name="@template" value="index|search">
  </td>
 </tr>
 

share/roundup/templates/devel/html/bug.search.html

@@ -280,6 +280,7 @@
  <td tal:define="value request/form/@queryname/value | nothing">
   <input name="@queryname" tal:attributes="value value"/>
   <input type="hidden" name="@old-queryname" tal:attributes="value value"/>
+  <input type="hidden" name="@template" value="index|search"/>
  </td>
 </tr>
 

share/roundup/templates/devel/html/task.search.html

@@ -280,6 +280,7 @@
  <td tal:define="value request/form/@queryname/value | nothing">
   <input name="@queryname" tal:attributes="value value"/>
   <input type="hidden" name="@old-queryname" tal:attributes="value value"/>
+  <input type="hidden" name="@template" value="index|search"/>
  </td>
 </tr>
 

share/roundup/templates/responsive/html/bug.search.html

@@ -280,6 +280,7 @@
  <td tal:define="value request/form/@queryname/value | nothing">
   <input name="@queryname" tal:attributes="value value"/>
   <input type="hidden" name="@old-queryname" tal:attributes="value value"/>
+  <input type="hidden" name="@template" value="index|search"/>
  </td>
 </tr>
 

share/roundup/templates/responsive/html/task.search.html

@@ -241,6 +241,7 @@
  <td tal:define="value request/form/@queryname/value | nothing">
   <input name="@queryname" tal:attributes="value value"/>
   <input type="hidden" name="@old-queryname" tal:attributes="value value"/>
+  <input type="hidden" name="@template" value="index|search"/>
  </td>
 </tr>