Add deliver_at and deliver_in methods for scheduling mail #32

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
5 participants
@hmarr
Contributor

hmarr commented May 10, 2012

If resque-scheduler is installed, two extra methods (deliver_at and deliver_in) are available for scheduling mail in the future.

Thanks!

@tomblomfield

This comment has been minimized.

Show comment
Hide comment
@tomblomfield

tomblomfield May 10, 2012

This looks awesome!

👍

This looks awesome!

👍

Move environment_excluded logic to MessageDecoy
This means that extra methods defined on MessageDecoy (e.g. enqueue_at)
will still be available in excluded environments.
@hmarr

This comment has been minimized.

Show comment
Hide comment
@hmarr

hmarr May 22, 2012

Contributor

I just added a commit that moves the logic for checking if the Rails environment is excluded to the MessageDecoy class.

Before, if we were in an excluded environment, no MessageDecoy was returned. This meant that in excluded environments (such as in tests) the extra methods added to MessageDecoy weren't present and result in a method undefined error.

Now, a MessageDecoy is always returned. When you call one of the #deliver* methods on the decoy object, the environment is checked.

Contributor

hmarr commented May 22, 2012

I just added a commit that moves the logic for checking if the Rails environment is excluded to the MessageDecoy class.

Before, if we were in an excluded environment, no MessageDecoy was returned. This meant that in excluded environments (such as in tests) the extra methods added to MessageDecoy weren't present and result in a method undefined error.

Now, a MessageDecoy is always returned. When you call one of the #deliver* methods on the decoy object, the environment is checked.

@zapnap

This comment has been minimized.

Show comment
Hide comment
@zapnap

zapnap May 28, 2012

Owner

That definitely makes me a little more comfortable -- was one of my questions about the approach before and I probably should have commented earlier. I'm still a little unsure if we should be adding methods for compatibility with other resque plugins -- but it does seem that scheduler is in a unique position for this. Hmm... going to think about this. Anyone else want to weigh in with an opinion?

Owner

zapnap commented May 28, 2012

That definitely makes me a little more comfortable -- was one of my questions about the approach before and I probably should have commented earlier. I'm still a little unsure if we should be adding methods for compatibility with other resque plugins -- but it does seem that scheduler is in a unique position for this. Hmm... going to think about this. Anyone else want to weigh in with an opinion?

@hmarr

This comment has been minimized.

Show comment
Hide comment
@hmarr

hmarr Jun 8, 2012

Contributor

I know what you mean about adding support for other plugins, but it does seem that scheduler is popular enough to make it worthwhile including this. I'd quite like to see this merged, but if you feel it's out of scope, that's cool - I could always just release it as a separate gem (resque-mailer-scheuler?)

Contributor

hmarr commented Jun 8, 2012

I know what you mean about adding support for other plugins, but it does seem that scheduler is popular enough to make it worthwhile including this. I'd quite like to see this merged, but if you feel it's out of scope, that's cool - I could always just release it as a separate gem (resque-mailer-scheuler?)

@zapnap

This comment has been minimized.

Show comment
Hide comment
@zapnap

zapnap Jun 10, 2012

Owner

Seems reasonable. I'll probably yank this into the next release once I have a few minutes to test it out.

Owner

zapnap commented Jun 10, 2012

Seems reasonable. I'll probably yank this into the next release once I have a few minutes to test it out.

@nc

This comment has been minimized.

Show comment
Hide comment
@nc

nc Jul 17, 2012

+1 Can't wait for this to make it in.

nc commented Jul 17, 2012

+1 Can't wait for this to make it in.

@excid3

This comment has been minimized.

Show comment
Hide comment
@excid3

excid3 Aug 21, 2012

👍

excid3 commented Aug 21, 2012

👍

@zapnap

This comment has been minimized.

Show comment
Hide comment
@zapnap

zapnap Aug 23, 2012

Owner

Thanks for your patience on this. I've merged it into master so it should be available now. Please check it out and let me know if things look good. I'll release an updated gem after a few folks confirm that it's working as expected (I'm not using resque-scheduler myself).

Owner

zapnap commented Aug 23, 2012

Thanks for your patience on this. I've merged it into master so it should be available now. Please check it out and let me know if things look good. I'll release an updated gem after a few folks confirm that it's working as expected (I'm not using resque-scheduler myself).

@zapnap zapnap closed this Aug 23, 2012

@zapnap

This comment has been minimized.

Show comment
Hide comment
@zapnap

zapnap Sep 13, 2012

Owner

@hmarr @excid3 @nc @tomblomfield have any of you had a chance to test?

Owner

zapnap commented Sep 13, 2012

@hmarr @excid3 @nc @tomblomfield have any of you had a chance to test?

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