Roundup Tracker - Issues

Issue 1429368

classification
multiple query entries do not appear to work
Type: Severity: normal
Components: Web interface Versions:
process
Status: closed fixed
:
: richard : dbyrne, dlinke, richard, rouilj
Priority: normal :

Created on 2006-02-10 20:52 by anonymous, last changed 2006-03-21 12:39 by dbyrne.

Messages
msg2147 Author: [hidden] (anonymous) Date: 2006-02-10 20:52
When I try to create a second private query, I am
unable to.  I've looked at the code and I believe that
the problem is in the cgi/actions.py file.  It looks
like it walks through the current user-created queries
and if it doesn't find one that matches it doesn't
create the new one.

My patch is:
205,212c205,211
<                         self.db.commit()
<                         return
<                 # create a query
<                 if not self.hasPermission('Create',
'query'):
<                     raise exceptions.Unauthorised,
self._(
<                         "You do not have permission
to store queries")
<                 qid =
self.db.query.create(name=queryname,
<                     klass=self.classname, url=url,
private_for=uid)
---
>                 else:
>                     # create a query
>                     if not
self.hasPermission('Create', 'query'):
>                         raise
exceptions.Unauthorised, self._(
>                             "You do not have
permission to store queries")
>                     qid =
self.db.query.create(name=queryname,
>                         klass=self.classname,
url=url, private_for=uid)


What I am doing is if I find a match, I'm renaming like
before, committing, and returning from the routine.  If
I don't find it or there were any qids anyway, I'm
adding the new qid.

My email address is:
David.Byrne@cambridge-na.com

Thanks,
David Byrne
msg2148 Author: [hidden] (rouilj) Date: 2006-02-11 17:59
Logged In: YES 
user_id=707416

I can confirm this in the 1.1.0 release demo tracker.

To reproduce:
   start up demo tracker.
   log in as admin/admin
   click on search
   enter q1 in the query name box, hit return
   see q1 in the your queries area.
   click on search
   enter q2 in the query name box, hit return
   do not see q2 in the your queries area
   do not see query in query edit window.

Also David, when submitting patches it is better to use uni-diff
format (-u flag) or context diff (-c flag) since these
include the
filenames and enough info for patch to be able to
automatically apply
the diff even if the line numbers are different (say to an
updated
verion of the file).

Also you should attach the patch if possible because
sourceforge's
text box reformats and screws up the patches.

From the info in the patch I can't find the location for
your patch to
apply in 1.1.0 cgi/actions.py, however I do see a bug:

starting at line 183 (reindented):

else:
  # 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:
      # ok, so there's not a private query for the current user
      # - see if there's one created by them
      qids = self.db.query.filter(None, {'name': old_queryname,
	  'creator': uid})

  if qids:  # line 195
      # edit query - make sure we get an exact match on the name
      for qid in qids:
	  if old_queryname != self.db.query.get(qid, 'name'):
	      continue
	  if not self.hasPermission('Edit', 'query', itemid=qid):
	      raise exceptions.Unauthorised, self._(
	      "You do not have permission to edit queries")
	  self.db.query.set(qid, klass=self.classname,
	      url=url, name=queryname)
       [what happens if there are private/public queries but
no exact match?]
  else:
      # create a query
      if not self.hasPermission('Create', 'query'):
	  raise exceptions.Unauthorised, self._(
	      "You do not have permission to store queries")
      qid = self.db.query.create(name=queryname,
	  klass=self.classname, url=url, private_for=uid)

Where my comment is seems to be a problem. The way around it
would be
to set a handled flag in the "for qid" loop (line 197) if an
exact
match happened and then create the query if the flag is not set.

-- rouilj
msg2149 Author: [hidden] (dbyrne) Date: 2006-02-11 20:57
Logged In: YES 
user_id=53454

Yes, that is where I had found the issue.  There are a few
ways to handle it.  I chose to commit the change and return.
 Then move the else part out to catch the fall through.  The
else creates the new qid entry.
msg2150 Author: [hidden] (dlinke) Date: 2006-03-01 18:22
Logged In: YES 
user_id=734219

May this bug be the same as "[1436169 ] Searches do not save"?

You could test my patch (just a one-line-change) from there
and see if it solves this one too.

Regards,
David
msg2151 Author: [hidden] (dbyrne) Date: 2006-03-21 12:39
Logged In: YES 
user_id=53454

I have a slight problem with the one-line change.  If you
look at the logic (and comment), you have an unhandled case.
 Basically, the comment at line 196 states "# edit query -
make sure we get an exact match on the name".  What happens
if we don't have an exact match on the name?  Right now it
does nothing.  So if the form has an old_queryname, but it
does not match any current name, this code does nothing.  It
does not print an error, or save a query.  It falls through
to the "add it to the user's query multilink".  Now this
error condition MAY not be possible to create, but it
appears that there still is an error in the logic here.  So
the one-line change should fix the general problem but does
not totally eliminate the bug.
History
Date User Action Args
2006-02-10 20:52:05anonymouscreate