Edgewall Software
Modify

Opened 10 years ago

Closed 6 years ago

#9311 closed enhancement (fixed)

MyTickets report fails on non authenticated anonymous users who have their email set in the preferences

Reported by: Carsten Klein <carsten.klein@…> Owned by: Jun Omae
Priority: high Milestone: 1.0.2
Component: report system Version: 0.12dev
Severity: normal Keywords: user
Cc: Thijs Triemstra, mpotter@…, ethan.jucovy@… Branch:
Release Notes:

Fix a ProgrammingError due to DISTINCT and ORDER BY while executing "My Tickets" report on PostgreSQL

API Changes:

Description

*please* make this work.

I will provide a patch if possible.

Attachments (1)

mytickets result.jpg (130.4 KB ) - added by Carsten Klein <carsten.klein@…> 10 years ago.

Download all attachments as: .zip

Change History (37)

comment:1 by Carsten Klein <carsten.klein@…>, 10 years ago

I see now that the MyTickets report will only return those tickets for which one is the registered owner.

Perhaps extending this to also allowing it to search for reporter = $USER would do the trick?

comment:2 by Remy Blank, 10 years ago

Keywords: needinfo added

Could you please be more precise? The "My Tickets" report seems to work on this site, even if I log out and set an e-mail address in the preferences. What are you seeing, and what did you expect?

comment:3 by Carsten Klein <carsten.klein@…>, 10 years ago

on t.e.o. i see, with configured email address and name in the preferences (see attachment).

by Carsten Klein <carsten.klein@…>, 10 years ago

Attachment: mytickets result.jpg added

comment:4 by Carsten Klein <carsten.klein@…>, 10 years ago

What I would expect are at least those for which I am the reporter and not the responsible person (or assigned person) for.

Ideally it would also return those tickets on which I commented upon, but this is optional.

in reply to:  4 comment:5 by Carsten Klein <carsten.klein@…>, 10 years ago

Replying to Carsten Klein <carsten.klein@…>:

[…] and not _only_ the responsible person […]

comment:6 by Remy Blank, 10 years ago

Keywords: needinfo removed
Milestone: unscheduled

Ok, understood. I guess the "My tickets" report was originally designed for listing the tickets of which someone is the owner. Adding the tickets reported by the current user would make sense. However, adding those on which the current user has commented would probably add too much noise. We could add another report for that, though.

So yes, if you can come up with the SQL queries for these two, we could fix that.

comment:7 by Carsten Klein <carsten.klein@…>, 10 years ago

You asked for it:

SELECT p.value AS __color__,
   (CASE status WHEN 'accepted' THEN 'Accepted' ELSE 'Owned' END) AS __group__,
   id AS ticket, summary, component, version, milestone,
   t.type AS type, priority, time AS created,
   changetime AS _changetime, description AS _description,
   reporter AS _reporter
  FROM ticket t
  LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
  WHERE t.status <> 'closed' AND ( owner = $USER or reporter = $USER )
  ORDER BY (status = 'accepted') DESC, CAST(p.value AS int), milestone, t.type, time

comment:8 by Carsten Klein <carsten.klein@…>, 10 years ago

And here is the second query which also includes tickets where the user commented upon:

SELECT DISTINCT p.value AS __color__,
   (CASE status WHEN 'accepted' THEN 'Accepted' ELSE 'Owned' END) AS __group__,
   id AS ticket, summary, component, version, milestone,
   t.type AS type, priority, t.time AS created,
   changetime AS _changetime, description AS _description,
   reporter AS _reporter
  FROM ticket t
  LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
  LEFT JOIN ticket_change tc ON tc.ticket = t.id
  WHERE t.status <> 'closed' AND ( owner = $USER or reporter = $USER or author = $USER )
  ORDER BY (status = 'accepted') DESC, CAST(p.value AS int), milestone, t.type, t.time

comment:9 by Thijs Triemstra, 9 years ago

