Fixes OOIION-590 (Fixes UNS regarding emails being sent to events not subscribed to) #447

Merged
merged 5 commits into from Jan 8, 2013

Conversation

Projects
None yet
3 participants
@swarbhanu
Contributor

swarbhanu commented Jan 7, 2013

The UNS utility method designed to figure out interested users was not properly handling the case of events coming from the same origin and whose event subtypes and origin_type fields were empty.

This pull request patches the method so that it properly handles all cases.

It has been verified that only emails for the event subscribed to is being sent now.

@daf

This comment has been minimized.

Show comment Hide comment
@daf

daf Jan 7, 2013

Contributor

Two comments on style (not related to content):

1: use comma syntax on log statements, not % interpolation

For instance, you have several log statements like this:

log.debug("For event_origin_type = %s too, UNS got interested users here  %s" % (event.origin_type, set()))

Instead, use this form:

log.debug("For event_origin_type = %s too, UNS got interested users here  %s", event.origin_type, set())

The goal being that if the DEBUG level is not set, it doesn't have to build the string that it sends to the debug call. The call will take care of interpolating the string for you. (You also get to avoid the extra parentheses pair that % requires).

(also what is up with the set() as a log param, I don't think that's what you meant to have in that line).

2: use extend() to append to a list, not +=

it's just more pythonic.

user_list_2.extend(reverse_user_info['event_origin'][''])
Contributor

daf commented Jan 7, 2013

Two comments on style (not related to content):

1: use comma syntax on log statements, not % interpolation

For instance, you have several log statements like this:

log.debug("For event_origin_type = %s too, UNS got interested users here  %s" % (event.origin_type, set()))

Instead, use this form:

log.debug("For event_origin_type = %s too, UNS got interested users here  %s", event.origin_type, set())

The goal being that if the DEBUG level is not set, it doesn't have to build the string that it sends to the debug call. The call will take care of interpolating the string for you. (You also get to avoid the extra parentheses pair that % requires).

(also what is up with the set() as a log param, I don't think that's what you meant to have in that line).

2: use extend() to append to a list, not +=

it's just more pythonic.

user_list_2.extend(reverse_user_info['event_origin'][''])
@swarbhanu

This comment has been minimized.

Show comment Hide comment
@swarbhanu

swarbhanu Jan 7, 2013

Contributor

Yep, I will push the change in a moment.

Contributor

swarbhanu commented Jan 7, 2013

Yep, I will push the change in a moment.

@swarbhanu

This comment has been minimized.

Show comment Hide comment
@swarbhanu

swarbhanu Jan 7, 2013

Contributor

Okay, so I pushed in the change in the commit below: 8cebc69. It addresses the better log debug style and the extend for lists.

Contributor

swarbhanu commented Jan 7, 2013

Okay, so I pushed in the change in the commit below: 8cebc69. It addresses the better log debug style and the extend for lists.

MauriceManning added a commit that referenced this pull request Jan 8, 2013

Merge pull request #447 from swarbhanu/log_debug_uns_check
Fixes OOIION-590 (Fixes UNS regarding emails being sent to events not subscribed to)

@MauriceManning MauriceManning merged commit 3509421 into ooici:master Jan 8, 2013

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