Skip to content

Conversation

@the-moog
Copy link

I think this creates support for trac 1.4, or at least it seems to be working for us. Not sure I used the correct trac API methods though and I'm fairly sure there is more that needs updating. (I'm not familiar with trac APIs)

Copy link
Member

@rjollos rjollos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good aside from the minor things I noted. If we can remove the commented code too, I think it's close to ready to merge.

import pkg_resources

pkg_resources.require('Trac >= 1.0')
pkg_resources.require('Trac >= 1.3')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes you made should be compatible with 1.2 as well. Could we set this to 1.2?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can test it, sure

from genshi.filters import Transformer

from trac.web.chrome import web_context

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd move this into the from trac.* block of imports.

def __init__(self, target, time, author, comment=None):

super(TicketChangeEvent, self).__init__('ticket', 'reminder', target,
time, author)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class may not be necessary. TicketChangeEvent has a 6th arg comment. Can TicketChangeEvent be used instead?

@rjollos rjollos self-assigned this Nov 20, 2020
@the-moog
Copy link
Author

I may have been a bit premature. Still testing on our system. Found one issue already. Will know more tomorrow. I subclassed TicketChangeEvent as I thought perhaps a reminder needed some extra info e.g. age (not added yet)

@the-moog
Copy link
Author

I think I've found the cause of the problem. Not sure what to do to fix it.
Can somebody assist?
See the-moog#1 (comment)

@rjollos
Copy link
Member

rjollos commented Nov 23, 2020

I think I've found the cause of the problem. Not sure what to do to fix it.
Can somebody assist?
See the-moog#1 (comment)

You'll need to implement an INotificationSubscriber. Examples:

AlwaysEmailSubscriber is a subscriber that is always active. TicketOwnerSubscriber requires the user to subscribe by setting their preferences (/prefs/notification), or a default subscription configured. See INotificationSubscriber.

Let me know if you have any questions. Some aspects of implementing INotificationSubscriber can be confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants