diff --git a/src/apps/activities/api/activities.py b/src/apps/activities/api/activities.py index c8e7495f46e..193de76c11e 100644 --- a/src/apps/activities/api/activities.py +++ b/src/apps/activities/api/activities.py @@ -26,7 +26,6 @@ from apps.applets.service import AppletService from apps.authentication.deps import get_current_user from apps.shared.domain import Response, ResponseMulti -from apps.shared.exception import ValidationError from apps.shared.query_params import QueryParams, parse_query_params from apps.subjects.services import SubjectsService from apps.users import User @@ -190,7 +189,10 @@ async def applet_activities_for_target_subject( ).get_activity_and_flow_ids_by_target_subject(subject_id) activities_and_flows = await ActivityService(session, user.id).get_activity_and_flow_basic_info_by_ids_or_auto( - applet_id, activity_and_flow_ids_from_submissions + activity_and_flow_ids_from_assignments, language + applet_id=applet_id, + ids=activity_and_flow_ids_from_submissions + activity_and_flow_ids_from_assignments, + include_auto=True, + language=language, ) result = [] @@ -201,7 +203,7 @@ async def applet_activities_for_target_subject( if assignment.activity_id == activity_or_flow.id or assignment.activity_flow_id == activity_or_flow.id ] - activity_or_flow.set_status(activity_or_flow_assignments) + activity_or_flow.set_status(assignments=activity_or_flow_assignments, include_auto=True) result.append( ActivityOrFlowWithAssignmentsPublic( @@ -228,11 +230,7 @@ async def applet_activities_for_respondent_subject( await applet_service.exist_by_id(applet_id) subject = await SubjectsService(session, user.id).exist_by_id(subject_id) - - # Ensure the respondent is not a limited account - if subject.user_id is None: - # Return a generic bad request error to avoid leaking information - raise ValidationError(f"Subject {subject_id} is not a valid respondent") + is_limited_respondent = subject.user_id is None # Restrict the endpoint access to owners, managers, coordinators, and assigned reviewers await CheckAccessService(session, user.id).check_subject_subject_access(applet_id, subject_id) @@ -252,7 +250,10 @@ async def applet_activities_for_respondent_subject( ).get_activity_and_flow_ids_by_source_subject(subject_id) activities_and_flows = await ActivityService(session, user.id).get_activity_and_flow_basic_info_by_ids_or_auto( - applet_id, activity_and_flow_ids_from_submissions + activity_and_flow_ids_from_assignments, language + applet_id=applet_id, + ids=activity_and_flow_ids_from_submissions + activity_and_flow_ids_from_assignments, + include_auto=not is_limited_respondent, + language=language, ) result: list[ActivityOrFlowWithAssignmentsPublic] = [] @@ -263,7 +264,7 @@ async def applet_activities_for_respondent_subject( if assignment.activity_id == activity_or_flow.id or assignment.activity_flow_id == activity_or_flow.id ] - activity_or_flow.set_status(activity_or_flow_assignments) + activity_or_flow.set_status(assignments=activity_or_flow_assignments, include_auto=not is_limited_respondent) result.append( ActivityOrFlowWithAssignmentsPublic( diff --git a/src/apps/activities/crud/activity.py b/src/apps/activities/crud/activity.py index abacbbf53f1..6061c5681c8 100644 --- a/src/apps/activities/crud/activity.py +++ b/src/apps/activities/crud/activity.py @@ -108,7 +108,7 @@ async def get_ids_by_applet_id(self, applet_id: uuid.UUID) -> list[uuid.UUID]: return result.scalars().all() async def get_activity_and_flow_basic_info_by_ids_or_auto( - self, applet_id: uuid.UUID, ids: list[uuid.UUID], language: str + self, applet_id: uuid.UUID, ids: list[uuid.UUID], include_auto: bool, language: str ) -> list[ActivityOrFlowBasicInfoInternal]: activities_query: Query = select( ActivitySchema.id, @@ -126,7 +126,7 @@ async def get_activity_and_flow_basic_info_by_ids_or_auto( ActivitySchema.applet_id == applet_id, or_( ActivitySchema.id.in_(ids), - ActivitySchema.auto_assign.is_(True), + include_auto and ActivitySchema.auto_assign.is_(True), ), ) @@ -159,7 +159,7 @@ async def get_activity_and_flow_basic_info_by_ids_or_auto( flow_alias.applet_id == applet_id, or_( flow_alias.id.in_(ids), - flow_alias.auto_assign.is_(True), + include_auto and flow_alias.auto_assign.is_(True), ), ) .group_by(flow_alias.id, flow_alias.name, flow_alias.description, flow_alias.auto_assign) diff --git a/src/apps/activities/domain/activity.py b/src/apps/activities/domain/activity.py index 3403a7aa81a..8305d5e5e1f 100644 --- a/src/apps/activities/domain/activity.py +++ b/src/apps/activities/domain/activity.py @@ -49,13 +49,13 @@ class ActivityOrFlowBasicInfoInternal(InternalModel): performance_task_type: PerformanceTaskType | None = None is_performance_task: bool | None = None - def set_status(self, assignments: list[ActivityAssignmentWithSubject]): + def set_status(self, assignments: list[ActivityAssignmentWithSubject], include_auto: bool): """ Determine and set the value of the status field """ if self.is_hidden: self.status = ActivityOrFlowStatusEnum.HIDDEN - elif assignments or self.auto_assign: + elif assignments or (include_auto and self.auto_assign): self.status = ActivityOrFlowStatusEnum.ACTIVE else: self.status = ActivityOrFlowStatusEnum.INACTIVE diff --git a/src/apps/activities/services/activity.py b/src/apps/activities/services/activity.py index ea7ef7b89cd..b10d46527fa 100644 --- a/src/apps/activities/services/activity.py +++ b/src/apps/activities/services/activity.py @@ -457,8 +457,8 @@ async def get_info_by_applet_id(self, applet_id: uuid.UUID, language: str) -> li return activities async def get_activity_and_flow_basic_info_by_ids_or_auto( - self, applet_id: uuid.UUID, ids: list[uuid.UUID], language: str + self, applet_id: uuid.UUID, ids: list[uuid.UUID], include_auto: bool, language: str ) -> list[ActivityOrFlowBasicInfoInternal]: return await ActivitiesCRUD(self.session).get_activity_and_flow_basic_info_by_ids_or_auto( - applet_id, ids, language + applet_id, ids, include_auto, language ) diff --git a/src/apps/activities/tests/fixtures/items.py b/src/apps/activities/tests/fixtures/items.py index 0348fca3260..81cfbd24f22 100644 --- a/src/apps/activities/tests/fixtures/items.py +++ b/src/apps/activities/tests/fixtures/items.py @@ -316,8 +316,8 @@ def phrasal_template_with_time_create( ) return [time_item_create, phrasal_item] - - + + @pytest.fixture def phrasal_template_with_paragraph_create( phrasal_template_config: PhrasalTemplateConfig, diff --git a/src/apps/activities/tests/test_activities.py b/src/apps/activities/tests/test_activities.py index ffc6f65e878..43faa1f8352 100644 --- a/src/apps/activities/tests/test_activities.py +++ b/src/apps/activities/tests/test_activities.py @@ -1209,11 +1209,38 @@ async def test_assigned_activities_empty_applet( async def test_assigned_activities_limited_respondent( self, + session: AsyncSession, client: TestClient, tom: User, + tom_applet_one_subject: Subject, applet_one: AppletFull, applet_one_shell_account: Subject, + answer_create_payload: dict, ): + activity = applet_one.activities[0] + + activity_service = ActivityService(session, tom.id) + await activity_service.remove_applet_activities(applet_one.id) + await activity_service.update_create( + applet_one.id, + [ + ActivityUpdate( + **activity.dict(exclude={"auto_assign"}), + auto_assign=False, + ), + ], + ) + + # Create an activity answer + await AnswerService(session, tom.id).create_answer( + AppletAnswerCreate( + **answer_create_payload, + input_subject_id=tom_applet_one_subject.id, + source_subject_id=applet_one_shell_account.id, + target_subject_id=tom_applet_one_subject.id, + ) + ) + client.login(tom) response = await client.get( @@ -1222,11 +1249,22 @@ async def test_assigned_activities_limited_respondent( ) ) - assert response.status_code == http.HTTPStatus.BAD_REQUEST + assert response.status_code == http.HTTPStatus.OK result = response.json()["result"] - assert result[0]["type"] == "BAD_REQUEST" - assert result[0]["message"] == f"Subject {applet_one_shell_account.id} is not a valid respondent" + assert len(result) == 1 + + result_activity = result[0] + assert result_activity["id"] == str(activity.id) + assert result_activity["name"] == activity.name + assert result_activity["description"] == activity.description[Language.ENGLISH] + assert result_activity["status"] == ActivityOrFlowStatusEnum.INACTIVE.value + assert result_activity["isFlow"] is False + assert result_activity["autoAssign"] is False + assert result_activity["activityIds"] is None + assert result_activity["isPerformanceTask"] is False + assert result_activity["performanceTaskType"] is None + assert len(result_activity["assignments"]) == 0 @pytest.mark.parametrize("subject_type", ["target", "respondent"]) async def test_assigned_activities_auto_assigned( diff --git a/src/apps/subjects/api.py b/src/apps/subjects/api.py index ae6551ee3fb..fcb08e13301 100644 --- a/src/apps/subjects/api.py +++ b/src/apps/subjects/api.py @@ -310,11 +310,7 @@ async def get_target_subjects_by_respondent( ) -> ResponseMulti[TargetSubjectByRespondentResponse]: subjects_service = SubjectsService(session, user.id) respondent_subject = await subjects_service.exist_by_id(respondent_subject_id) - - # Ensure the respondent is not a limited account - if respondent_subject.user_id is None: - # Return a generic bad request error to avoid leaking information - raise ValidationError(f"Subject {respondent_subject_id} is not a valid respondent") + is_limited_respondent = respondent_subject.user_id is None # Make sure the authenticated user has access to the subject await CheckAccessService(session, user.id).check_subject_subject_access( @@ -327,7 +323,7 @@ async def get_target_subjects_by_respondent( ) is_auto_assigned = await assignment_service.check_if_auto_assigned(activity_or_flow_id) - if is_auto_assigned: + if is_auto_assigned and not is_limited_respondent: assignment_subject_ids.append(respondent_subject_id) submission_data: list[tuple[uuid.UUID, int]] = await AnswerService( diff --git a/src/apps/subjects/tests/tests.py b/src/apps/subjects/tests/tests.py index a2042a4c665..bcb98ec27f4 100644 --- a/src/apps/subjects/tests/tests.py +++ b/src/apps/subjects/tests/tests.py @@ -817,7 +817,7 @@ async def test_get_target_subjects_by_respondent_limited_account_respondent( respondent_subject_id=applet_one_shell_account.id, activity_or_flow_id=activity_or_flow_id ) response = await client.get(url) - assert response.status_code == http.HTTPStatus.BAD_REQUEST + assert response.status_code == http.HTTPStatus.OK async def test_get_target_subjects_by_respondent_editor_user( self, client, applet_one_pit_editor: AppletFull, pit: User, tom_applet_one_subject: Subject @@ -1074,17 +1074,21 @@ async def test_get_target_subjects_by_respondent_multiple_assignments( assert shell_account_result["submissionCount"] == 0 assert shell_account_result["currentlyAssigned"] is True + @pytest.mark.parametrize("subject_type", ["target", "respondent"]) async def test_get_target_subjects_by_respondent_via_submission( self, client, tom: User, tom_applet_one_subject: Subject, lucy_applet_one_subject: Subject, + applet_one_shell_account: Subject, + subject_type: str, applet_one_lucy_respondent: AppletFull, answer_create_payload: dict, session: AsyncSession, ): activity = applet_one_lucy_respondent.activities[0] + source_subject = lucy_applet_one_subject if subject_type == "respondent" else applet_one_shell_account # Turn off auto-assignment activity_service = ActivityService(session, tom.id) @@ -1104,7 +1108,7 @@ async def test_get_target_subjects_by_respondent_via_submission( AppletAnswerCreate( **answer_create_payload, input_subject_id=lucy_applet_one_subject.id, - source_subject_id=lucy_applet_one_subject.id, + source_subject_id=source_subject.id, target_subject_id=tom_applet_one_subject.id, ) ) @@ -1112,7 +1116,7 @@ async def test_get_target_subjects_by_respondent_via_submission( client.login(tom) url = self.subject_target_by_respondent_url.format( - respondent_subject_id=lucy_applet_one_subject.id, activity_or_flow_id=str(activity.id) + respondent_subject_id=source_subject.id, activity_or_flow_id=str(activity.id) ) response = await client.get(url) assert response.status_code == http.HTTPStatus.OK