Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #37 - send email when askers' bids are met #61

Merged
merged 2 commits into from
Dec 27, 2014
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions auctions/migrations/0003_bid_ask_match_sent.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import models, migrations


class Migration(migrations.Migration):

dependencies = [
('auctions', '0002_auto_20141005_0226'),
]

operations = [
migrations.AddField(
model_name='bid',
name='ask_match_sent',
field=models.BooleanField(default=False),
preserve_default=True,
),
]
23 changes: 23 additions & 0 deletions auctions/models.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,32 @@
from django.conf import settings
from django.db import models
from django.db.models import Sum
from django.db.models.signals import post_save
from django.dispatch import receiver

from mailer import send_mail


class Bid(models.Model):
user = models.ForeignKey(settings.AUTH_USER_MODEL)
url = models.URLField()
ask = models.DecimalField(max_digits=6, decimal_places=2)
ask_match_sent = models.BooleanField(default=False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nullable DateTimeField might be more useful than just a BooleanField.

offer = models.DecimalField(max_digits=6, decimal_places=2)


@receiver(post_save, sender=Bid)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to decouple sending emails from the post_save signal.

def notify_matching_askers(sender, instance, **kwargs):
issue_bids = Bid.objects.filter(url=instance.url).aggregate(Sum('offer'))
met_asks = (Bid.objects.filter(url=instance.url,
ask__lte=issue_bids['offer__sum'],
ask_match_sent=False)
.exclude(ask__lte=0))
for bid in met_asks:
send_mail("[codesy] Your ask for %(ask)d for %(url)s has been met" % (
{'ask': bid.ask, 'url': bid.url}),
"Bidders have met your asking price for %s." % bid.url,
settings.DEFAULT_FROM_EMAIL,
[bid.user.email])
# use .update to avoid recursive signal processing
Bid.objects.filter(id=bid.id).update(ask_match_sent=True)
64 changes: 64 additions & 0 deletions auctions/tests/model_tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import fudge

from django.conf import settings
from django.test import TestCase

from model_mommy import mommy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally prefer fully isolated unit tests over the model_mommy approach-- see the discussion here for more mozilla/masterfirefoxos#23 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I follow your reasoning against the model_mommy approach? The 2 articles you cited in the comment both exempt model-level test code from isolated testing.

  • "Many of those mock-free tests are on models, which I test in integration with the database." -Destroy All Software
  • "At the models layer, we no longer need to write isolated tests—the whole point of the models layer is to integrate with the database, so it’s appropriate to write integrated tests" -TDD with Python



class NotifyMatchersReceiverTest(TestCase):
def setUp(self):
"""
Set up the following market of Bids for a single bug:
user1: ask 50, offer 0
user2: ask 100, offer 10
user3: ask 0, offer 30
"""
self.url = 'http://github.com/codesy/codesy/issues/37'
self.bid1 = mommy.make('auctions.Bid', user__email='user1@test.com',
ask=50, offer=0, url=self.url)
self.bid2 = mommy.make('auctions.Bid', user__email='user2@test.com',
ask=100, offer=10, url=self.url)
self.bid3 = mommy.make('auctions.Bid', user__email='user3@test.com',
ask=0, offer=30, url=self.url)

def _add_url(self, string):
return string.format(url=self.url)

@fudge.patch('auctions.models.send_mail')
def test_send_mail_to_matching_askers(self, mock_send_mail):
user = mommy.make(settings.AUTH_USER_MODEL)

mock_send_mail.expects_call().with_args(
self._add_url('[codesy] Your ask for 50 for {url} has been met'),
self._add_url('Bidders have met your asking price for {url}.'),
settings.DEFAULT_FROM_EMAIL,
['user1@test.com']
)
mock_send_mail.next_call().with_args(
self._add_url('[codesy] Your ask for 100 for {url} has been met'),
self._add_url('Bidders have met your asking price for {url}.'),
settings.DEFAULT_FROM_EMAIL,
['user2@test.com']
)

offer_bid = mommy.prepare(
'auctions.Bid', offer=100, user=user, ask=1000, url=self.url)
offer_bid.save()

@fudge.patch('auctions.models.send_mail')
def test_only_send_mail_to_unsent_matching_askers(self, mock_send_mail):
user = mommy.make(settings.AUTH_USER_MODEL)
self.bid1.ask_match_sent = True
self.bid1.save()

mock_send_mail.expects_call().with_args(
self._add_url('[codesy] Your ask for 100 for {url} has been met'),
self._add_url('Bidders have met your asking price for {url}.'),
settings.DEFAULT_FROM_EMAIL,
['user2@test.com']
)

offer_bid = mommy.prepare(
'auctions.Bid', offer=100, user=user, ask=1000, url=self.url)
offer_bid.save()
1 change: 1 addition & 0 deletions codesy/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
'allauth.account',
'allauth.socialaccount',
'allauth.socialaccount.providers.github',
'mailer',
'rest_framework',
'rest_framework_swagger'
)
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ Django==1.7.1
dj-database-url==0.3.0
dj-static==0.0.6
django-allauth==0.18.0
django-mailer==0.1.0
django-toolbelt==0.0.1
django-rest-swagger==0.2.0
djangorestframework==2.4.4
Expand Down