If a user is subscribed to a tag and that tag is deleted, a blank subscription remains under the user's "Notifications - Subscriptions" tab, as well as under the "Manage subscriptions" admin page. This is because the subscription is deleted from notifications_fields but not from notifications.

Looking at the code, I don't know why this would be, since all of the deletes occur in a simple foreach loop. Any ideas?

CommentFileSizeAuthor
#1 notifications-477954-1.patch1006 bytesdanepowell

Comments

danepowell’s picture

danepowell commented
Title: Table "notifications" not updated when a subscribed-to tag is deleted » Subscriptions to tags not deleted when tags are deleted
Status: Active » Needs review
StatusFileSize
new notifications-477954-1.patch1006 bytes

Sorry, my analysis was wrong - the subscription isn't deleted at all, because hook_taxonomy is not even implemented :) Here's a patch that takes care of that.

David Goode’s picture

David Goode commented
Priority: Minor » Normal

Hey,

Thanks for the catch, doesn't look like they would be deleted. However, this would actually delete anything that happens to have the same int value as the tid--say a subscription to a nid for notifications_content. Anyway, there is a function in notifications that does this for you. Here's the preliminary modified code from your patch--I'd appreciate it if you could test this out and see if it works for you.

Also, we should take care of vocabs, look at other contingencies, and perhaps check out other subscription types to see if they are missing delete hooks before committing a patch. Any help anyone can provide in these areas would be appreciated.

Thanks a lot,
David

/**
 * Implementation of hook_taxonomy().
 */
function notifications_tags_taxonomy($op, $type, $array = NULL) {
  switch ($op) {
    case 'delete':
      switch ($type) {
        // Only act on terms (TODO: deal with deleted vocabularies)
        case 'term':
          notifications_delete_subscriptions(array('event_type' => 'node'), array('tid' => $array['tid']), FALSE);
          break;
      }
      break;
  }
}
jose reyero’s picture

jose reyero commented

@David, I think your fixed version looks good, commit when you think its ready.

If unsure, you can add more tests :-)

danepowell’s picture

danepowell commented

@David, Good catch, I don't know why I didn't think to use that function. I applied your version of the patch on my site and it works great.

I will take a look at handling deleted vocabularies and content types. My philosophy is usually "if you've got a fix, commit it", and you can flesh it out later. But since this isn't a critical issue, a commit can probably wait until we do it right.

David Goode’s picture

David Goode commented
Status: Needs review » Fixed

Committed! Thanks for the help danep.

David

danepowell’s picture

danepowell commented

On further review it looks like deleted content types are already taken care of in the code. Deleting vocabularies are also taken care of because when a vocabulary is deleted, hook_taxonomy fires for each deleted term.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.