Merge pull request #245 from eaudeweb/fix-duplicates

Fix repeated emails about the same tenders

app/management/commands/base/notify.py

@@ -3,7 +3,7 @@
 from getenv import env
 
 from app.notifications import build_email
-from app.models import Notification, SOURCE_CHOICES
+from app.models import Tender, Notification, SOURCE_CHOICES
 from app.parsers.ted import TEDWorker
 from app.parsers.ungm import UNGMWorker
 from app.management.commands.base.params import BaseParamsUI
@@ -24,17 +24,29 @@ def get_parameters():
 
     @property
     def notification_type(self):
-        raise NotImplementedError('subclasses of BaseNotifyCommand must provide a notification_type member')
+        raise NotImplementedError(
+            'subclasses of BaseNotifyCommand must provide a notification_type '
+            'member'
+        )
 
     def get_tenders(self):
-        raise NotImplementedError('subclasses of BaseNotifyCommand must provide a get_tenders() method')
+        raise NotImplementedError(
+            'subclasses of BaseNotifyCommand must provide a get_tenders() '
+            'method'
+        )
 
     def handle(self, *args, **options):
         digest = options['digest']
         changed_tenders = self.scrape_tenders()
 
         if changed_tenders:
-            BaseNotifyCommand.send_update_email(changed_tenders, digest, self.notification_type())
+            BaseNotifyCommand.send_update_email(
+                changed_tenders, digest, self.notification_type())
+        self.stdout.write(
+            self.style.SUCCESS(
+                f'Sent notifications about {changed_tenders.count()} tender(s).'
+            )
+        )
 
     def add_arguments(self, parser):
         parser.add_argument(
@@ -45,16 +57,16 @@ def add_arguments(self, parser):
 
     @staticmethod
     def send_update_email(tenders, digest, notification_type):
-        subject = f'{notification_type} tenders Update' if digest else f'{notification_type} tender Update'
+        s = 's' if digest else ''
+        subject = f'{notification_type} tender{s} Update'
         notifications = Notification.objects.all()
         recipients = [notification.email for notification in notifications]
-        tends = [tx[0] for tx in tenders]
 
         if digest:
             html_content = render_to_string(
                 'mails/tender_update.html',
                 {
-                    'tenders': tends,
+                    'tenders': tenders,
                     'domain': env('BASE_URL'),
                     'notification_type': notification_type
                 }
@@ -68,7 +80,7 @@ def send_update_email(tenders, digest, notification_type):
                 html_content = render_to_string(
                     'mails/tender_update.html',
                     {
-                        'tenders': [tender[0]],
+                        'tenders': [tender],
                         'domain': env('BASE_URL'),
                         'notification_type': notification_type
                     }
@@ -77,6 +89,8 @@ def send_update_email(tenders, digest, notification_type):
                 email = build_email(subject, recipients, None, html_content)
                 email.send()
 
+        tenders.update(notified=True)
+
     def scrape_tenders(self):
         """
         Downloads new tender data and scrape it, using the information to
@@ -98,5 +112,9 @@ def scrape_tenders(self):
         if ungm_tenders.exists():
             w = UNGMWorker()
             changed_ungm_tenders, _ = w.parse_tenders(ungm_tenders)
-
-        return changed_ted_tenders + changed_ungm_tenders
+        return Tender.objects.filter(
+            reference__in=[
+                t[0]['reference']
+                for t in changed_ted_tenders + changed_ungm_tenders
+            ]
+        )

app/management/commands/notify_keywords.py

@@ -4,7 +4,10 @@
 
 
 class Command(BaseNotifyCommand, BaseParamsUI):
-    help = 'Notifies all users about tenders which contain one or more keywords'
+    help = (
+        "Notifies all users about updates to existing tenders which contain "
+        "one or more keywords."
+    )
 
     def notification_type(self):
         return 'Keyword'

app/management/commands/notify_tenders.py

@@ -27,5 +27,11 @@ def add_arguments(self, parser):
     def handle(self, *args, **options):
         digest = options['digest']
         tenders = Tender.objects.filter(notified=False).order_by('-published')
-        if tenders:
+        tender_count = tenders.count()
+        if tender_count:
             send_tenders_email(tenders, digest)
+        self.stdout.write(
+            self.style.SUCCESS(
+                f'Sent notifications about {tender_count} tender(s).'
+            )
+        )

app/notifications.py

@@ -24,8 +24,6 @@ def send_tenders_email(tenders, digest):
         email = build_email(subject, recipients, None, html_content)
         email.send()
 
-        tenders.update(notified=True)
-
     else:
         for tender in tenders:
             html_content = render_to_string(
@@ -40,7 +38,7 @@ def send_tenders_email(tenders, digest):
             email = build_email(subject, recipients, None, html_content)
             email.send()
 
-            set_notified(tender)
+    tenders.update(notified=True)
 
 
 def send_awards_email(awards, digest):

app/parsers/ted.py

@@ -114,7 +114,7 @@ def download_archive(self, ftp, archive_date, archives):
 
     def parse_notices(
         self, tenders=None, set_notified=False
-    ) -> Tuple[Tuple[Tender, dict], int]:
+    ) -> Tuple[Tuple[dict, dict], int]:
         """
         Parse an archive file, extract all tender data from it and use it to
         update existing tenders or create new ones.
@@ -123,16 +123,16 @@ def parse_notices(
             tenders = []
         all_updated_tenders = []
         folders = []
-        new_tender_count = 0
+        total_created_tenders = 0
         for archive_path in self.archives:
             folder_name = self.extract_data(archive_path, self.path)
             if folder_name:
                 folders.append(folder_name)
                 p = TEDParser(self.path, [folder_name])
-                updated_tenders, created_tenders = p.parse_notices(
+                updated_tenders, num_created_tenders = p.parse_notices(
                     tenders, set_notified)
                 all_updated_tenders += updated_tenders
-                new_tender_count += created_tenders
+                total_created_tenders += num_created_tenders
                 folder_date = folder_name[:8]
                 formatted_date = datetime.strptime(
                     folder_date, '%Y%m%d').strftime('%d/%m/%Y')
@@ -152,7 +152,7 @@ def parse_notices(
                 logging.warning(e)
                 pass
 
-        return all_updated_tenders, new_tender_count
+        return all_updated_tenders, total_created_tenders
 
     @staticmethod
     def extract_data(archive_path, extract_path):
@@ -383,8 +383,6 @@ def _parse_notice(
         if tender['notice_type'] == 'Contract award notice':
             self.update_contract_award_notice_awards(awards, soup, set_notified)
 
-        tender['notified'] = set_notified
-
         return tender, awards
 
     @staticmethod
@@ -393,10 +391,12 @@ def file_in_tender_list(xml_file, tenders):
         return os.path.basename(xml_file).replace(
             '_', '-').replace('.xml', '') in tender_references
 
-    def parse_notices(self, tenders, set_notified):
+    def parse_notices(
+        self, tenders: List[dict], set_notified: bool
+    ) -> Tuple[List[Tuple[dict, dict]], int]:
         changed_tenders = []
         codes = {}
-        new_tender_count = 0
+        num_created_tenders = 0
 
         # self.xml_files[:] is used instead of self.xml_files because as we are
         # iterating over the list, we"re deleting it"s entries if they doesn't
@@ -406,26 +406,28 @@ def parse_notices(self, tenders, set_notified):
         for xml_file in self.xml_files[:]:
             with open(xml_file, 'r') as f:
                 try:
-                    tender, awards = self._parse_notice(
+                    tender_dict, awards = self._parse_notice(
                         f.read(), tenders, xml_file, codes, set_notified)
                 except StopIteration:
                     continue
 
-                created, attr_changes = self.save_tender(tender, codes.get(xml_file, []))
+                created, attr_changes = self.save_tender(
+                    tender_dict, codes.get(xml_file, []))
 
                 if awards:
-                    for award in awards:
-                        self.save_award(tender, award)
+                    for award_dict in awards:
+                        self.save_award(tender_dict, award_dict)
 
                 if created:
-                    new_tender_count += 1
+                    num_created_tenders += 1
 
                 if not created and attr_changes:
-                    changed_tenders.append((tender, attr_changes))
+                    changed_tenders.append((tender_dict, attr_changes))
 
             os.remove(xml_file)
 
-        return changed_tenders, new_tender_count
+        # Only the changed tender info is returned, the created ones are not
+        return changed_tenders, num_created_tenders
 
     @staticmethod
     def update_contract_award_awards(awards, soup, set_notified):
@@ -502,37 +504,38 @@ def update_contract_award_notice_awards(awards, soup, set_notified):
                     awards.append(award)
 
     @staticmethod
-    def save_award(tender_fields, award_fields):
-        reference = tender_fields['reference']
+    def save_award(tender_dict, award_dict) -> Award:
+        reference = tender_dict['reference']
         tender_entry = Tender.objects.filter(reference=reference).first()
 
         if tender_entry:
-            vendors = award_fields.pop('vendors')
+            vendors = award_dict.pop('vendors')
             vendor_objects = []
             for vendor in vendors:
                 vendor_object, _ = Vendor.objects.get_or_create(name=vendor)
                 vendor_objects.append(vendor_object)
-            award, created = Award.objects.get_or_create(tender=tender_entry, defaults=award_fields)
+            award, created = Award.objects.get_or_create(
+                tender=tender_entry, defaults=award_dict)
             if not created:
-                award.value += award_fields['value']
+                award.value += award_dict['value']
                 award.save()
             award.vendors.add(*vendor_objects)
             return award
 
     @staticmethod
-    def save_tender(tender: dict, codes: list):
-        old_tender = Tender.objects.filter(reference=tender['reference']).first()
+    def save_tender(tender_dict, codes) -> Tuple[bool, dict]:
+        old_tender = Tender.objects.filter(
+            reference=tender_dict['reference']).first()
         new_tender, created = Tender.objects.update_or_create(
-            reference=tender['reference'],
-            defaults=dict(tender, **{'cpv_codes': ', '.join(codes)}),
+            reference=tender_dict['reference'],
+            defaults=dict(tender_dict, **{'cpv_codes': ', '.join(codes)}),
         )
 
         attr_changes = {}
-        for attr, value in [(k, v) for (k, v) in tender.items()]:
+        for attr, value in [(k, v) for (k, v) in tender_dict.items()]:
             old_value = getattr(old_tender, str(attr), None)
             if str(value) != str(old_value):
                 attr_changes.update({attr: (old_value, value)})
-
         return created, attr_changes
 
 

app/parsers/ungm.py

@@ -174,7 +174,7 @@ def update_ungm_tenders(parsed_tenders):
         changed_tenders = []
         new_tenders = 0
         for item in parsed_tenders:
-            reference = item['tender'].pop('reference')
+            reference = item['tender'].get('reference')
             old_tender = Tender.objects.filter(reference=reference).first()
             new_tender, created = Tender.objects.update_or_create(
                 reference=reference, defaults=dict(item['tender']))

app/tests/test_mail.py

@@ -222,9 +222,9 @@ def test_mailing_keywords_digest(self):
 
         tender_list = soup.find('ol', {'class': 'tender-list'}).find_all('a')
         self.assertEqual(len(tender_list), 3)
-        self.assertEqual(tender_list[0]['href'], self.tender3.url)
-        self.assertEqual(tender_list[1]['href'], self.tender1.url)
-        self.assertEqual(tender_list[2]['href'], self.tender2.url)
+        email_urls = {t['href'] for t in tender_list}
+        db_urls = {self.tender1.url, self.tender2.url, self.tender3.url}
+        self.assertEqual(email_urls, db_urls)
 
     def test_deadline_notification(self):
         self.tender2 = Tender.objects.get(reference=self.tender2.reference)