improve readability and clean up some code smells

monty5811 · Oct 9, 2015 · 2c75ef1 · 2c75ef1
1 parent 680d306
commit 2c75ef1
Show file tree

Hide file tree

Showing 17 changed files with 296 additions and 120 deletions.
diff --git a/.checkignore b/.checkignore
@@ -0,0 +1,2 @@
+tests/
+docs/
diff --git a/api/urls.py b/api/urls.py
@@ -17,86 +17,100 @@
 urlpatterns = [
     # sms views
     url(r'^v1/sms/in/$',
-        ApiCollection.as_view(model_class=SmsInbound,
-                              serializer_class=SmsInboundSerializer,
-                              permission_classes=(IsAuthenticated, CanSeeIncoming)
-                              ),
+        ApiCollection.as_view(
+            model_class=SmsInbound,
+            serializer_class=SmsInboundSerializer,
+            permission_classes=(IsAuthenticated, CanSeeIncoming)
+        ),
         name='in_log'),
     url(r'^v1/sms/out/$',
-        ApiCollection.as_view(model_class=SmsOutbound,
-                              serializer_class=SmsOutboundSerializer,
-                              permission_classes=(IsAuthenticated, CanSeeOutgoing),
-                              ),
+        ApiCollection.as_view(
+            model_class=SmsOutbound,
+            serializer_class=SmsOutboundSerializer,
+            permission_classes=(IsAuthenticated, CanSeeOutgoing),
+        ),
         name='out_log'),
     url(r'^v1/sms/live_wall/in/$',
         ApiCollectionAllWall.as_view(
             permission_classes=(IsAuthenticated, CanSeeIncoming)
         ),
         name='all_live_wall'),
     url(r'^v1/sms/in/recpient/(?P<pk>\d+)/$',
-        ApiCollectionRecentSms.as_view(permission_classes=(IsAuthenticated, CanSeeContactNames, CanSeeIncoming)),
+        ApiCollectionRecentSms.as_view(
+            permission_classes=(IsAuthenticated, CanSeeContactNames, CanSeeIncoming)),
         name='contact_recent_sms'),
     url(r'^v1/sms/in/keyword/(?P<pk>\d+)/$',
-        ApiCollectionKeywordSms.as_view(permission_classes=(IsAuthenticated, CanSeeKeywords, CanSeeKeyword, CanSeeIncoming),
-                                        archive=False
-                                        ),
+        ApiCollectionKeywordSms.as_view(
+            permission_classes=(IsAuthenticated, CanSeeKeywords, CanSeeKeyword, CanSeeIncoming),
+            archive=False
+        ),
         name='keyword_sms'),
     url(r'^v1/sms/live_wall/in/keyword/(?P<pk>\d+)/$',
-        ApiCollectionKeywordWall.as_view(permission_classes=(IsAuthenticated, CanSeeKeywords, CanSeeKeyword, CanSeeIncoming),),
+        ApiCollectionKeywordWall.as_view(
+            permission_classes=(IsAuthenticated, CanSeeKeywords, CanSeeKeyword, CanSeeIncoming),
+        ),
         name='keyword_live_wall'),
     url(r'^v1/sms/in/keyword/(?P<pk>\d+)/archive/$',
-        ApiCollectionKeywordSms.as_view(permission_classes=(IsAuthenticated, CanSeeKeywords, CanSeeKeyword, CanSeeIncoming),
-                                        archive=True),
+        ApiCollectionKeywordSms.as_view(
+            permission_classes=(IsAuthenticated, CanSeeKeywords, CanSeeKeyword, CanSeeIncoming),
+            archive=True),
         name='keyword_sms_archive'),
     url(r'^v1/sms/in/(?P<pk>[0-9]+)$',
-        ApiMember.as_view(model_class=SmsInbound,
-                          serializer_class=SmsInboundSerializer,
-                          permission_classes=(IsAuthenticated, CanSeeIncoming),
-                          ),
+        ApiMember.as_view(
+            model_class=SmsInbound,
+            serializer_class=SmsInboundSerializer,
+            permission_classes=(IsAuthenticated, CanSeeIncoming),
+        ),
         name='sms_in_member'),
     # recipient views
     url(r'^v1/recipients/$',
-        ApiCollection.as_view(model_class=Recipient,
-                              serializer_class=RecipientSerializer,
-                              filter_list=True,
-                              filters={'is_archived': False},
-                              permission_classes=(IsAuthenticated, CanSeeContactNames)
-                              ),
+        ApiCollection.as_view(
+            model_class=Recipient,
+            serializer_class=RecipientSerializer,
+            filter_list=True,
+            filters={'is_archived': False},
+            permission_classes=(IsAuthenticated, CanSeeContactNames)
+        ),
         name='recipients'),
     url(r'^v1/recipients/(?P<pk>[0-9]+)$',
-        ApiMember.as_view(model_class=Recipient,
-                          serializer_class=RecipientSerializer,
-                          permission_classes=(IsAuthenticated, CanSeeContactNames)
-                          ),
+        ApiMember.as_view(
+            model_class=Recipient,
+            serializer_class=RecipientSerializer,
+            permission_classes=(IsAuthenticated, CanSeeContactNames)
+        ),
         name='recipient'),
     # group views
     url(r'^v1/groups/$',
-        ApiCollection.as_view(model_class=RecipientGroup,
-                              serializer_class=RecipientGroupSerializer,
-                              filter_list=True,
-                              filters={'is_archived': False},
-                              permission_classes=(IsAuthenticated, CanSeeGroups)
-                              ),
+        ApiCollection.as_view(
+            model_class=RecipientGroup,
+            serializer_class=RecipientGroupSerializer,
+            filter_list=True,
+            filters={'is_archived': False},
+            permission_classes=(IsAuthenticated, CanSeeGroups)
+        ),
         name='recipient_groups'),
     url(r'^v1/groups/(?P<pk>[0-9]+)$',
-        ApiMember.as_view(model_class=RecipientGroup,
-                          serializer_class=RecipientGroupSerializer,
-                          permission_classes=(IsAuthenticated, CanSeeGroups)
-                          ),
+        ApiMember.as_view(
+            model_class=RecipientGroup,
+            serializer_class=RecipientGroupSerializer,
+            permission_classes=(IsAuthenticated, CanSeeGroups)
+        ),
         name='group'),
     # keyword views
     url(r'^v1/keywords/$',
-        ApiCollection.as_view(model_class=Keyword,
-                              serializer_class=KeywordSerializer,
-                              filter_list=True,
-                              filters={'is_archived': False},
-                              permission_classes=(IsAuthenticated, CanSeeKeywords)
-                              ),
+        ApiCollection.as_view(
+            model_class=Keyword,
+            serializer_class=KeywordSerializer,
+            filter_list=True,
+            filters={'is_archived': False},
+            permission_classes=(IsAuthenticated, CanSeeKeywords)
+        ),
         name='keywords'),
     url(r'^v1/keywords/(?P<pk>[0-9]+)$',
-        ApiMember.as_view(model_class=Keyword,
-                          serializer_class=KeywordSerializer,
-                          permission_classes=(IsAuthenticated, CanSeeKeyword)
-                          ),
+        ApiMember.as_view(
+            model_class=Keyword,
+            serializer_class=KeywordSerializer,
+            permission_classes=(IsAuthenticated, CanSeeKeyword)
+        ),
         name='keyword'),
 ]