Cc: Thijs Triemstra added
Keywords: review added

Queries need a review.

comment:10 by Remy Blank, 9 years ago

Milestone: unscheduled0.13
Owner: set to Remy Blank
Priority: normalhigh

Will do.

comment:11 by Remy Blank, 9 years ago

Good start. How about adding groups for "reported" and "commented"?

SELECT DISTINCT p.value AS __color__,
       (CASE
         WHEN owner = $USER AND status = 'accepted' THEN 'Accepted'
         WHEN owner = $USER THEN 'Owned'
         WHEN reporter = $USER THEN 'Reported'
         ELSE 'Commented' END) AS __group__,
       t.id AS ticket, summary, component, version, milestone,
       t.type AS type, priority, t.time AS created,
       t.changetime AS _changetime, description AS _description,
       reporter AS _reporter
  FROM ticket t
  LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
  LEFT JOIN ticket_change tc ON tc.ticket = t.id AND tc.author = $USER
                                AND tc.field = 'comment'
  WHERE t.status <> 'closed'
        AND (owner = $USER OR reporter = $USER OR author = $USER)
  ORDER BY (owner = $USER AND status = 'accepted') DESC,
           owner = $USER DESC, reporter = $USER DESC,
           CAST(p.value AS int), milestone, t.type, t.time

Let me know if this does what you wanted.

comment:12 by Remy Blank, 9 years ago

Resolution: fixed
Status: newclosed

Improved "My Tickets" report committed in [10385].

comment:13 by Christian Boos, 9 years ago

Resolution: fixed
Status: closedreopened

I wanted to test r10385 here, but I got:

ProgrammingError: for SELECT DISTINCT, ORDER BY expressions must appear in select list

The following "fix" worked:

SELECT DISTINCT
        CAST(p.value AS int) AS __color__,
       (CASE
         WHEN owner = $USER AND status = 'accepted' THEN 'Accepted'
         WHEN owner = $USER THEN 'Owned'
         WHEN reporter = $USER THEN 'Reported'
         ELSE 'Commented' END) AS __group__,
       t.id AS ticket, summary, component, version, milestone,
       t.type AS type, priority, t.time AS created,
       t.changetime AS _changetime, description AS _description,
       reporter AS _reporter,
       (owner = $USER AND status = 'accepted') AS __exp1,
       (owner = $USER) as __exp2,
       (reporter = $USER) as __exp3
  FROM ticket t
  LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
  LEFT JOIN ticket_change tc ON tc.ticket = t.id AND tc.author = $USER
                                AND tc.field = 'comment'
  WHERE t.status <> 'closed'
        AND (owner = $USER OR reporter = $USER OR author = $USER)
  ORDER BY (owner = $USER AND status = 'accepted') DESC,
           owner = $USER DESC, reporter = $USER DESC,
           CAST(p.value AS int),
            milestone, t.type, t.time

But then the groups are not contiguous. What about this instead:

SELECT DISTINCT
        CAST(p.value AS int) AS __color__,
       (CASE
         WHEN owner = $USER AND status = 'accepted' THEN 'Accepted'
         WHEN owner = $USER THEN 'Owned'
         WHEN reporter = $USER THEN 'Reported'
         ELSE 'Commented' END) AS __group__,
       t.id AS ticket, summary, component, version, milestone,
       t.type AS type, priority, t.time AS created,
       t.changetime AS _changetime, description AS _description,
       reporter AS _reporter
  FROM ticket t
  LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
  LEFT JOIN ticket_change tc ON tc.ticket = t.id AND tc.author = $USER
                                AND tc.field = 'comment'
  WHERE t.status <> 'closed'
        AND (owner = $USER OR reporter = $USER OR author = $USER)
  ORDER BY __group__, __color__, milestone, t.type, t.time

Or even better, to have the group order Accepted/Owned/Reported/Commented:

SELECT  __color__, __group,
       (CASE
         WHEN __group = 1 THEN 'Accepted'
         WHEN __group = 2 THEN 'Owned'
         WHEN __group = 3 THEN 'Reported'
         ELSE 'Commented' END) AS __group__,
       ticket, summary, component, version, milestone,
       type, priority, created, _changetime, _description,
       _reporter
FROM (
 SELECT DISTINCT CAST(p.value AS int) AS __color__,
      (CASE
         WHEN owner = $USER AND status = 'accepted' THEN 1
         WHEN owner = $USER THEN 2
         WHEN reporter = $USER THEN 3
         ELSE 4 END) AS __group,
       t.id AS ticket, summary, component, version, milestone,
       t.type AS type, priority, t.time AS created,
       t.changetime AS _changetime, description AS _description,
       reporter AS _reporter
  FROM ticket t
  LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
  LEFT JOIN ticket_change tc ON tc.ticket = t.id AND tc.author = $USER
                                AND tc.field = 'comment'
  WHERE t.status <> 'closed'
        AND (owner = $USER OR reporter = $USER OR author = $USER)
) AS sub
ORDER BY __group, __color__, milestone, type, created

VERIFY However, it remains to be seen if one of the above would work with MySQL ;-) (they both do with SQLite).

Last edited 8 years ago by Christian Boos (previous) (diff)

comment:14 by Christian Boos, 9 years ago

TODO On a "different" topic, actually back to the original request, it seems that $USER is still replaced by anonymous when not logged in, not with the name/e-mail set in the preferences. It should be possible to fix also that…

Last edited 8 years ago by Christian Boos (previous) (diff)

in reply to:  13 ; comment:15 by Remy Blank, 9 years ago

Replying to cboos:

I wanted to test r10385 here, but I got:

ProgrammingError: for SELECT DISTINCT, ORDER BY expressions must appear in select list

Which database backend is this? SQLite worked fine here, maybe it's a question of version. But I didn't test on the other backends. Feel free to commit your fixes.

The discussion did drift a bit starting from comment:4, so the initial issue could indeed be fixed as well.

in reply to:  15 comment:16 by Christian Boos, 9 years ago

Replying to rblank:

Replying to cboos:

I wanted to test r10385 here, but I got:

ProgrammingError: for SELECT DISTINCT, ORDER BY expressions must appear in select list

Which database backend is this?

Sorry, I should have been more precise: here == t.e.o ;-) See {12}. So this was with PostgreSQL 8.3.

Ok, I'll commit the changes once I've tested them with MySQL.

comment:17 by Remy Blank, 9 years ago

Owner: changed from Remy Blank to Christian Boos
Status: reopenednew

Reassigning as per comment:16.

comment:18 by anonymous, 8 years ago

It looks good now, but returns too many results for the non authenticated user with email address set in the preferences as it will also return results where there is simply the anonymous user, i.e. a user with no email having been set…

comment:19 by Christian Boos, 8 years ago

#10677 was closed as duplicate for the problem discussed in comment:13.

in reply to:  18 comment:20 by Christian Boos, 8 years ago

Replying to anonymous:

It looks good now, but returns too many results for the non authenticated user with email address set in the preferences as it will also return results where there is simply the anonymous user, i.e. a user with no email having been set…

See comment:14.

