From a08315c9707f2fd5d8faf8fa06675248c5b90327 Mon Sep 17 00:00:00 2001
From: Erin Mullaney <erin.mullaney@torchbox.com>
Date: Fri, 8 Feb 2019 08:54:52 -0500
Subject: [PATCH] only display updated role reviewers if they changed on
 messaging

---
 opentech/apply/activity/messaging.py | 16 ++++++++--------
 opentech/apply/funds/views.py        | 11 ++++++++---
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/opentech/apply/activity/messaging.py b/opentech/apply/activity/messaging.py
index 0711b809f..344a19914 100644
--- a/opentech/apply/activity/messaging.py
+++ b/opentech/apply/activity/messaging.py
@@ -188,7 +188,7 @@ class ActivityAdapter(AdapterBase):
             return {'visibility': INTERNAL}
         return {}
 
-    def reviewers_updated(self, users_with_roles=list(), added_external=list(), removed_external=list(), **kwargs):
+    def reviewers_updated(self, updated_reviewers_with_roles=list(), added_external=list(), removed_external=list(), **kwargs):
         message = ['Reviewers updated.']
         if added_external:
             message.append('Added:')
@@ -198,9 +198,9 @@ class ActivityAdapter(AdapterBase):
             message.append('Removed:')
             message.append(', '.join([str(reviewer.reviewer) for reviewer in removed_external]) + '.')
 
-        if users_with_roles:
-            for user_role in users_with_roles:
-                message.append(f"{str(user_role['user'])} added as {str(user_role['role'])}. ")
+        if updated_reviewers_with_roles:
+            for reviewer in updated_reviewers_with_roles:
+                message.append(f"{str(reviewer.reviewer)} added as {str(reviewer.role)}. ")
 
         return ' '.join(message)
 
@@ -312,7 +312,7 @@ class SlackAdapter(AdapterBase):
             } for lead in leads
         ]
 
-    def reviewers_updated(self, submission, link, user, users_with_roles=list(), added_external=list(), removed_external=list(), **kwargs):
+    def reviewers_updated(self, submission, link, user, updated_reviewers_with_roles=list(), added_external=list(), removed_external=list(), **kwargs):
         message = f'{user} has updated the reviewers on <{link}|{submission.title}>. '
 
         if added_external:
@@ -321,9 +321,9 @@ class SlackAdapter(AdapterBase):
         if removed_external:
             message += 'Reviewers removed: ' + ', '.join([str(reviewer.reviewer) for reviewer in removed_external]) + '. '
 
-        if users_with_roles:
-            for user_role in users_with_roles:
-                message += f"{str(user_role['user'])} added as {str(user_role['role'])}. "
+        if updated_reviewers_with_roles:
+            for reviewer in updated_reviewers_with_roles:
+                message += f"{str(reviewer.reviewer)} added as {str(reviewer.role)}. "
 
         return message
 
diff --git a/opentech/apply/funds/views.py b/opentech/apply/funds/views.py
index 4a08d6765..237cb53a2 100644
--- a/opentech/apply/funds/views.py
+++ b/opentech/apply/funds/views.py
@@ -282,9 +282,10 @@ class UpdateReviewersView(DelegatedViewMixin, UpdateView):
 
     def form_valid(self, form):
         old_reviewers_external = set(AssignedReviewers.objects.filter(submission=self.get_object(), role__isnull=True))
+        old_reviewers_role_dict = list(AssignedReviewers.objects.filter(submission=self.get_object(), role__isnull=False).values('role', 'reviewer'))
+        updated_reviewers_with_roles = []
         response = super().form_valid(form)
 
-        users_with_roles = []
         # Save role reviewers ONLY, we saved others in UpdateReviewersForm.save()
         form.cleaned_data.pop('reviewer_reviewers')
         for key, user in form.cleaned_data.items():
@@ -292,7 +293,11 @@ class UpdateReviewersView(DelegatedViewMixin, UpdateView):
             # Create the reviewer/role association to submission if it doesn't exist, or update it
             obj, created = AssignedReviewers.objects.update_or_create(
                 submission=form.instance, role=role, defaults={'reviewer': user})
-            users_with_roles.append({'user': user, 'role': role})
+            idx = next((index for (index, d) in enumerate(old_reviewers_role_dict) if d["role"] == role.pk), None)
+            if idx < 0:  # New assigned reviewer/role, where one didn't exist before
+                updated_reviewers_with_roles.append(obj)
+            elif old_reviewers_role_dict[idx]['reviewer'] != obj.reviewer.pk:  # Updated assigned reviewer/role
+                updated_reviewers_with_roles.append(obj)
 
         new_reviewers_external = set(AssignedReviewers.objects.filter(submission=form.instance, role__isnull=True))
         added_external = new_reviewers_external - old_reviewers_external
@@ -303,7 +308,7 @@ class UpdateReviewersView(DelegatedViewMixin, UpdateView):
             request=self.request,
             user=self.request.user,
             submission=self.kwargs['submission'],
-            users_with_roles=users_with_roles,
+            updated_reviewers_with_roles=updated_reviewers_with_roles,
             added_external=added_external,
             removed_external=removed_external,
         )
-- 
GitLab