diff --git a/api/views.py b/api/views.py
@@ -39,9 +39,9 @@ def get(self, request, format=None, **kwargs):
     def post(self, request, format=None, **kwargs):
         pk = kwargs['pk']
         reingest_sms = True if request.data.get('reingest', False) == 'true' else False
-        deal_with_sms = request.data.get('deal_with', None)
-        archive = request.data.get('archive', None)
-        display_on_wall = request.data.get('display_on_wall', None)
+        deal_with_sms = request.data.get('deal_with')
+        archive = request.data.get('archive')
+        display_on_wall = request.data.get('display_on_wall')
 
         obj = get_object_or_404(self.model_class, pk=pk)
         if archive is not None:

diff --git a/apostello/elvanto.py b/apostello/elvanto.py
@@ -77,12 +77,7 @@ def import_elvanto_groups(group_ids, user_email):
             bad_ppl.append(person)
 
         group.save()
-        if len(bad_ppl) == 0:
-            # send good email
-            send_async_mail.delay('[Apostello] Successful Elvanto Group Import',
-                                  '"{}" was imported successfully.'.format(group_name),
-                                  [user_email])
-        else:
+        if bad_ppl:
             # send email with failed data
             failed_people = ""
             for p in bad_ppl:
@@ -96,3 +91,8 @@ def import_elvanto_groups(group_ids, user_email):
             send_async_mail.delay('[Apostello] Failed Elvanto Group Import' + group_name,
                                   email_body,
                                   [user_email])
