Email notifications not sent for updates to watched tickets and still sent for blocked tickets

[iandunn]

It looks like email notifications are no longer being sent for watched tickets. They're still sent for tickets that the user has reported or replied to.

It's likely that this broke during the recent migration.

​https://wordpress.slack.com/archives/C0C89GD35/p1524083152000448

[netweb]

I'm subscribed to coding-standards and javascript focuses ​https://make.wordpress.org/core/notifications/

I expected to have received emails for ​#WP43907 and ​#WP43871 when the tickets were created though I did not

[swissspidy]

I am getting notifications for tickets even after I blocked them. So that case needs to be tested as well.

[danieltj]

I'm getting emails for lots of tickets I've explicitly blocked too. Is there an obvious cause to this?

[dd32]

Replying to danieltj:

Is there an obvious cause to this?

The updated version of Trac probably sends things slightly differently, or checks things differently than our existing Notifications handling code expects.

Unfortunately we have no-one who understands how Trac works, or our custom notifications stuff, so finding what's broken isn't straight forward.

[SergeyBiryukov]

Related: #4369

[ocean90]

#4369 was marked as a duplicate.

[nacin]

I only learned about this a few weeks ago, and made some time this weekend to look.

This appears to have broken when Trac was upgraded to 1.2.2 last year. We were running a core patch, and while that patch was updated for 1.2.2, it ran against deprecated code because the entire notifications process was refactored. The most relevant link upstream is probably ​https://trac.edgewall.org/changeset/13469.

The refactoring gives Trac some proper support for managing how you wish to be notified. If we wanted, we may be able to move to Trac's UI and DB schema, but it seems not worth the effort (and probably requires some work from ​https://trac.edgewall.org/ticket/11871 that is not complete).

In practice, at the very least, we can use the new API and extension points to inject our own subscription logic pretty easily. I've lightly tested this code below. If installed as a plugin, it should work. I'll do some more testing and talk with the systems team.

from trac.core import Component, implements
from trac.notification.mail import RecipientMatcher
from trac.notification.api import INotificationSubscriber


class WordPressNotificationSubscriber(Component):
    implements(INotificationSubscriber)

    def matches(self, event):
        if not self.env.config.getbool('wordpress', 'fine_grained_notifications'):
            return
        if event.realm != 'ticket':
            return
        if event.category not in ('created', 'changed', 'attachment added'):
            return

        ticket = event.target

        matcher = RecipientMatcher(self.env)
        klass = self.__class__.__name__
        format = None

        priority = 2
        for username in self.get_subscribers(ticket, event):
            recipient = matcher.match_recipient(username)
            if recipient:
                sid, authenticated, address = recipient
                yield (klass, 'email', sid, authenticated, address, format,
                       priority, 'always')

        priority = 1
        for username in self.get_unsubscribers(ticket, event):
            recipient = matcher.match_recipient(username)
            if recipient:
                sid, authenticated, address = recipient
                yield (klass, 'email', sid, authenticated, address, format,
                       priority, 'never')

    def get_subscribers(self, ticket, event):
        # People can subscribe to new tickets
        if event.category == 'created':
            for row in self.env.db_query("""
                    SELECT username FROM _notifications
                    WHERE type = 'newticket' AND value = '1'
                    """):
                yield row[0]

        # People can subscribe to components, milestones, and focuses
        component = old_component = ticket['component']
        milestone = old_milestone = ticket['milestone']
        if 'focuses' in ticket:
            focuses = set(ticket['focuses'].split(', '))
        else:
            focuses = None

        # Make sure we include the previous component, milestone, or focus too
        if 'fields' in event.changes:
            if 'component' in event.changes['fields']:
                old_component = event.changes['fields']['component']['old']

            if 'milestone' in event.changes['fields']:
                old_milestone = event.changes['fields']['milestone']['old']

            if 'focuses' in ticket and 'focuses' in event.changes['fields']:
                focuses |= set(event.changes['fields']['focuses']['old'].split(', '))

        # Add component subscribers
        for row in self.env.db_query("""
                SELECT username FROM _notifications
                WHERE type = 'component' AND value IN ( %s, %s )
                """, (component, old_component)):
            yield row[0]

        # Add milestone subscribers
        for row in self.env.db_query("""
                SELECT username FROM _notifications
                WHERE type = 'milestone' AND value IN ( %s, %s )
                """, (milestone, old_milestone)):
            yield row[0]

        # Add focus subscribers
        if focuses:
            focuses = list(focuses)
            for row in self.env.db_query("""
                    SELECT username FROM _notifications
                    WHERE type = 'focus' AND value IN ( %s )
                    """ % ','.join(['%s'] * len(focuses)), focuses):
                yield row[0]

        # Add individual ticket subscribers
        for row in self.env.db_query("""
                SELECT username FROM _ticket_subs
                WHERE ticket = %s AND status > 0
                """, (ticket.id,)):
            yield row[0]

    def get_unsubscribers(self, ticket, event):
        # If a user has specifically blocked notifications for a ticket,
        # remove them (even if they are a reporter, owner, or updater).
        for row in self.env.db_query("""
                SELECT username FROM _ticket_subs
                WHERE ticket = %s AND status = 0
                """, (ticket.id,)):
            yield row[0]

    def description(self):
        return None  # not configurable

    def requires_authentication(self):
        return False

    def default_subscriptions(self):
        return ()

[dd32]

In 9240:

Trac: Remove the Notification control functionality from the UI to avoid people thinking it'll work when it's infact been broken for 18mths.

See #3594.

[dd32]

In 9241:

Trac: Bump scripts cache after r9240.

See #3594.

[archon810]

This ticket recently came up here ​https://core.trac.wordpress.org/ticket/37110#comment:170.

[SergeyBiryukov]

#5095 was marked as a duplicate.