From be9e6d52a392e270f788e7e6bb9b1a5f1a89ea71 Mon Sep 17 00:00:00 2001
From: NatSquared <nat.k.weiland@gmail.com>
Date: Mon, 8 Apr 2024 12:08:57 -0700
Subject: [PATCH] =?UTF-8?q?fixed=20it=20=F0=9F=98=81=F0=9F=91=8D?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
---
CHANGELOG.MD | 6 ++++
.../met_api/services/submission_service.py | 19 ++++++----
met-api/tests/unit/api/test_submission.py | 35 ++++++++++++++++++
.../tests/unit/services/test_submission.py | 36 +++++++++++++++++++
met-api/tests/utilities/factory_utils.py | 20 +++++++----
5 files changed, 104 insertions(+), 12 deletions(-)
diff --git a/CHANGELOG.MD b/CHANGELOG.MD
index 06afa75fd..bceb22198 100644
--- 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
index 9e640c862..06d48022b 100644
--- 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
index 724a55902..ecb8ce48d 100644
--- a/met-api/tests/unit/api/test_submission.py
+++ b/met-api/tests/unit/api/test_submission.py
@@ -24,7 +24,11 @@
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.engagement_status import SubmissionStatus
from met_api.constants.membership_type import MembershipType
+from met_api.schemas.comment import CommentSchema
from met_api.services.submission_service import SubmissionService
from met_api.utils.enums import ContentType
from tests.utilities.factory_scenarios import TestJwtClaims, TestSubmissionInfo
@@ -66,6 +70,37 @@ 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)
+ 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)
+ 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
index 87b82eddb..465d0a3b0 100644
--- 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,38 @@ 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()
+ 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)
+ # pretend the comment was rejected
+ submission.comment_status_id = Status.Rejected.value
+ session.flush()
+ 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
index e1a3f3801..245f1f0e7 100644
--- 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