+        else:
+            # send good email
+            send_async_mail.delay('[Apostello] Successful Elvanto Group Import',
+                                  '"{}" was imported successfully.'.format(group_name),
+                                  [user_email])
diff --git a/apostello/forms.py b/apostello/forms.py
@@ -8,32 +8,41 @@
 
 
 class SendAdhocRecipientsForm(forms.Form):
-    content = forms.CharField(validators=[gsm_validator, less_than_sms_char_limit],
-                              required=True,
-                              min_length=1)
+    content = forms.CharField(
+        validators=[gsm_validator, less_than_sms_char_limit],
+        required=True,
+        min_length=1
+    )
     recipients = forms.ModelMultipleChoiceField(
         queryset=Recipient.objects.filter(is_archived=False),
         required=True,
         help_text=''
     )
-    scheduled_time = forms.DateTimeField(required=False,
-                                         widget=DateTimeWidget(bootstrap_version=3, options={'format': 'yyyy-mm-dd hh:ii'}),
-                                         help_text='Leave this blank to send your message immediately, otherwise select a date and time to schedule your message')
+    scheduled_time = forms.DateTimeField(
+        required=False,
+        widget=DateTimeWidget(bootstrap_version=3, options={'format': 'yyyy-mm-dd hh:ii'}),
+        help_text='Leave this blank to send your message immediately, otherwise select a date and time to schedule your message'
+    )
 
 
 class SendRecipientGroupForm(forms.Form):
-    content = forms.CharField(validators=[gsm_validator, less_than_sms_char_limit],
-                              required=True,
-                              min_length=1)
+    content = forms.CharField(
+        validators=[gsm_validator, less_than_sms_char_limit],
+        required=True,
+        min_length=1
+    )
     recipient_group = forms.ModelChoiceField(
         queryset=RecipientGroup.objects.filter(
-            is_archived=False),
+            is_archived=False
+        ),
         required=True,
         empty_label='Choose a group...'
     )
-    scheduled_time = forms.DateTimeField(required=False,
-                                         widget=DateTimeWidget(bootstrap_version=3, options={'format': 'yyyy-mm-dd hh:ii'}),
-                                         help_text='Leave this blank to send your message immediately, otherwise select a date and time to schedule your message')
+    scheduled_time = forms.DateTimeField(
+        required=False,
+        widget=DateTimeWidget(bootstrap_version=3, options={'format': 'yyyy-mm-dd hh:ii'}),
+        help_text='Leave this blank to send your message immediately, otherwise select a date and time to schedule your message'
+    )
 
 
 class ManageRecipientGroupForm(forms.ModelForm):
@@ -43,9 +52,12 @@ class Meta:
         exclude = ['is_archived']
 
     # Representing the many to many related field in SmsGroup
-    members = forms.ModelMultipleChoiceField(queryset=Recipient.objects.filter(is_archived=False),
-                                             required=False
-                                             )
+    members = forms.ModelMultipleChoiceField(
+        queryset=Recipient.objects.filter(
+            is_archived=False
+        ),
+        required=False
+    )
 
     def __init__(self, *args, **kwargs):
         if 'instance' in kwargs:
