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..2969c1b28 100644 --- 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 index 87b82eddb..961bf5742 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,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 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