From bc97b3a116a506d329deec06b875cf9c889b5bee Mon Sep 17 00:00:00 2001 From: Kenroy Gobourne Date: Tue, 23 Jul 2024 11:53:39 -0500 Subject: [PATCH 1/3] fix: Failing unit tests (M2-7414) (#1505) This PR fixes the broken unit tests from #1494, and adds some tests that were missing from that PR. --- src/apps/shared/subjects.py | 5 +- src/apps/subjects/api.py | 4 +- src/apps/subjects/tests/tests.py | 85 +++++++++++++++++++++++++++++++- 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/src/apps/shared/subjects.py b/src/apps/shared/subjects.py index 93583124ac2..d3efcd5d86d 100644 --- a/src/apps/shared/subjects.py +++ b/src/apps/shared/subjects.py @@ -11,7 +11,8 @@ def is_valid_take_now_relation(relation: SubjectRelation | None) -> bool: if is_take_now_relation(relation): assert isinstance(relation, SubjectRelation) assert isinstance(relation.meta, dict) - expires_at = datetime.fromisoformat(relation.meta["expiresAt"]) - return expires_at > datetime.now() + if "expiresAt" in relation.meta: + expires_at = datetime.fromisoformat(relation.meta["expiresAt"]) + return expires_at > datetime.now() return False diff --git a/src/apps/subjects/api.py b/src/apps/subjects/api.py index a22294dfa66..6fc54fc04b0 100644 --- a/src/apps/subjects/api.py +++ b/src/apps/subjects/api.py @@ -232,7 +232,9 @@ async def get_subject( raise NotFoundError() user_subject = await subjects_service.get_by_user_and_applet(user.id, subject.applet_id) - if user_subject: + if not user_subject: + await CheckAccessService(session, user.id).check_subject_subject_access(subject.applet_id, subject_id) + else: relation = await subjects_service.get_relation(user_subject.id, subject_id) has_relation = relation is not None and ( relation.relation != "take-now" or is_valid_take_now_relation(relation) diff --git a/src/apps/subjects/tests/tests.py b/src/apps/subjects/tests/tests.py index da2244e3c5c..d4b57b42eaf 100644 --- a/src/apps/subjects/tests/tests.py +++ b/src/apps/subjects/tests/tests.py @@ -2,15 +2,19 @@ import json import uuid from copy import copy +from datetime import datetime, timedelta import pytest from asyncpg import UniqueViolationError +from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from apps.answers.crud.answers import AnswersCRUD from apps.applets.domain.applet_full import AppletFull from apps.shared.test import BaseTest +from apps.shared.test.client import TestClient from apps.subjects.crud import SubjectsCrud +from apps.subjects.db.schemas import SubjectSchema from apps.subjects.domain import Subject, SubjectCreate, SubjectCreateRequest, SubjectRelationCreate from apps.subjects.services import SubjectsService from apps.users import User @@ -197,6 +201,16 @@ async def applet_one_lucy_respondent(session: AsyncSession, applet_one: AppletFu return applet_one +@pytest.fixture +async def lucy_applet_one_subject(session: AsyncSession, lucy: User, applet_one_lucy_respondent: AppletFull) -> Subject: + applet_id = applet_one_lucy_respondent.id + user_id = lucy.id + query = select(SubjectSchema).where(SubjectSchema.user_id == user_id, SubjectSchema.applet_id == applet_id) + res = await session.execute(query, execution_options={"synchronize_session": False}) + model = res.scalars().one() + return Subject.from_orm(model) + + @pytest.fixture async def applet_one_lucy_reviewer_with_subject( session: AsyncSession, applet_one: AppletFull, tom_applet_one_subject, tom, lucy @@ -215,13 +229,13 @@ async def tom_invitation_payload(tom: User) -> dict: @pytest.fixture -async def lucy_invitation_payload(lucy: User) -> dict: +async def lucy_participant_invitation_payload(lucy: User) -> dict: return dict( email=lucy.email_encrypted, first_name=lucy.first_name, last_name=lucy.last_name, language="en", - role=Role.MANAGER, + role=Role.RESPONDENT, ) @@ -543,6 +557,73 @@ async def test_get_subject_full(self, client, tom: User, tom_applet_one_subject: response = await client.get(self.subject_detail_url.format(subject_id=subject_id)) assert response.status_code == http.HTTPStatus.FORBIDDEN + async def test_get_subject_related_participant( + self, + session: AsyncSession, + client: TestClient, + tom: User, + lucy: User, + tom_applet_one_subject: Subject, + lucy_applet_one_subject: Subject, + applet_one_shell_account: Subject, + ): + client.login(lucy) + response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) + assert response.status_code == http.HTTPStatus.FORBIDDEN + + await SubjectsService(session, tom.id).create_relation( + applet_one_shell_account.id, lucy_applet_one_subject.id, "parent" + ) + + response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) + assert response.status_code == http.HTTPStatus.OK + + async def test_get_subject_temp_related_participant( + self, + session: AsyncSession, + client: TestClient, + tom: User, + lucy: User, + tom_applet_one_subject: Subject, + lucy_applet_one_subject: Subject, + applet_one_shell_account: Subject, + ): + client.login(lucy) + response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) + assert response.status_code == http.HTTPStatus.FORBIDDEN + + # Expires tomorrow + expires_at = datetime.now() + timedelta(days=1) + await SubjectsService(session, tom.id).create_relation( + applet_one_shell_account.id, lucy_applet_one_subject.id, "take-now", {"expiresAt": expires_at.isoformat()} + ) + + response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) + assert response.status_code == http.HTTPStatus.OK + + async def test_get_subject_expired_temp_related_participant( + self, + session: AsyncSession, + client: TestClient, + tom: User, + lucy: User, + tom_applet_one_subject: Subject, + lucy_applet_one_subject: Subject, + applet_one_shell_account: Subject, + ): + client.login(lucy) + response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) + assert response.status_code == http.HTTPStatus.FORBIDDEN + + # Expired yesterday + expires_at = datetime.now() - timedelta(days=1) + await SubjectsService(session, tom.id).create_relation( + applet_one_shell_account.id, lucy_applet_one_subject.id, "take-now", {"expiresAt": expires_at.isoformat()} + ) + + response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) + assert response.status_code == http.HTTPStatus.FORBIDDEN + async def test_get_my_subject(self, client, tom: User, tom_applet_one_subject: Subject): client.login(tom) response = await client.get(self.my_subject_url.format(applet_id=tom_applet_one_subject.applet_id)) From e739857a041937e391b4ab18f680e4f823d8a088 Mon Sep 17 00:00:00 2001 From: Marty <2614025+mbanting@users.noreply.github.com> Date: Wed, 24 Jul 2024 12:14:52 -0700 Subject: [PATCH 2/3] Revert "fix: Failing unit tests (M2-7414) (#1505)" This reverts commit bc97b3a116a506d329deec06b875cf9c889b5bee. --- src/apps/shared/subjects.py | 5 +- src/apps/subjects/api.py | 4 +- src/apps/subjects/tests/tests.py | 85 +------------------------------- 3 files changed, 5 insertions(+), 89 deletions(-) diff --git a/src/apps/shared/subjects.py b/src/apps/shared/subjects.py index d3efcd5d86d..93583124ac2 100644 --- a/src/apps/shared/subjects.py +++ b/src/apps/shared/subjects.py @@ -11,8 +11,7 @@ def is_valid_take_now_relation(relation: SubjectRelation | None) -> bool: if is_take_now_relation(relation): assert isinstance(relation, SubjectRelation) assert isinstance(relation.meta, dict) - if "expiresAt" in relation.meta: - expires_at = datetime.fromisoformat(relation.meta["expiresAt"]) - return expires_at > datetime.now() + expires_at = datetime.fromisoformat(relation.meta["expiresAt"]) + return expires_at > datetime.now() return False diff --git a/src/apps/subjects/api.py b/src/apps/subjects/api.py index 6fc54fc04b0..a22294dfa66 100644 --- a/src/apps/subjects/api.py +++ b/src/apps/subjects/api.py @@ -232,9 +232,7 @@ async def get_subject( raise NotFoundError() user_subject = await subjects_service.get_by_user_and_applet(user.id, subject.applet_id) - if not user_subject: - await CheckAccessService(session, user.id).check_subject_subject_access(subject.applet_id, subject_id) - else: + if user_subject: relation = await subjects_service.get_relation(user_subject.id, subject_id) has_relation = relation is not None and ( relation.relation != "take-now" or is_valid_take_now_relation(relation) diff --git a/src/apps/subjects/tests/tests.py b/src/apps/subjects/tests/tests.py index d4b57b42eaf..da2244e3c5c 100644 --- a/src/apps/subjects/tests/tests.py +++ b/src/apps/subjects/tests/tests.py @@ -2,19 +2,15 @@ import json import uuid from copy import copy -from datetime import datetime, timedelta import pytest from asyncpg import UniqueViolationError -from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from apps.answers.crud.answers import AnswersCRUD from apps.applets.domain.applet_full import AppletFull from apps.shared.test import BaseTest -from apps.shared.test.client import TestClient from apps.subjects.crud import SubjectsCrud -from apps.subjects.db.schemas import SubjectSchema from apps.subjects.domain import Subject, SubjectCreate, SubjectCreateRequest, SubjectRelationCreate from apps.subjects.services import SubjectsService from apps.users import User @@ -201,16 +197,6 @@ async def applet_one_lucy_respondent(session: AsyncSession, applet_one: AppletFu return applet_one -@pytest.fixture -async def lucy_applet_one_subject(session: AsyncSession, lucy: User, applet_one_lucy_respondent: AppletFull) -> Subject: - applet_id = applet_one_lucy_respondent.id - user_id = lucy.id - query = select(SubjectSchema).where(SubjectSchema.user_id == user_id, SubjectSchema.applet_id == applet_id) - res = await session.execute(query, execution_options={"synchronize_session": False}) - model = res.scalars().one() - return Subject.from_orm(model) - - @pytest.fixture async def applet_one_lucy_reviewer_with_subject( session: AsyncSession, applet_one: AppletFull, tom_applet_one_subject, tom, lucy @@ -229,13 +215,13 @@ async def tom_invitation_payload(tom: User) -> dict: @pytest.fixture -async def lucy_participant_invitation_payload(lucy: User) -> dict: +async def lucy_invitation_payload(lucy: User) -> dict: return dict( email=lucy.email_encrypted, first_name=lucy.first_name, last_name=lucy.last_name, language="en", - role=Role.RESPONDENT, + role=Role.MANAGER, ) @@ -557,73 +543,6 @@ async def test_get_subject_full(self, client, tom: User, tom_applet_one_subject: response = await client.get(self.subject_detail_url.format(subject_id=subject_id)) assert response.status_code == http.HTTPStatus.FORBIDDEN - async def test_get_subject_related_participant( - self, - session: AsyncSession, - client: TestClient, - tom: User, - lucy: User, - tom_applet_one_subject: Subject, - lucy_applet_one_subject: Subject, - applet_one_shell_account: Subject, - ): - client.login(lucy) - response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) - assert response.status_code == http.HTTPStatus.FORBIDDEN - - await SubjectsService(session, tom.id).create_relation( - applet_one_shell_account.id, lucy_applet_one_subject.id, "parent" - ) - - response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) - assert response.status_code == http.HTTPStatus.OK - - async def test_get_subject_temp_related_participant( - self, - session: AsyncSession, - client: TestClient, - tom: User, - lucy: User, - tom_applet_one_subject: Subject, - lucy_applet_one_subject: Subject, - applet_one_shell_account: Subject, - ): - client.login(lucy) - response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) - assert response.status_code == http.HTTPStatus.FORBIDDEN - - # Expires tomorrow - expires_at = datetime.now() + timedelta(days=1) - await SubjectsService(session, tom.id).create_relation( - applet_one_shell_account.id, lucy_applet_one_subject.id, "take-now", {"expiresAt": expires_at.isoformat()} - ) - - response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) - assert response.status_code == http.HTTPStatus.OK - - async def test_get_subject_expired_temp_related_participant( - self, - session: AsyncSession, - client: TestClient, - tom: User, - lucy: User, - tom_applet_one_subject: Subject, - lucy_applet_one_subject: Subject, - applet_one_shell_account: Subject, - ): - client.login(lucy) - response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) - assert response.status_code == http.HTTPStatus.FORBIDDEN - - # Expired yesterday - expires_at = datetime.now() - timedelta(days=1) - await SubjectsService(session, tom.id).create_relation( - applet_one_shell_account.id, lucy_applet_one_subject.id, "take-now", {"expiresAt": expires_at.isoformat()} - ) - - response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) - assert response.status_code == http.HTTPStatus.FORBIDDEN - async def test_get_my_subject(self, client, tom: User, tom_applet_one_subject: Subject): client.login(tom) response = await client.get(self.my_subject_url.format(applet_id=tom_applet_one_subject.applet_id)) From 55c08c79d8a0fe64248a7794e6f10e1d2a6af688 Mon Sep 17 00:00:00 2001 From: Kenroy Gobourne Date: Tue, 23 Jul 2024 11:53:39 -0500 Subject: [PATCH 3/3] fix: Failing unit tests (M2-7414) (#1505) This PR fixes the broken unit tests from #1494, and adds some tests that were missing from that PR. --- src/apps/shared/subjects.py | 5 +- src/apps/subjects/api.py | 4 +- src/apps/subjects/tests/tests.py | 85 +++++++++++++++++++++++++++++++- 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/src/apps/shared/subjects.py b/src/apps/shared/subjects.py index 93583124ac2..d3efcd5d86d 100644 --- a/src/apps/shared/subjects.py +++ b/src/apps/shared/subjects.py @@ -11,7 +11,8 @@ def is_valid_take_now_relation(relation: SubjectRelation | None) -> bool: if is_take_now_relation(relation): assert isinstance(relation, SubjectRelation) assert isinstance(relation.meta, dict) - expires_at = datetime.fromisoformat(relation.meta["expiresAt"]) - return expires_at > datetime.now() + if "expiresAt" in relation.meta: + expires_at = datetime.fromisoformat(relation.meta["expiresAt"]) + return expires_at > datetime.now() return False diff --git a/src/apps/subjects/api.py b/src/apps/subjects/api.py index a22294dfa66..6fc54fc04b0 100644 --- a/src/apps/subjects/api.py +++ b/src/apps/subjects/api.py @@ -232,7 +232,9 @@ async def get_subject( raise NotFoundError() user_subject = await subjects_service.get_by_user_and_applet(user.id, subject.applet_id) - if user_subject: + if not user_subject: + await CheckAccessService(session, user.id).check_subject_subject_access(subject.applet_id, subject_id) + else: relation = await subjects_service.get_relation(user_subject.id, subject_id) has_relation = relation is not None and ( relation.relation != "take-now" or is_valid_take_now_relation(relation) diff --git a/src/apps/subjects/tests/tests.py b/src/apps/subjects/tests/tests.py index da2244e3c5c..d4b57b42eaf 100644 --- a/src/apps/subjects/tests/tests.py +++ b/src/apps/subjects/tests/tests.py @@ -2,15 +2,19 @@ import json import uuid from copy import copy +from datetime import datetime, timedelta import pytest from asyncpg import UniqueViolationError +from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from apps.answers.crud.answers import AnswersCRUD from apps.applets.domain.applet_full import AppletFull from apps.shared.test import BaseTest +from apps.shared.test.client import TestClient from apps.subjects.crud import SubjectsCrud +from apps.subjects.db.schemas import SubjectSchema from apps.subjects.domain import Subject, SubjectCreate, SubjectCreateRequest, SubjectRelationCreate from apps.subjects.services import SubjectsService from apps.users import User @@ -197,6 +201,16 @@ async def applet_one_lucy_respondent(session: AsyncSession, applet_one: AppletFu return applet_one +@pytest.fixture +async def lucy_applet_one_subject(session: AsyncSession, lucy: User, applet_one_lucy_respondent: AppletFull) -> Subject: + applet_id = applet_one_lucy_respondent.id + user_id = lucy.id + query = select(SubjectSchema).where(SubjectSchema.user_id == user_id, SubjectSchema.applet_id == applet_id) + res = await session.execute(query, execution_options={"synchronize_session": False}) + model = res.scalars().one() + return Subject.from_orm(model) + + @pytest.fixture async def applet_one_lucy_reviewer_with_subject( session: AsyncSession, applet_one: AppletFull, tom_applet_one_subject, tom, lucy @@ -215,13 +229,13 @@ async def tom_invitation_payload(tom: User) -> dict: @pytest.fixture -async def lucy_invitation_payload(lucy: User) -> dict: +async def lucy_participant_invitation_payload(lucy: User) -> dict: return dict( email=lucy.email_encrypted, first_name=lucy.first_name, last_name=lucy.last_name, language="en", - role=Role.MANAGER, + role=Role.RESPONDENT, ) @@ -543,6 +557,73 @@ async def test_get_subject_full(self, client, tom: User, tom_applet_one_subject: response = await client.get(self.subject_detail_url.format(subject_id=subject_id)) assert response.status_code == http.HTTPStatus.FORBIDDEN + async def test_get_subject_related_participant( + self, + session: AsyncSession, + client: TestClient, + tom: User, + lucy: User, + tom_applet_one_subject: Subject, + lucy_applet_one_subject: Subject, + applet_one_shell_account: Subject, + ): + client.login(lucy) + response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) + assert response.status_code == http.HTTPStatus.FORBIDDEN + + await SubjectsService(session, tom.id).create_relation( + applet_one_shell_account.id, lucy_applet_one_subject.id, "parent" + ) + + response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) + assert response.status_code == http.HTTPStatus.OK + + async def test_get_subject_temp_related_participant( + self, + session: AsyncSession, + client: TestClient, + tom: User, + lucy: User, + tom_applet_one_subject: Subject, + lucy_applet_one_subject: Subject, + applet_one_shell_account: Subject, + ): + client.login(lucy) + response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) + assert response.status_code == http.HTTPStatus.FORBIDDEN + + # Expires tomorrow + expires_at = datetime.now() + timedelta(days=1) + await SubjectsService(session, tom.id).create_relation( + applet_one_shell_account.id, lucy_applet_one_subject.id, "take-now", {"expiresAt": expires_at.isoformat()} + ) + + response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) + assert response.status_code == http.HTTPStatus.OK + + async def test_get_subject_expired_temp_related_participant( + self, + session: AsyncSession, + client: TestClient, + tom: User, + lucy: User, + tom_applet_one_subject: Subject, + lucy_applet_one_subject: Subject, + applet_one_shell_account: Subject, + ): + client.login(lucy) + response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) + assert response.status_code == http.HTTPStatus.FORBIDDEN + + # Expired yesterday + expires_at = datetime.now() - timedelta(days=1) + await SubjectsService(session, tom.id).create_relation( + applet_one_shell_account.id, lucy_applet_one_subject.id, "take-now", {"expiresAt": expires_at.isoformat()} + ) + + response = await client.get(self.subject_detail_url.format(subject_id=applet_one_shell_account.id)) + assert response.status_code == http.HTTPStatus.FORBIDDEN + async def test_get_my_subject(self, client, tom: User, tom_applet_one_subject: Subject): client.login(tom) response = await client.get(self.my_subject_url.format(applet_id=tom_applet_one_subject.applet_id))