Modify

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#10696 closed defect (fixed)

Recent changes drop support of $USER from Gantt invocation

Reported by: Chris Nelson Owned by: Chris Nelson
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 (0)

Change History (15)

comment:1 Changed 4 years ago by Chris Nelson

Status: newassigned

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

comment:2 Changed 4 years ago by 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() 

comment:3 Changed 4 years ago by 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

comment:4 in reply to:  3 Changed 4 years ago by 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.

comment:5 Changed 4 years ago by 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()?

comment:6 in reply to:  5 Changed 4 years ago by 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.

comment:7 Changed 4 years ago by falkb

Probably logging can tell after adding

comment:8 Changed 4 years ago by 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]

comment:9 in reply to:  8 Changed 4 years ago by 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?

comment:10 Changed 4 years ago by 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?

comment:11 in reply to:  10 Changed 4 years ago by 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.

comment:12 Changed 4 years ago by 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.

comment:13 Changed 4 years ago by falkb

Resolution: fixed
Status: assignedclosed

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

comment:14 in reply to:  13 Changed 4 years ago by 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.

comment:15 Changed 4 years ago by Chris Nelson

Summary: Recent changes drop the ability to add query fields to Gantt invocationRecent changes drop support of $USER from Gantt invocation

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Chris Nelson.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.