WordPress.org

Making WordPress.org

Opened 20 months ago

Last modified 7 weeks ago

#3594 new defect

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

Reported by: iandunn Owned by:
Milestone: Priority: high
Component: Trac Keywords: has-patch
Cc:

Description

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

Change History (13)

#1 @netweb
20 months ago

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

#2 @swissspidy
20 months ago

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

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


17 months ago

#4 follow-up: @danieltj
17 months ago

  • Keywords needs-patch added

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

#5 in reply to: ↑ 4 @dd32
17 months ago

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.

#7 @ocean90
8 months ago

#4369 was marked as a duplicate.

#8 @ocean90
8 months ago

  • Summary changed from Email notifications not sent for updates to watched tickets to Email notifications not sent for updates to watched tickets and still sent for blocked tickets

This ticket was mentioned in Slack in #meta by ocean90. View the logs.


8 months ago

#10 @nacin
7 months ago

  • Keywords has-patch added; needs-patch removed

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 ()
Last edited 7 months ago by nacin (previous) (diff)

This ticket was mentioned in Slack in #meta by elrae. View the logs.


7 weeks ago

#12 @dd32
7 weeks ago

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.

#13 @dd32
7 weeks ago

In 9241:

Trac: Bump scripts cache after r9240.

See #3594.

Note: See TracTickets for help on using tickets.