[OT] Experimenting with WikiPhrases to highlight the remaining open points in long tickets (as proposed in ticket:525#comment:113)

comment:21 by Mark Potter <mpotter@…>, 8 years ago

Cc: mpotter@… added

Coming late to the discussion of this ticket, so I may be off base; apologies in advance if I am.

It appears to me that we are blurring the purpose of the "My Tickets" report. I view the original purpose of "My Tickets" as:

As a developer, what are the tickets that I need to work on?
i.e. Tickets where owner is the current user.
i.e. My tickets.

Most of the discussion appears to be adding:

As a user, what are the ticket that are of interest to me?
i.e Tickets that the current user reported, is watching (in the CC), or commented on). (see also #8559).

I find these two purposes to be distinctly different and thus should be different reports.

comment:22 by Christian Boos, 8 years ago

Aren't the proposed categories enough to address both use cases?

  • as a user, you'd get the "Reported" and "Commented" tickets,
  • as a developer, you'll get (in addition) the "Owned" and "Accepted" tickets, and they'll appear first

I'm not sure whether you'd have in "Commented" the tickets where you simply added yourself to the CC:, but if not we could have a fifth category "On CC:".

Of course, the alternative to have more specialized reports for different roles is also a valid approach, but I think the idea of this ticket was just to make the default "My Tickets" report more generally useful.

in reply to:  22 ; comment:23 by Mark Potter <mpotter@…>, 8 years ago

Replying to cboos:

Aren't the proposed categories enough to address both use cases?

Yes, but it starts becoming "a jack of all trades but master of none". Of the top of my head:

  • As a developer:
    • I do not want to see Closed; I'm done with those.
    • I would want to see Accepted, then Owned, then CC'ed, then Reported, then Commented. If I cared enough to CC a ticket it's more important than those I only reported or commented on.
  • As a user:
    • I would want to see Closed tickets, as yet another category. (What's the status of my tickets of interest? Where did my issue go?)
    • I would want to see Reported, then CC'ed, then Commented, then Deleted, then Accepted, then Owned.
      • A normal user would not have any Accepted or Owned, so them coming first is a not an issue.
      • But what about a dual user looking at the tickets as a user. ("What's the status of my tickets of interest?" vs. as a developer, "What should I be working on?") Obviously the order would be non-ideal, but also a ticket could end up in the wrong grouping (e.g. Developer vs User: Owned vs. Reported, Owned vs. Commented, Owned vs. CC, or CC vs. Reported).

I'm not sure whether you'd have in "Commented" the tickets where you simply added yourself to the CC:, but if not we could have a fifth category "On CC:".

No, there is no comment when you only add yourself to the CC. But not, was added to the CC, but is in the CC; thus directly test the ticket cc field. See #8559.

Of course, the alternative to have more specialized reports for different roles is also a valid approach,

I would think that best.

but I think the idea of this ticket was just to make the default "My Tickets" report more generally useful.

To the original request of making $USER work for non authenticated users, I would also like to see a $USER dynamic variable UI, for manager type functions. (I don't want to see my tickets, I want to see arthur.dent's tickets.) But that should be another ticket.

in reply to:  23 ; comment:24 by Christian Boos, 8 years ago

Replying to Mark Potter <mpotter@…>:

To the original request of making $USER work for non authenticated users, I would also like to see a $USER dynamic variable UI, for manager type functions. (I don't want to see my tickets, I want to see arthur.dent's tickets.) But that should be another ticket.

I'll answer to the rest later, but for this one, you know that you can already specify USER in the URL? We could perhaps also make it part of the UI, but for now it's excluded on purpose (don't remember exactly why though, see #7293 for details).

in reply to:  24 comment:25 by Mark Potter <mpotter@…>, 8 years ago

Replying to cboos:

I'll answer to the rest later, but for this one, you know that you can already specify USER in the URL?

Yes, that is what I do, thanks.

comment:26 by Remy Blank, 7 years ago

Milestone: 1.01.0-triage

Preparing for 1.0.

comment:27 by Christian Boos, 7 years ago

Component: generalreport system
Keywords: user added; review removed

Moving enhancements to dev.

comment:28 by Remy Blank, 7 years ago

Milestone: next-stable-1.0.xnext-dev-1.1.x

I assume you meant to set the milestone.

comment:29 by ethan.jucovy@…, 7 years ago

Cc: ethan.jucovy@… added

in reply to:  13 ; comment:30 by Ethan Jucovy <ethan.jucovy@…>, 7 years ago

Replying to cboos:

Or even better, to have the group order Accepted/Owned/Reported/Commented:

SELECT  __color__, __group,
       (CASE
         WHEN __group = 1 THEN 'Accepted'
         WHEN __group = 2 THEN 'Owned'
         WHEN __group = 3 THEN 'Reported'
         ELSE 'Commented' END) AS __group__,
       ticket, summary, component, version, milestone,
       type, priority, created, _changetime, _description,
       _reporter
FROM (
 SELECT DISTINCT CAST(p.value AS int) AS __color__,
      (CASE
         WHEN owner = $USER AND status = 'accepted' THEN 1
         WHEN owner = $USER THEN 2
         WHEN reporter = $USER THEN 3
         ELSE 4 END) AS __group,
       t.id AS ticket, summary, component, version, milestone,
       t.type AS type, priority, t.time AS created,
       t.changetime AS _changetime, description AS _description,
       reporter AS _reporter
  FROM ticket t
  LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
  LEFT JOIN ticket_change tc ON tc.ticket = t.id AND tc.author = $USER
                                AND tc.field = 'comment'
  WHERE t.status <> 'closed'
        AND (owner = $USER OR reporter = $USER OR author = $USER)
) AS sub
ORDER BY __group, __color__, milestone, type, created

VERIFY However, it remains to be seen if one of the above would work with MySQL ;-) (they both do with SQLite).

I just tested the above version of Report 7 with a MySQL backend. It fails:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'INT) AS __color__,\r\n (CASE\r\n WHEN (owner = 'ethan' AND status = 'ac' at line 12

The problem is DISTINCT CAST(p.value AS int) AS __color__. For MySQL, it needs to be DISTINCT CAST(p.value AS decimal) AS __color__ — casting to "decimal" instead of "int". I just tested on postgres and sqlite, and using DISTINCT CAST(p.value AS decimal) __color__ seems to work properly on all of the database backends.

So, this version of the report works well on postgres 8.4, mysql 5.1.63-0ubuntu0.11.04.1, and sqlite 3.6.22:

SELECT  __color__, __group,
       (CASE
         WHEN __group = 1 THEN 'Accepted'
         WHEN __group = 2 THEN 'Owned'
         WHEN __group = 3 THEN 'Reported'
         ELSE 'Commented' END) AS __group__,
       ticket, summary, component, version, milestone,
       type, priority, created, _changetime, _description,
       _reporter
FROM (
 SELECT DISTINCT CAST(p.value AS decimal) AS __color__,
      (CASE
         WHEN owner = $USER AND status = 'accepted' THEN 1
         WHEN owner = $USER THEN 2
         WHEN reporter = $USER THEN 3
         ELSE 4 END) AS __group,
       t.id AS ticket, summary, component, version, milestone,
       t.type AS type, priority, t.time AS created,
       t.changetime AS _changetime, description AS _description,
       reporter AS _reporter
  FROM ticket t
  LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
  LEFT JOIN ticket_change tc ON tc.ticket = t.id AND tc.author = $USER
                                AND tc.field = 'comment'
  WHERE t.status <> 'closed'
        AND (owner = $USER OR reporter = $USER OR author = $USER)
) AS sub
ORDER BY __group, __color__, milestone, type, created

in reply to:  30 comment:31 by anonymous, 7 years ago

Replying to Ethan Jucovy <ethan.jucovy@…>:

Backing this up with a link to the offical documentation:

http://dev.mysql.com/doc/refman/5.1/de/cast-functions.html

There is no type INTeger to which you can cast to, except for UNSIGNED (INTEGER).

+1 by me.

comment:32 by Jun Omae, 6 years ago

#11480 was closed as duplicated.

It would be simple to use EXISTS (SELECT * FROM ticket_change ...) rather than DISTINCT ... LEFT JOIN ticket_change .... I verified with SQLite 3.3.6, PostgreSQL 8.1.23 and MySQL 5.0.95. If using MySQL, replace with CAST(p.value AS signed).

SELECT p.value AS __color__,
       (CASE
         WHEN owner = $USER AND status = 'accepted' THEN 'Accepted'
         WHEN owner = $USER THEN 'Owned'
         WHEN reporter = $USER THEN 'Reported'
         ELSE 'Commented' END) AS __group__,
       t.id AS ticket, summary, component, version, milestone,
       t.type AS type, priority, t.time AS created,
       t.changetime AS _changetime, description AS _description,
       reporter AS _reporter
  FROM ticket t
  LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
  WHERE t.status <> 'closed' AND
        (owner = $USER OR reporter = $USER OR
         EXISTS (SELECT * FROM ticket_change tc
                 WHERE tc.ticket = t.id AND tc.author = $USER AND
                       tc.field = 'comment'))
  ORDER BY (COALESCE(owner, '') = $USER AND status = 'accepted') DESC,
           COALESCE(owner, '') = $USER DESC,
           COALESCE(reporter, '') = $USER DESC,
           CAST(p.value AS integer), milestone, t.type, t.time
Last edited 6 years ago by Jun Omae (previous) (diff)

comment:33 by Jun Omae, 6 years ago

Milestone: next-dev-1.1.x1.0.2
Owner: changed from Christian Boos to Jun Omae
Status: newassigned

Proposed fix for ProgrammingError on PostgreSQL with unit tests for reports can be found in jomae.git@t9311. I've verified with SQLite 3.3.6, PostgreSQL 8.1.23 and MySQL 5.0.95.

comment:34 by Jun Omae, 6 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed in [12533] and merged to trunk in [12534].

Also, the DISTINCT and LEFT JOIN ticket_change in report:12 has been changed to EXISTS (SELECT * FROM ticket_change ...).

comment:35 by Jun Omae, 6 years ago

Resolution: fixed
Status: closedreopened

Oops. The following failure cause of milliseconds resolution time on Windows.

======================================================================
ERROR: test_report_7_my_tickets (__main__.ExecuteReportTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "trac\ticket\tests\report.py", line 452, in test_report_7_my_tickets
    comment='from anonymous')
  File "trac\ticket\tests\report.py", line 145, in _save_ticket
    return ticket.save_changes(author=author, comment=comment, when=when)
  File "C:\usr\src\trac\trac.git\trac\ticket\model.py", line 358, in save_changes
    """, (self.id, when_ts, author, cnum, comment))
  File "C:\usr\src\trac\trac.git\trac\db\util.py", line 121, in execute
    cursor.execute(query, params)
  File "C:\usr\src\trac\trac.git\trac\db\util.py", line 54, in execute
    r = self.cursor.execute(sql_escape_percent(sql), args)
  File "C:\usr\src\trac\trac.git\trac\db\sqlite_backend.py", line 78, in execute
    result = PyFormatCursor.execute(self, *args)
  File "C:\usr\src\trac\trac.git\trac\db\sqlite_backend.py", line 56, in execute
    args or [])
  File "C:\usr\src\trac\trac.git\trac\db\sqlite_backend.py", line 48, in _rollback_on_error
    return function(self, *args, **kwargs)
IntegrityError: columns ticket, time, field are not unique
  • trac/ticket/tests/report.py

    diff --git a/trac/ticket/tests/report.py b/trac/ticket/tests/report.py
    index 0cd4280..acb957b 100644
    a b class ExecuteReportTestCase(unittest.TestCase):  
    140140
    141141    def _save_ticket(self, ticket, author=None, comment=None, when=None,
    142142                     **kwargs):
     143        if when is None:
     144            when = ticket['changetime'] + timedelta(microseconds=1)
    143145        for name, value in kwargs.iteritems():
    144146            ticket[name] = value
    145147        return ticket.save_changes(author=author, comment=comment, when=when)

comment:36 by Jun Omae, 6 years ago

Resolution: fixed
Status: reopenedclosed

The patch has been committed in [12535] and merged to trunk in [12536].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Jun Omae 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.