[To Main] Bugfix: Comments do not update when editing rejected commen…

…ts [DESENG-527] (#2452) * fixed it 😁👍 * Cleaning out my dryer (fix lint errors) * Improvement: Reject the comment using SubmissionService * Remove unused imports (silly me)
bcgov · Apr 8, 2024 · 13729fb · 13729fb
1 parent 25e396b
commit 13729fb
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 12 deletions.
diff --git a/CHANGELOG.MD b/CHANGELOG.MD
@@ -1,3 +1,9 @@
+## April 08, 2024
+
+- **Bugfix**: Submission of rejected comments [🎟️ DESENG-527](https://apps.itsm.gov.bc.ca/jira/browse/DESENG-527)
+  - Fixed issue where resubmitted comments would not reappear in the queue for approval.
+  - Added unit tests to ensure the issue does not reoccur.
+
 ## April 05, 2024
 
 - **Task**: Allow for any gov.bc.ca email to receive emails in MET's dev & test environments [DESENG-533](https://apps.itsm.gov.bc.ca/jira/browse/DESENG-533)

diff --git a/met-api/src/met_api/services/submission_service.py b/met-api/src/met_api/services/submission_service.py
@@ -62,7 +62,8 @@ def _check_comment_auth(cls, submission):
             MembershipType.TEAM_MEMBER.name,
             Role.REVIEW_COMMENTS.value
         )
-        authorization.check_auth(one_of_roles=one_of_roles, engagement_id=engagement.id)
+        authorization.check_auth(
+            one_of_roles=one_of_roles, engagement_id=engagement.id)
 
     @classmethod
     def get_by_token(cls, token):
@@ -101,7 +102,8 @@ def create(cls, token, submission: SubmissionSchema):
             EngagementSettingsModel.find_by_id(engagement_id)
         if engagement_settings:
             if engagement_settings.send_report:
-                SubmissionService._send_submission_response_email(participant_id, engagement_id)
+                SubmissionService._send_submission_response_email(
+                    participant_id, engagement_id)
         return submission_result
 
     @classmethod
@@ -127,6 +129,7 @@ def update_comments(cls, token, data: PublicSubmissionSchema):
         comments_result = [Comment.update(submission.id, comment, db.session)
                            for comment in data.get('comments', [])]
         SubmissionModel.update(SubmissionSchema().dump(submission), db.session)
+        db.session.commit()
         return comments_result
 
     @staticmethod
@@ -350,7 +353,8 @@ def _render_email_template(staff_review_details: dict, submission: SubmissionMod
         templates = current_app.config.get('EMAIL_TEMPLATES')
         if engagement.status_id == EngagementStatus.Closed.value:
             template_id = templates['CLOSED_ENGAGEMENT_REJECTED']['ID']
-            template = Template.get_template('email_rejected_comment_closed.html')
+            template = Template.get_template(
+                'email_rejected_comment_closed.html')
             subject = templates['CLOSED_ENGAGEMENT_REJECTED']['SUBJECT']. \
                 format(engagement_name=engagement_name)
         else:
@@ -401,7 +405,8 @@ def _send_submission_response_email(participant_id, engagement_id) -> None:
         participant = ParticipantModel.find_by_id(participant_id)
         templates = current_app.config['EMAIL_TEMPLATES']
         template_id = templates['SUBMISSION_RESPONSE']['ID']
-        subject, body, args = SubmissionService._render_submission_response_email_template(engagement_id)
+        subject, body, args = SubmissionService._render_submission_response_email_template(
+            engagement_id)
         try:
             notification.send_email(subject=subject,
                                     email=ParticipantModel.decode_email(
@@ -423,7 +428,8 @@ def _render_submission_response_email_template(engagement_id):
         template = Template.get_template('submission_response.html')
         subject = templates['SUBMISSION_RESPONSE']['SUBJECT']
         dashboard_path = SubmissionService._get_dashboard_path(engagement)
-        engagement_url = notification.get_tenant_site_url(engagement.tenant_id, dashboard_path)
+        engagement_url = notification.get_tenant_site_url(
+            engagement.tenant_id, dashboard_path)
         email_environment = templates['ENVIRONMENT']
         tenant_name = SubmissionService._get_tenant_name(
             engagement.tenant_id)
@@ -445,7 +451,8 @@ def _render_submission_response_email_template(engagement_id):
 
     @staticmethod
     def _get_dashboard_path(engagement: EngagementModel):
-        engagement_slug = EngagementSlugModel.find_by_engagement_id(engagement.id)
+        engagement_slug = EngagementSlugModel.find_by_engagement_id(
+            engagement.id)
         paths = current_app.config['PATH_CONFIG']
         if engagement_slug:
             return paths['ENGAGEMENT']['DASHBOARD_SLUG'].format(

diff --git a/met-api/tests/unit/api/test_submission.py b/met-api/tests/unit/api/test_submission.py
@@ -24,6 +24,8 @@
 import pytest
 from faker import Faker
 
+from met_api.constants.comment_status import Status
+from met_api.constants.email_verification import EmailVerificationType
 from met_api.constants.membership_type import MembershipType
 from met_api.services.submission_service import SubmissionService
 from met_api.utils.enums import ContentType
@@ -66,6 +68,36 @@ def test_valid_submission(client, jwt, session, side_effect, expected_status):
     assert rv.status_code == expected_status
 
 
+def test_edit_rejected_submission(client, jwt, session):  # pylint:disable=unused-argument
+    """Assert that an engagement can updated after rejection."""
+    claims = TestJwtClaims.public_user_role
+
+    survey, eng = factory_survey_and_eng_model()
+    email_verification = factory_email_verification(survey.id)
+    participant = factory_participant_model()
+    factory_engagement_setting_model(eng.id)
+    submission_request = {
+        'submission_json': {'test_question': 'test answer'},
+        'engagement_id': eng.id,
+        'survey_id': survey.id,
+        'participant_id': participant.id,
+        'verification_token': email_verification.verification_token,
+    }
+    submission = factory_submission_model(
+        survey.id, eng.id, participant.id, TestSubmissionInfo.rejected_submission)
+    email_verification = factory_email_verification(
+        survey.id, type=EmailVerificationType.RejectedComment, submission_id=submission.id)
+    headers = factory_auth_header(jwt=jwt, claims=claims)
+    rv = client.put(f'/api/submissions/public/{email_verification.verification_token}',
+                    data=json.dumps(submission_request),
+                    headers=headers, content_type=ContentType.JSON.value)
+    assert rv.status_code == 200
+    # roll back any pending changes to make sure the session is clean
+    # the changes should still be committed because the submission was updated
+    session.rollback()
+    assert submission.comment_status_id == Status.Pending.value
+
+
 @pytest.mark.parametrize('submission_info', [TestSubmissionInfo.submission1])
 def test_get_submission_by_id(client, jwt, session, submission_info,
                               setup_admin_user_and_claims):  # pylint:disable=unused-argument

diff --git a/met-api/tests/unit/services/test_submission.py b/met-api/tests/unit/services/test_submission.py
@@ -15,8 +15,12 @@
 
 Test suite to ensure that the Submission service routines are working as expected.
 """
+from typing import List
 from unittest.mock import patch
 
+from met_api.constants.email_verification import EmailVerificationType
+from met_api.models.comment import Comment
+from met_api.schemas.comment import CommentSchema
 from met_api.services import authorization
 from met_api.constants.comment_status import Status
 from met_api.schemas.submission import SubmissionSchema
@@ -50,6 +54,47 @@ def test_create_submission(session):  # pylint:disable=unused-argument
     assert actual_email_verification['is_active'] is False
 
 
+def test_update_submission(session):  # pylint:disable=unused-argument
+    """Assert that a submission can be updated using update_comments."""
+    survey, eng = factory_survey_and_eng_model()
+    staff_user = factory_staff_user_model(3)
+    email_verification = factory_email_verification(survey.id)
+    participant = factory_participant_model()
+    factory_engagement_setting_model(eng.id)
+    submission_request: SubmissionSchema = {
+        'submission_json': {'test_question': 'test answer'},
+        'engagement_id': eng.id,
+        'survey_id': survey.id,
+        'participant_id': participant.id,
+        'verification_token': email_verification.verification_token,
+    }
+    submission = SubmissionService().create(
+        email_verification.verification_token, submission_request)
+    # reject the commnent
+    staff_review_details = {
+        'status_id': Status.Rejected.value,
+        'has_personal_info': True,  # the reason for rejection
+        'notify_email': False,
+    }
+    with patch.object(authorization, 'check_auth', return_value=True):
+        submission_record = SubmissionService().review_comment(
+            submission.id, staff_review_details, staff_user.external_id)
+        assert submission_record.get(
+            'comment_status_id') == Status.Rejected.value
+    assert submission.comment_status_id == Status.Rejected.value
+    comment: CommentSchema = CommentSchema().dump(
+        factory_comment_model(survey.id, submission.id))
+    email_verification = factory_email_verification(
+        survey.id, type=EmailVerificationType.RejectedComment, submission_id=submission.id)
+    updated_submission: List[Comment] = SubmissionService().update_comments(
+        email_verification.verification_token, {'comments': [comment]})
+    # Roll back any changes still in the transaction to make sure data was
+    # committed by the service
+    session.rollback()
+    assert updated_submission[0].text == comment['text']
+    assert submission.comment_status_id == Status.Pending.value
+
+
 def test_create_submission_rollback(session):  # pylint:disable=unused-argument
     """Assert that a submission failure will rollback changes to email verification."""
     survey, _ = factory_survey_and_eng_model()

diff --git a/met-api/tests/utilities/factory_utils.py b/met-api/tests/utilities/factory_utils.py
@@ -138,7 +138,7 @@ def factory_subscription_model():
     return subscription
 
 
-def factory_email_verification(survey_id, type=None):
+def factory_email_verification(survey_id, type=None, submission_id=None):
     """Produce a EmailVerification model."""
     email_verification = EmailVerificationModel(
         verification_token=fake.uuid4(),
@@ -152,6 +152,9 @@ def factory_email_verification(survey_id, type=None):
     if survey_id:
         email_verification.survey_id = survey_id
 
+    if submission_id:
+        email_verification.submission_id = submission_id
+
     email_verification.save()
     return email_verification
 
@@ -205,7 +208,8 @@ def factory_metadata_requirements(auth: Optional[Auth] = None):
     """Create a tenant, an associated staff user, and engagement, for tests."""
     tenant = factory_tenant_model()
     tenant.short_name = fake.lexify(text='????').upper()
-    (engagement_info := TestEngagementInfo.engagement1.copy())['tenant_id'] = tenant.id
+    (engagement_info := TestEngagementInfo.engagement1.copy())[
+        'tenant_id'] = tenant.id
     engagement = factory_engagement_model(engagement_info)
     (staff_info := TestUserInfo.user_staff_1.copy())['tenant_id'] = tenant.id
     factory_staff_user_model(TestJwtClaims.staff_admin_role['sub'], staff_info)
@@ -225,7 +229,8 @@ def factory_taxon_requirements(auth: Optional[Auth] = None):
     tenant = factory_tenant_model()
     tenant.short_name = fake.lexify(text='????').upper()
     (staff_info := TestUserInfo.user_staff_1.copy())['tenant_id'] = tenant.id
-    factory_staff_user_model(TestJwtClaims.staff_admin_role.get('sub'), staff_info)
+    factory_staff_user_model(
+        TestJwtClaims.staff_admin_role.get('sub'), staff_info)
     if auth:
         headers = factory_auth_header(
             auth,
@@ -276,7 +281,8 @@ def factory_participant_model(
 ):
     """Produce a participant model."""
     participant = ParticipantModel(
-        email_address=ParticipantModel.encode_email(participant['email_address']),
+        email_address=ParticipantModel.encode_email(
+            participant['email_address']),
     )
     participant.save()
     return participant
@@ -414,7 +420,8 @@ def token_info():
         """Return token info."""
         return claims
 
-    monkeypatch.setattr('met_api.utils.user_context._get_token_info', token_info)
+    monkeypatch.setattr(
+        'met_api.utils.user_context._get_token_info', token_info)
 
     # Add a database user that matches the token
     # factory_staff_user_model(external_id=claims.get('sub'))
@@ -474,7 +481,8 @@ def factory_poll_model(widget, poll_info: dict = TestWidgetPollInfo.poll1):
 
 def factory_poll_answer_model(poll, answer_info: dict = TestPollAnswerInfo.answer1):
     """Produce a Poll  model."""
-    answer = PollAnswerModel(answer_text=answer_info.get('answer_text'), poll_id=poll.id)
+    answer = PollAnswerModel(answer_text=answer_info.get(
+        'answer_text'), poll_id=poll.id)
     answer.save()
     return answer