Edgewall Software
Modify

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#12704 closed defect (fixed)

TypeError: unicode argument expected, got 'str'

Reported by: anonymous Owned by: Ryan J Ollos
Priority: normal Milestone: 1.3.2
Component: report system Version: 1.3dev
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Fixed TypeError when report is saved with an empty Query.

API Changes:

Report.insert and Report.update raise a TracError when query is empty (1.2.1).

Internal Changes:

Description

Creating a new report with an empty query field:

Traceback (most recent call last):
  File "/trac/trunk/trac/web/main.py", line 630, in _dispatch_request
    dispatcher.dispatch(req)
  File "/trac/trunk/trac/web/main.py", line 252, in dispatch
    resp = chosen_handler.process_request(req)
  File "/trac/trunk/trac/ticket/report.py", line 190, in process_request
    template, data, content_type = self._render_view(req, id)
  File "/trac/trunk/trac/ticket/report.py", line 422, in _render_view
    res = self.execute_paginated_report(req, id, sql, args, limit, offset)
  File "/trac/trunk/trac/ticket/report.py", line 659, in execute_paginated_report
    sql, args, missing_args = self.sql_sub_vars(sql, args)
  File "/trac/trunk/trac/ticket/report.py", line 846, in sql_sub_vars
    sql_io.write(sub_vars_re.sub(repl, expr))
TypeError: unicode argument expected, got 'str'

Attachments (1)

Screen Shot 2017-02-27 at 22.57.27.png (42.1 KB ) - added by Ryan J Ollos 7 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 by Jun Omae, 7 years ago

Milestone: 1.3.2
Version: 1.3dev

comment:2 by Ryan J Ollos, 7 years ago

Owner: set to Ryan J Ollos
Status: newassigned

This appears to be due to changes in #12046. It seems to be fixed with either of these patches:

  • trac/ticket/model.py

    diff --git a/trac/ticket/model.py b/trac/ticket/model.py
    index 9cec28d12..21ceb87e3 100644
    a b class Report(object):  
    11671167                        WHERE id=%s
    11681168                        """, (id_as_int,)):
    11691169                    self.id = id_as_int
    1170                     self.title = title or ''
    1171                     self.description = description or ''
    1172                     self.query = query or ''
     1170                    self.title = title or u''
     1171                    self.description = description or u''
     1172                    self.query = query or u''
    11731173                    return
    11741174            raise ResourceNotFound(_("Report {%(num)s} does not exist.",
    11751175                                     num=id), _("Invalid Report Number"))
  • trac/ticket/report.py

    diff --git a/trac/ticket/report.py b/trac/ticket/report.py
    index 12e7bf9c1..c3ebc1145 100644
    a b class ReportModule(Component):  
    843843                if expr.startswith("'"):
    844844                    sql_io.write(repl_literal(expr, db))
    845845                else:
    846                     sql_io.write(sub_vars_re.sub(repl, expr))
     846                    sql_io.write(sub_vars_re.sub(repl, unicode(expr)))
    847847
    848848        # Remove arguments that don't appear in the SQL query
    849849        for name in set(args) - names:

Is one of those preferred, or is there an even better fix? I'll add a unit test before committing.

comment:3 by Ryan J Ollos, 7 years ago

Release Notes: modified (diff)

by Ryan J Ollos, 7 years ago

comment:4 by Ryan J Ollos, 7 years ago

Fixed in r15593, however I think we should preventing allowing to create a report with empty SQL query.

comment:5 by Jun Omae, 7 years ago

When no SQL query error occurs, Edit and Delete buttons would be invisible. The buttons should be visible.

Last edited 7 years ago by Jun Omae (previous) (diff)

in reply to:  4 comment:6 by Ryan J Ollos, 7 years ago

Replying to Ryan J Ollos:

Fixed in r15593, however I think we should preventing allowing to create a report with empty SQL query.

Simplest way to avoid empty query would be:

  • trac/ticket/model.py

    diff --git a/trac/ticket/model.py b/trac/ticket/model.py
    index 7f65e5c03..2c9d39330 100644
    a b class Report(object):  
    11891189    def insert(self):
    11901190        """Insert a new report."""
    11911191        assert not self.exists, "Cannot insert existing report"
     1192        if not self.query:
     1193            raise TracError(_("Query cannot be empty."))
    11921194
    11931195        self.env.log.debug("Creating new report '%s'", self.id)
    11941196        with self.env.db_transaction as db:
    class Report(object):  
    11991201            self.id = db.get_last_id(cursor, 'report')
    12001202
    12011203    def update(self):
     1204        if not self.query:
     1205            raise TracError(_("Query cannot be empty."))
    12021206        self.env.db_transaction("""
    12031207            UPDATE report SET title=%s, query=%s, description=%s
    12041208            WHERE id=%s

I think it would be good to add a warning instead, but we should make those changes throughout TRac. For example, when adding a milestone with empty name.

in reply to:  5 ; comment:7 by Ryan J Ollos, 7 years ago

Replying to Jun Omae:

When no SQL query error occurs, Edit and Delete buttons would be invisible. The buttons should be visible.

Agreed, but that is unrelated to the regression and 1.2-stable has same issue. We can avoid by preventing saving a report with an empty query.

in reply to:  7 ; comment:8 by Jun Omae, 7 years ago

Replying to Ryan J Ollos:

Replying to Jun Omae:

When no SQL query error occurs, Edit and Delete buttons would be invisible. The buttons should be visible.

Agreed, but that is unrelated to the regression and 1.2-stable has same issue.

Yeah. I'll create new ticket for invisible buttons.

We can avoid by preventing saving a report with an empty query.

Minor thing, we cannot avoid empty saved query before the fix. I manually added ?action=edit to URL.

Version 0, edited 7 years ago by Jun Omae (next)

in reply to:  8 comment:9 by Ryan J Ollos, 7 years ago

Replying to Jun Omae:

We can avoid by preventing saving a report with an empty query.

Minor thing, we cannot avoid empty saved query before the fix. I manually added ?action=edit to URL.

Good point. I think it's a good idea to show those buttons on the page even when an error is raised.

comment:10 by Ryan J Ollos, 7 years ago

API Changes: modified (diff)
Resolution: fixed
Status: assignedclosed

comment:6 changes committed to 1.2-stable in r15598, merged to trunk in r15599.

comment:11 by Ryan J Ollos, 7 years ago

Release Notes: modified (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.