Opened 13 years ago
Closed 13 years ago
#236 closed enhancement (fixed)
Patch for ticket update bug, and some other minor enhancements
Reported by: | dmcr@… | Owned by: | bas |
---|---|---|---|
Priority: | major | Milestone: | New email2trac release 2.4.0 |
Component: | email2trac | Version: | 2.0.2 |
Keywords: | Cc: |
Description
Hi Bas,
The attached patch fixes a bug when updating a ticket, in the case where the user submitting the update has Trac permission TICKET_MODIFY, but not TICKET_APPEND or TICKET_CHGPROP. This causes the ticket update to fail.
Since Trac documentation states that TICKET_MODIFY includes TICKET_APPEND and TICKET_CHGPROP permissions, I changed email2trac to check for TICKET_MODIFY whenever it checks for one of the other two permissions.
I also matched Trac behavior with respect to logging, so that when logging to file or stderr without an explicit log_format, a timestamp precedes the log message.
And I changed the log line that reports the subject of the email from 'debug' to 'info' so that in the case of success, there would be at least one line in the log file to indicate that email2trac processed that email.
Hope this helps.
Dennis McRitchie?
Attachments (2)
Change History (8)
Changed 13 years ago by dmcr@…
comment:1 Changed 13 years ago by bas
- Status changed from new to assigned
- Type changed from defect to enhancement
Dennis thanks for the patch. I will apply it tomorrow ;-)
comment:2 Changed 13 years ago by Dennis McRitchie <dmcr@…>
Hi Bas,
You're welcome. However, hold off a bit on applying that patch. I tested it and adding an explicit test for TICKET_MODIFY seemed to fix my problem earlier today. But, I was just doing some tests of the Trac permission system using the same perm.check_permission() call you use, and those tests are returning True for a check of TICKET_APPEND, even when the user only has TICKET_MODIFY.
So I need to check this out further tomorrow, and I'll get back to you.
Sorry for the confusion.
Dennis McRitchie
comment:3 Changed 13 years ago by Dennis McRitchie <dmcr@…>
Hi again Bas,
My bad. Your code was correct: checking for a subset permission is sufficient when the user has the superset permission. My problem was that I did not have changeset #484 as part of my code (I'm still on 1.6.0), so the ticket update failed when checking permissions since the smtp_default_domain was not being used to do the comparison. So now my 2nd patch excludes that 'fix', and only includes:
"I also matched Trac behavior with respect to logging, so that when logging to file or stderr without an explicit log_format, a timestamp precedes the log message.
And I changed the log line that reports the subject of the email from 'debug' to 'info' so that in the case of success, there would be at least one line in the log file to indicate that email2trac processed that email."
No big deal, but they might be useful.
Sorry for the confusion.
Dennis
comment:4 Changed 13 years ago by bas
Thanks for the feedback. I will apply the minor patches. It is useful.
comment:5 Changed 13 years ago by bas
- Milestone set to New email2trac release 2.X.X
comment:6 Changed 13 years ago by bas
- Resolution set to fixed
- Status changed from assigned to closed
(In [516]) applied patch from Dennis McRitchie?, closes #236
Fix to ticket update bug, and other minor enhancements