Modify

Opened 12 years ago

Closed 12 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)

email2trac-2.0.2.patch (2.0 KB) - added by dmcr@… 12 years ago.
Fix to ticket update bug, and other minor enhancements
email2trac-2.0.2.patch2 (1.1 KB) - added by Dennis McRitchie <dmcr@…> 12 years ago.
Fix to previous patch

Download all attachments as: .zip

Change History (8)

Changed 12 years ago by dmcr@…

Fix to ticket update bug, and other minor enhancements

comment:1 Changed 12 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 12 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 12 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

Changed 12 years ago by Dennis McRitchie <dmcr@…>

Fix to previous patch

comment:4 Changed 12 years ago by bas

Thanks for the feedback. I will apply the minor patches. It is useful.

comment:5 Changed 12 years ago by bas

  • Milestone set to New email2trac release 2.X.X

comment:6 Changed 12 years ago by bas

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [516]) applied patch from Dennis McRitchie?, closes #236

Add Comment

Modify Ticket

Change Properties
Action
as closed The owner will remain bas.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.