Ticket #10696 (closed defect: fixed)

Opened 5 months ago

Last modified 5 months ago

Recent changes drop support of $USER from Gantt invocation

Reported by: ChrisNelson Assigned to: ChrisNelson
Priority: high Component: TracJsGanttPlugin
Severity: normal Keywords:
Cc: falkb Trac Release:

Description

As noted in 9648, sometime between r2213 and r12330, the ability to include extra query options like:

[[TracJSGanttChart(format=week,order=milestone|type|wbs,res=0,colorBy=priority,owner=$USER&status!=closed)]]

was broken. (I expect it was r12329.)

Attachments

Change History

12/11/12 14:29:04 changed by ChrisNelson

  • status changed from new to assigned.

Yes, it was the query refactoring in r12329 that broke this.

12/11/12 14:37:50 changed by ChrisNelson

The old query logic in tracjsgantt.py was:

    def _query_tickets(self, options):
        query_args = {}
        for key in options.keys():
            if not key in self.options:
                query_args[key] = options[key]

        # Expand (or set) list of IDs to include those specified by PM
        # query meta-options (e.g., root)
        pm_id = self.pm.preQuery(options, self._this_ticket())
        if pm_id != '':
            if 'id' in query_args:
                query_args['id'] += '|' + pm_id
            else:
                query_args['id'] = pm_id

        # Start with values that are always needed
        fields = set([
            'description', 
            'owner', 
            'type', 
            'status', 
            'summary', 
            'milestone', 
            'priorty'])

        # Add configured PM fields
        fields |= set(self.pm.queryFields())

        # Make sure the coloring field is included
        if 'colorBy' in options and options['colorBy'] not in fields:
            fields.add(options['colorBy'])

        # Make the query argument
        query_args['col'] = "|".join(fields)  

        # Construct the querystring. 
        query_string = '&'.join(['%s=%s' % 
                                 (f, unicode(v)) for (f, v) in 
                                 query_args.iteritems()]) 

        # Get the Query Object. 
        query = Query.from_string(self.env, query_string)

        # Get all tickets 
 	rawtickets = query.execute(self.req) 

after r12329, it is:

    def _query_tickets(self, options):
        query_options = {}
        for key in options.keys():
            if not key in self.options:
                query_options[key] = options[key]

        # The fields always needed by the Gantt
        fields = set([
            'description', 
            'owner', 
            'type', 
            'status', 
            'summary', 
            'milestone', 
            'priority'])

        # Make sure the coloring field is included
        if 'colorBy' in options:
            fields.add(str(options['colorBy']))

        rawtickets = self.pm.query(query_options, fields, self._this_ticket())

and TracPM.query() is:

    def query(self, options, fields, ticket=None):
        query_args = {}
        # Copy query args from caller (e.g., q_a['owner'] = 'monty|phred')
        for key in options.keys():
            if key not in [ 'goal', 'root' ]:
                query_args[str(key)] = options[key]
        
        # Expand (or set) list of IDs to include those specified by PM
        # query meta-options (e.g., root)
        pm_ids = self.preQuery(options, ticket)
        if len(pm_ids) != 0:
            if 'id' in query_args:
                query_args['id'] += '|' + '|'.join(pm_ids)
            else:
                query_args['id'] = '|'.join(pm_ids)

        # Default to getting all tickets
        if 'max' not in query_args:
            quary_args['max'] = 0

        # Tell the query what columns to return
        query_args['col'] = "|".join(self.queryFields() | fields)

        # Construct the querystring. 
        query_string = '&'.join(['%s=%s' % 
                                 (str(f), unicode(v)) for (f, v) in 
                                 query_args.iteritems()]) 

        # Get the Query object. 
        query = Query.from_string(self.env, query_string)

        # Get all tickets
 	tickets = query.execute() 

(follow-up: ↓ 4 ) 12/11/12 14:47:45 changed by ChrisNelson

The old code has the query string

owner=$USER&status!=closed&max=999&col=status|totalhours|description|priorty|blockedby|summary|priority|userstart|parents|milestone|owner|estimatedhours|type|blocking|userfinish

the new code has:

owner=$USER&status!=closed&max=999&col=status|totalhours|description|blockedby|summary|priority|userstart|parents|milestone|owner|estimatedhours|type|blocking|userfinish

(in reply to: ↑ 3 ) 12/11/12 14:49:55 changed by ChrisNelson

Replying to ChrisNelson:

The old code has the query string {{{ owner=$USER&status!=closed&max=999&col=status|totalhours|description|priorty|blockedby|summary|priority|userstart|parents|milestone|owner|estimatedhours|type|blocking|userfinish }}} the new code has: {{{ owner=$USER&status!=closed&max=999&col=status|totalhours|description|blockedby|summary|priority|userstart|parents|milestone|owner|estimatedhours|type|blocking|userfinish }}}

Those appear to be the same (except for order) so the context of the Query.from_string() may be the clue. The old one is done in the context of the macro expansion. The new one is done in the TracPM library.

(follow-up: ↓ 6 ) 12/11/12 14:55:13 changed by ChrisNelson

Likely the problem line is:

query = Query.from_string(self.env, query_string)

and the difference is self.env comes from TracPM or the macro expansion. What if I passed an env into TracPM.query()?

(in reply to: ↑ 5 ) 12/11/12 14:58:31 changed by ChrisNelson

Replying to ChrisNelson:

Likely the problem line is: {{{ #!py query = Query.from_string(self.env, query_string) }}} and the difference is self.env comes from TracPM or the macro expansion. What if I passed an env into TracPM.query()?

No, that doesn't do it.

12/11/12 15:10:52 changed by falkb

Probably logging can tell after adding

(follow-up: ↓ 9 ) 12/11/12 15:24:39 changed by ChrisNelson

query.py in Trac has several instances of $USER and a comment that the replacement is done in get_sql().

query.execute() calls self.get_sql() which does

        for k, v in self.constraints.items():
            if req:
                v = [val.replace('$USER', req.authname) for val in v]

(in reply to: ↑ 8 ) 12/11/12 15:32:32 changed by ChrisNelson

Replying to ChrisNelson:

query.py in Trac has several instances of $USER and a comment that the replacement is done in get_sql(). query.execute() calls self.get_sql() which does {{{ #!py for k, v in self.constraints.items(): if req: v = [val.replace('$USER', req.authname) for val in v] }}}

So, if there was no req, there would be no user substitution! Where does req come from?

(follow-up: ↓ 11 ) 12/11/12 15:34:27 changed by ChrisNelson

req is passed into get_sql():

    def get_sql(self, req=None, cached_ids=None):

and query.execute() starts:

    def execute(self, req=None, db=None, cached_ids=None):
        if not db:
            db = self.env.get_db_cnx()
        cursor = db.cursor()

        sql, args = self.get_sql(req, cached_ids)

so I can pass req into execute() but I never did so how did this work before?

(in reply to: ↑ 10 ) 12/11/12 15:35:15 changed by ChrisNelson

Replying to ChrisNelson:

... so I can pass req into execute() but I never did so how did this work before?

Wrong. I did. I need to pass it around in the new structure.

12/11/12 15:42:05 changed by ChrisNelson

(In [12438]) Pass macro req down to TracPM.query(). Refs #10696.

This allow expansion of $USER which restores the functionality of charts like

[[TracJSGanttChart(owner=$USER&status!=closed)]]

which were broken by r12329.

(follow-up: ↓ 14 ) 12/11/12 15:51:24 changed by falkb

  • status changed from assigned to closed.
  • resolution set to fixed.

Yeah, good boy! It works again. Appreciating your support very much, that is near realtime! :-)

(in reply to: ↑ 13 ) 12/11/12 16:03:39 changed by ChrisNelson

Replying to falkb:

Yeah, good boy! It works again. Appreciating your support very much, that is near realtime! :-)

I appreciate your feedback. I always say I can fix a problem I can see. I can't fix "sometimes things don't work." ;-) Having clear reproducible cases and your tickets in importable CSV files is a big help in resolving the issues. Thanks.

12/11/12 16:53:22 changed by ChrisNelson

  • summary changed from Recent changes drop the ability to add query fields to Gantt invocation to Recent changes drop support of $USER from Gantt invocation.

Add/Change #10696 (Recent changes drop support of $USER from Gantt invocation)




Change Properties
Action