Recent changes drop support of $USER from Gantt invocation

[Chris Nelson]

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.)

[Chris Nelson]

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

[Chris Nelson]

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() 

[Chris Nelson]

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

[Chris Nelson]

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.

[Chris Nelson]

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()?

[Chris Nelson]

Replying to 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()?

No, that doesn't do it.

[falkb]

Probably logging can tell after adding

[Chris Nelson]

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]

[Chris Nelson]

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

        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?

[Chris Nelson]

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?

[Chris Nelson]

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.

[Chris Nelson]

(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.

[falkb]

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

[Chris Nelson]

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.