From 574286e8e39323220e3a1969621321bc96db0705 Mon Sep 17 00:00:00 2001 From: Oliver Marks Date: Sun, 17 Sep 2017 10:50:09 +0100 Subject: [PATCH] Fix duplicate records in membership table. --- config/settings/common.py | 7 ++- config/settings/local.py | 2 +- mhackspace/base/tasks.py | 11 ++-- mhackspace/requests/models.py | 2 +- mhackspace/subscriptions/helper.py | 14 +++-- .../commands/update_membership_status.py | 62 +++++++------------ .../{user => membership}/change_list.html | 0 mhackspace/templates/base.html | 1 + mhackspace/users/admin.py | 27 ++++---- .../migrations/0008_auto_20170917_0948.py | 27 ++++++++ mhackspace/users/models.py | 7 ++- 11 files changed, 91 insertions(+), 69 deletions(-) rename mhackspace/templates/admin/users/{user => membership}/change_list.html (100%) create mode 100644 mhackspace/users/migrations/0008_auto_20170917_0948.py diff --git a/config/settings/common.py b/config/settings/common.py index 71f3c95..8df72c5 100644 --- a/config/settings/common.py +++ b/config/settings/common.py @@ -375,7 +375,12 @@ CELERY_BROKER_URL = env('CELERY_BROKER_URL', default='redis://redis:6379/0') #if CELERY_BROKER_URL == 'django://': # CELERY_RESULT_BACKEND = 'redis://' #else: -CELERY_RESULT_BACKEND = 'django-cache' +CELERY_RESULT_BACKEND = 'redis://redis:6379/0' +CELERY_IGNORE_RESULT = False +CELERY_REDIS_HOST = "redis" +CELERY_REDIS_PORT = 6379 +CELERY_REDIS_DB = 0 + CELERY_BEAT_SCHEDULER = 'django_celery_beat.schedulers:DatabaseScheduler' INSTALLED_APPS += ('django_celery_results','django_celery_beat',) CELERY_TIMEZONE = 'UTC' diff --git a/config/settings/local.py b/config/settings/local.py index 8022b67..1b431e6 100644 --- a/config/settings/local.py +++ b/config/settings/local.py @@ -86,7 +86,7 @@ TEST_RUNNER = 'django.test.runner.DiscoverRunner' ########## CELERY # In development, all tasks will be executed locally by blocking until the task returns -CELERY_ALWAYS_EAGER = True +# CELERY_ALWAYS_EAGER = True ########## END CELERY # Your local stuff: Below this line define 3rd party library settings diff --git a/mhackspace/base/tasks.py b/mhackspace/base/tasks.py index 74a998d..8b6a040 100644 --- a/mhackspace/base/tasks.py +++ b/mhackspace/base/tasks.py @@ -6,8 +6,8 @@ from mhackspace.feeds.helper import import_feeds @shared_task def update_homepage_feeds(): - return import_feeds() - + import_feeds() + return {'result': 'Homepage feeds imported'} matrix_url = "https://matrix.org/_matrix/client/r0" matrix_login_url = matrix_url + "/login" @@ -31,9 +31,10 @@ def send_email(email_to, to=[email_to], headers={'Reply-To': 'no-reply@maidstone-hackspace.org.uk'}) email.send() + return {'result', 'Email sent to %s' % email_to} @shared_task -def matrix_message(message): +def matrix_message(message, prefix=''): # we dont rely on theses, so ignore if it goes wrong # TODO at least log that something has gone wrong try: @@ -59,8 +60,8 @@ def matrix_message(message): url = matrix_send_msg_url.format(**url_params) details = { "msgtype": "m.text", - "body": "[%s] %s" % (settings.MSG_PREFIX, message)} + "body": "[%s%s] %s" % (settings.MSG_PREFIX, prefix, message)} r2 = requests.post(url, json=details) except: pass - return True + return {'result', 'Matrix message sent successfully'} diff --git a/mhackspace/requests/models.py b/mhackspace/requests/models.py index 982abe7..edf32db 100644 --- a/mhackspace/requests/models.py +++ b/mhackspace/requests/models.py @@ -45,7 +45,7 @@ class UserRequests(models.Model): def send_topic_update_email(sender, instance, **kwargs): - matrix_message.delay('New Request - %s' % instance.title) + matrix_message.delay(prefix=' - REQUEST', message=instance.title) post_save.connect(send_topic_update_email, sender=UserRequests) diff --git a/mhackspace/subscriptions/helper.py b/mhackspace/subscriptions/helper.py index 434bc63..f060d41 100644 --- a/mhackspace/subscriptions/helper.py +++ b/mhackspace/subscriptions/helper.py @@ -1,13 +1,18 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals, absolute_import -from django.contrib import messages from django.contrib.auth.models import Group +from django.utils.dateparse import parse_datetime from mhackspace.users.models import Membership def create_or_update_membership(user, signup_details, complete=False): try: - member = Membership.objects.get(user=user) + member = Membership.objects.get(email=signup_details.get('email')) + # Only update if newer than last record, this way we only get the latest status + # cancellation and changed payment will not be counted against current status + start_date = parse_datetime(signup_details.get('start_date')) + if start_date < member.date: + return True except Membership.DoesNotExist: member = Membership() member.user = user @@ -25,6 +30,7 @@ def create_or_update_membership(user, signup_details, complete=False): return False # sign up not completed # add user to group on success - group = Group.objects.get(name='members') - user.groups.add(group) + if user: + group = Group.objects.get(name='members') + user.groups.add(group) return True # Sign up finished diff --git a/mhackspace/subscriptions/management/commands/update_membership_status.py b/mhackspace/subscriptions/management/commands/update_membership_status.py index c88901e..56af20e 100644 --- a/mhackspace/subscriptions/management/commands/update_membership_status.py +++ b/mhackspace/subscriptions/management/commands/update_membership_status.py @@ -1,5 +1,3 @@ -from datetime import datetime -from django.utils import timezone from django.contrib.auth.models import Group from django.forms.models import model_to_dict from django.core.management.base import BaseCommand @@ -8,6 +6,10 @@ from mhackspace.users.models import Membership, User from mhackspace.subscriptions.helper import create_or_update_membership +# this should be done in bulk, create the objects and save all at once +# for now its not an issue, because of small membership size + + def update_subscriptions(provider_name): provider = select_provider('gocardless') @@ -24,17 +26,9 @@ def update_subscriptions(provider_name): except User.DoesNotExist: user_model = None - subscriptions.append( - Membership( - user=user_model, - email=sub.get('email'), - reference=sub.get('reference'), - payment=10.00, - date= sub.get('start_date'), - # date=timezone.now(), - status=Membership.lookup_status(name=sub.get('status')) - ) - ) + create_or_update_membership(user=user_model, + signup_details=sub, + complete=True) yield model_to_dict(subscriptions[-1]) @@ -49,11 +43,11 @@ class Command(BaseCommand): provider = select_provider('gocardless') Membership.objects.all().delete() - subscriptions = [] group = Group.objects.get(name='members') for sub in provider.fetch_subscriptions(): + prefix = '' sub['amount'] = sub['amount'] * 0.01 try: user_model = User.objects.get(email=sub.get('email')) @@ -61,37 +55,23 @@ class Command(BaseCommand): user_model.groups.add(group) except User.DoesNotExist: user_model = None - self.style.NOTICE( - '\tMissing User {reference} - {payment} - {status} - {email}'.format(**{ - 'reference': sub.get('reference'), - 'payment': sub.get('amount'), - 'status': sub.get('status'), - 'email': sub.get('email') - })) - continue + prefix = 'NO USER - ' create_or_update_membership(user=user_model, signup_details=sub, complete=True) - subscriptions.append( - Membership( - user=user_model, - email=sub.get('email'), - reference=sub.get('reference'), - payment=sub.get('amount'), - date=sub.get('start_date'), - status=Membership.lookup_status(name=sub.get('status')) - ) - ) - self.stdout.write( - self.style.SUCCESS( - '\t{reference} - {payment} - {status} - {email}'.format(**{ - 'reference': sub.get('reference'), - 'payment': sub.get('amount'), - 'status': sub.get('status'), - 'email': sub.get('email') - }))) + message = '\t{prefix}{date} - {reference} - {payment} - {status} - {email}'.format(**{ + 'prefix': prefix, + 'date': sub.get('start_date'), + 'reference': sub.get('reference'), + 'payment': sub.get('amount'), + 'status': sub.get('status'), + 'email': sub.get('email') + }) - Membership.objects.bulk_create(subscriptions) + if user_model: + self.stdout.write(self.style.SUCCESS(message)) + else: + self.stdout.write(self.style.NOTICE(message)) diff --git a/mhackspace/templates/admin/users/user/change_list.html b/mhackspace/templates/admin/users/membership/change_list.html similarity index 100% rename from mhackspace/templates/admin/users/user/change_list.html rename to mhackspace/templates/admin/users/membership/change_list.html diff --git a/mhackspace/templates/base.html b/mhackspace/templates/base.html index f0fc887..d4b87dc 100644 --- a/mhackspace/templates/base.html +++ b/mhackspace/templates/base.html @@ -168,6 +168,7 @@
{% block footer-left %}{% endblock footer-left %} © {% now "Y" %} Maidstone Hackspace + Constitution
diff --git a/mhackspace/users/admin.py b/mhackspace/users/admin.py index cd49bb6..ef57971 100644 --- a/mhackspace/users/admin.py +++ b/mhackspace/users/admin.py @@ -11,7 +11,8 @@ from django.urls import reverse from django.conf.urls import url from .models import User, Rfid, Membership, MEMBERSHIP_STATUS_CHOICES -from mhackspace.subscriptions.management.commands.update_membership_status import update_subscriptions +# from mhackspace.subscriptions.management.commands.update_membership_status import update_subscriptions +from mhackspace.users.tasks import update_users_memebership_status class MyUserChangeForm(UserChangeForm): @@ -46,27 +47,27 @@ class MyUserAdmin(AuthUserAdmin): list_display = ('username', 'name', 'is_superuser') search_fields = ['name'] + +@admin.register(Membership) +class MembershipAdmin(ModelAdmin): + list_display = ('user_id', 'email', 'payment', 'date', 'status') + list_filter = ('status',) + def get_urls(self): - urls = super(MyUserAdmin, self).get_urls() + urls = super(MembershipAdmin, self).get_urls() my_urls = [ url(r'^refresh/payments/$', self.admin_site.admin_view(self.refresh_payments)) ] return my_urls + urls def refresh_payments(self, request): - for user in update_subscriptions(provider_name='gocardless'): - continue - self.message_user(request, 'Successfully imported refresh users payment status') - return HttpResponseRedirect(reverse('admin:feeds_article_changelist')) - - -@admin.register(Membership) -class MembershipAdmin(ModelAdmin): - list_display = ('user_id', 'user', 'payment', 'date', 'status') - list_filter = ('status',) + update_users_memebership_status() + # for user in update_subscriptions(provider_name='gocardless'): + # continue + self.message_user(request, 'Successfully triggered user payment refresh') + return HttpResponseRedirect(reverse('admin:index')) @admin.register(Rfid) class RfidAdmin(ModelAdmin): list_display = ('code', 'description') - diff --git a/mhackspace/users/migrations/0008_auto_20170917_0948.py b/mhackspace/users/migrations/0008_auto_20170917_0948.py new file mode 100644 index 0000000..5fdfbb3 --- /dev/null +++ b/mhackspace/users/migrations/0008_auto_20170917_0948.py @@ -0,0 +1,27 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.4 on 2017-09-17 09:48 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('users', '0007_auto_20170914_2021'), + ] + + operations = [ + migrations.AlterField( + model_name='membership', + name='email', + field=models.CharField(max_length=255, unique=True), + ), + migrations.AlterField( + model_name='membership', + name='user', + field=models.OneToOneField(blank=True, default=None, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='user', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/mhackspace/users/models.py b/mhackspace/users/models.py index 45d69ea..84c1526 100644 --- a/mhackspace/users/models.py +++ b/mhackspace/users/models.py @@ -70,17 +70,18 @@ MEMBERSHIP_STATUS = { class Membership(models.Model): - user = models.ForeignKey( + user = models.OneToOneField( settings.AUTH_USER_MODEL, null=True, blank=True, default=None, - related_name='user' + related_name='user', + unique=True ) payment = models.DecimalField(max_digits=6, decimal_places=2, default=0.0) date = models.DateTimeField() reference = models.CharField(max_length=255) status = models.PositiveSmallIntegerField(default=0, choices=MEMBERSHIP_STATUS_CHOICES) - email = models.CharField(max_length=255) + email = models.CharField(max_length=255, unique=True) @property def get_status(self):