@@ -69,7 +81,9 @@ class RecipientForm(forms.ModelForm):
     class Meta:
         model = Recipient
         exclude = ['is_archived', 'is_blocking']
-        widgets = {'number': forms.TextInput(attrs={'placeholder': '+447259006790'})}
+        widgets = {
+            'number': forms.TextInput(attrs={'placeholder': '+447259006790'})
+        }
 
 
 class KeywordForm(forms.ModelForm):
@@ -92,8 +106,10 @@ class ArchiveKeywordResponses(forms.Form):
 
 
 class CsvImport(forms.Form):
-    csv_data = forms.CharField(help_text='John, Calvin, +447095237960',
-                               widget=forms.Textarea)
+    csv_data = forms.CharField(
+        help_text='John, Calvin, +447095237960',
+        widget=forms.Textarea
+    )
 
 
 class ElvantoImport(forms.Form):

diff --git a/apostello/models.py b/apostello/models.py
@@ -267,7 +267,7 @@ def num_archived_matches(self):
 
     def is_locked(self):
         """Determines if keyword is locked"""
-        if len(self.owners.all()) > 0:
+        if self.owners.all().count() > 0:
             return True
         else:
             return False
@@ -410,7 +410,7 @@ def reimport(self):
     def save(self, *args, **kwargs):
         super(SmsInbound, self).save(*args, **kwargs)
         keyword_ = Keyword.objects.filter(keyword=self.matched_keyword)
-        if len(keyword_) > 0:
+        if keyword_.count() > 0:
             cache.set('keyword_{}_only_live'.format(keyword_[0].pk), None, 0)
             cache.set('keyword_{}_all'.format(keyword_[0].pk), None, 0)
 

diff --git a/apostello/reply.py b/apostello/reply.py
@@ -33,11 +33,16 @@ def get_person_or_ask_for_name(from_, sms_body, keyword_obj):
             from apostello.models import SiteConfiguration
             config = SiteConfiguration.get_solo()
             if not config.disable_all_replies:
-                person_from.send_message(content=fetch_default_reply('auto_name_request'),
-                                         sent_by="auto name request")
+                person_from.send_message(
+                    content=fetch_default_reply('auto_name_request'),
+                    sent_by="auto name request"
+                )
                 notify_office_mail.delay(
                     '[Apostello] Unknown Contact!',
-                    'SMS:%s\nFrom:%s\n\n\nThis person is unknown and has been asked for their name.' % (sms_body, from_),
+                    'SMS: {0}\nFrom: {1}\n\n\nThis person is unknown and has been asked for their name.'.format(
+                        sms_body,
+                        from_
+                    ),
                 )
 
     return person_from
@@ -63,21 +68,28 @@ def reply_to_incoming(person_from, from_, sms_body, keyword):
             # update person's name:
             person_from.first_name = sms_body.split()[1].strip()
             person_from.last_name = " ".join(sms_body.split()[2:]).strip()
-            if len(person_from.last_name) < 1:
+            if not person_from.last_name:
                 raise ValidationError('No last name')
             person_from.save()
             # update old messages with this person's name
             update_msgs_name.delay(person_from.id)
             # thank person
             notify_office_mail.delay(
                 '[Apostello] New Signup!',
-                'SMS:\n\t%s\nFrom:\n\t%s\n' % (sms_body, from_),
+                'SMS:\n\t{0}\nFrom:\n\t{1}\n'.format(
+                    sms_body,
+                    from_,
+                ),
             )
+            # TODO update to use .format() and add help text to model
             return fetch_default_reply('name_update_reply') % person_from.first_name
         except (ValidationError, IndexError):
             notify_office_mail.delay(
                 '[Apostello] New Signup - FAILED!',
-                'SMS:\n\t%s\nFrom:\n\t%s\n' % (sms_body, from_),
+                'SMS:\n\t{0}\nFrom:\n\t{1}\n'.format(
+                    sms_body,
+                    from_
+                ),
             )
             return fetch_default_reply('name_failure_reply')
     else: