Skip to content

Commit

Permalink
Add device ID validation and handling
Browse files Browse the repository at this point in the history
- Added `device_id` as a header parameter in the API.
- Integrated `UserDeviceService` to validate `device_id`.
- Updated answer creation logic to include `device_id`.
- Removed `device_id` from the domain model.
- Enhanced tests for device ID scenarios, including invalid cases.
- Fixed a bug in schedule history retrieval by correcting variable name.

Enhance device and event validation logic

- Updated device retrieval to check both device_id and user_id.
- Enhanced event history lookup with activity or flow ID validation.
- Modified test data for consistency with new logic.
- Added SQLAlchemy case statement for conditional queries.

Refactor device and event history checks

- Updated `aio-pika` to version 9.5.5 in dependencies.
- Simplified device validation by removing user ID check from the query.
- Streamlined event history validation logic, removing complex query conditions.
- Removed unused imports and redundant methods for cleaner codebase.
- Adjusted database migration down_revision for consistency.
  • Loading branch information
rcmerlo committed Feb 26, 2025
1 parent 9e9d287 commit 81d2964
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 25 deletions.
8 changes: 4 additions & 4 deletions Pipfile.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 16 additions & 3 deletions src/apps/answers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
import base64
import datetime
import uuid
from typing import Annotated

from fastapi import Body, Depends, Query
from fastapi import Body, Depends, Header, Query
from fastapi.responses import Response as FastApiResponse
from pydantic import parse_obj_as

Expand Down Expand Up @@ -60,6 +61,7 @@
from apps.subjects.services import SubjectsService
from apps.users import UsersCRUD
from apps.users.domain import User
from apps.users.services.user_device import UserDeviceService
from apps.workspaces.domain.constants import Role
from apps.workspaces.service.check_access import CheckAccessService
from apps.workspaces.service.workspace import WorkspaceService
Expand All @@ -75,6 +77,7 @@ async def create_answer(
tz_offset: int | None = Depends(get_tz_utc_offset()),
session=Depends(get_session),
answer_session=Depends(get_answer_session),
device_id: Annotated[str | None, Header()] = None,
) -> None:
async with atomic(session):
await CheckAccessService(session, user.id).check_answer_create_access(schema.applet_id)
Expand All @@ -83,16 +86,26 @@ async def create_answer(
except NotValidAppletHistory:
raise InvalidVersionError()

if device_id:
device = await UserDeviceService(session, user.id).get_by_device_id(device_id)
if device is None or device.user_id != user.id:
raise NotFoundError("Invalid device_id provided")

if schema.event_history_id:
event = await ScheduleHistoryService(session).get_by_id(schema.event_history_id)
if not event:
if (
event is None
or event.activity_flow_id != schema.flow_id
or event.activity_id != schema.activity_id
or (event.user_id is not None and event.user_id != user.id)
):
raise NotFoundError("Invalid event_history_id provided")

service = AnswerService(session, user.id, answer_session)
if tz_offset is not None and schema.answer.tz_offset is None:
schema.answer.tz_offset = tz_offset // 60 # value in minutes
async with atomic(answer_session):
answer = await service.create_answer(schema)
answer = await service.create_answer(schema, device_id)
await service.create_report_from_answer(answer)


Expand Down
1 change: 0 additions & 1 deletion src/apps/answers/domain/answers.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ class AppletAnswerCreate(InternalModel):
input_subject_id: uuid.UUID | None = None
consent_to_share: bool | None = False
event_history_id: str | None = None
device_id: str | None = None

_dates_from_ms = validator("created_at", pre=True, allow_reuse=True)(datetime_from_ms)

Expand Down
22 changes: 13 additions & 9 deletions src/apps/answers/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,23 @@ def key_generator(pk: uuid.UUID) -> str:

return key_generator

async def create_answer(self, activity_answer: AppletAnswerCreate) -> AnswerSchema:
async def create_answer(self, activity_answer: AppletAnswerCreate, device_id: str | None = None) -> AnswerSchema:
if self.user_id:
return await self._create_respondent_answer(activity_answer)
return await self._create_respondent_answer(activity_answer, device_id)
else:
return await self._create_anonymous_answer(activity_answer)
return await self._create_anonymous_answer(activity_answer, device_id)

async def _create_respondent_answer(self, activity_answer: AppletAnswerCreate) -> AnswerSchema:
async def _create_respondent_answer(
self, activity_answer: AppletAnswerCreate, device_id: str | None
) -> AnswerSchema:
await self._validate_respondent_answer(activity_answer)
return await self._create_answer(activity_answer)
return await self._create_answer(activity_answer, device_id)

async def _create_anonymous_answer(self, activity_answer: AppletAnswerCreate) -> AnswerSchema:
async def _create_anonymous_answer(
self, activity_answer: AppletAnswerCreate, device_id: str | None
) -> AnswerSchema:
await self._validate_anonymous_answer(activity_answer)
return await self._create_answer(activity_answer)
return await self._create_answer(activity_answer, device_id)

async def _validate_respondent_answer(self, activity_answer: AppletAnswerCreate) -> None:
await self._validate_answer(activity_answer)
Expand Down Expand Up @@ -305,7 +309,7 @@ async def _get_answer_relation(

return relation.relation

async def _create_answer(self, applet_answer: AppletAnswerCreate) -> AnswerSchema:
async def _create_answer(self, applet_answer: AppletAnswerCreate, device_id: str | None) -> AnswerSchema:
assert self.user_id
pk = self._generate_history_id(applet_answer.version)
created_at = applet_answer.created_at or datetime.datetime.now(datetime.UTC).replace(tzinfo=None)
Expand Down Expand Up @@ -383,7 +387,7 @@ async def _create_answer(self, applet_answer: AppletAnswerCreate) -> AnswerSchem
relation=relation,
consent_to_share=applet_answer.consent_to_share,
event_history_id=applet_answer.event_history_id,
device_id=applet_answer.device_id,
device_id=device_id,
)
)
item_answer = applet_answer.answer
Expand Down
37 changes: 32 additions & 5 deletions src/apps/answers/tests/test_answers.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
from apps.subjects.services import SubjectsService
from apps.users import User
from apps.users.cruds.user import UsersCRUD
from apps.users.domain import UserCreate
from apps.users.domain import AppInfoOS, UserCreate, UserDeviceCreate
from apps.users.router import router as user_router
from apps.users.tests.fixtures.users import _get_or_create_user
from apps.workspaces.crud.user_applet_access import UserAppletAccessCRUD
from apps.workspaces.db.schemas import UserAppletAccessSchema
Expand Down Expand Up @@ -682,6 +683,11 @@ async def applet_default_events(session: AsyncSession, applet: AppletFull) -> li
return events


@pytest.fixture
def device_create_data() -> UserDeviceCreate:
return UserDeviceCreate(os=AppInfoOS(name="os1", version="1.0.0"), app_version="51.0.0", device_id="device_id")


@pytest.mark.usefixtures("mock_kiq_report")
class TestAnswerActivityItems(BaseTest):
fixtures = [
Expand Down Expand Up @@ -822,14 +828,25 @@ async def test_create_answer__wrong_applet_version(
assert response.json()["result"][0]["message"] == InvalidVersionError.message

async def test_create_answer__with_device_id_and_event_history_id(
self, client: TestClient, tom: User, answer_create: AppletAnswerCreate, applet_default_events
self,
client: TestClient,
tom: User,
answer_create: AppletAnswerCreate,
applet_default_events,
device_create_data,
):
client.login(tom)

response = await client.post(
user_router.url_path_for("user_save_device"), data=device_create_data.dict(include={"device_id"})
)
assert response.status_code == http.HTTPStatus.OK

data = answer_create.copy(deep=True)
event = applet_default_events[0]
event = next((event for event in applet_default_events if event.activity_id == answer_create.activity_id), None)
assert event
data.event_history_id = f"{event.id}_{event.version}"
data.device_id = "test_device_id"
response = await client.post(self.answer_url, data=data)
response = await client.post(self.answer_url, data=data, headers={"Device-Id": device_create_data.device_id})

assert response.status_code == http.HTTPStatus.CREATED

Expand All @@ -844,6 +861,16 @@ async def test_create_answer_with_wrong_event_history_id(
assert response.status_code == http.HTTPStatus.NOT_FOUND
assert response.json()["result"][0]["message"] == "Invalid event_history_id provided"

async def test_create_answer_with_wrong_device_id(
self, client: TestClient, tom: User, answer_create: AppletAnswerCreate
):
client.login(tom)
data = answer_create.copy(deep=True)
response = await client.post(self.answer_url, data=data, headers={"Device-Id": "wrong_device_id"})

assert response.status_code == http.HTTPStatus.NOT_FOUND
assert response.json()["result"][0]["message"] == "Invalid device_id provided"

async def test_create_activity_answers__submit_duplicate(
self,
client: TestClient,
Expand Down
2 changes: 1 addition & 1 deletion src/apps/schedule/crud/schedule_history.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class ScheduleHistoryCRUD(BaseCRUD[EventHistorySchema]):
schema_class = EventHistorySchema

async def get_by_id(self, id_version: str) -> EventHistorySchema | None:
return await self._get("id_version", id)
return await self._get("id_version", id_version)

async def add(self, event: EventHistorySchema) -> EventHistorySchema:
return await self._create(event)
Expand Down
5 changes: 5 additions & 0 deletions src/apps/users/services/user_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ def __init__(self, session, user_id: uuid.UUID) -> None:
self.session = session
self.user_id = user_id

async def get_by_device_id(self, device_id: str) -> UserDevice | None:
schema = await UserDevicesCRUD(self.session).get_by_device_id(device_id)

return UserDevice.from_schema(schema) if schema else None

async def add_device(self, data: UserDeviceCreate) -> UserDevice:
app_data = dict(
app_version=data.app_version,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@

import sqlalchemy as sa
from alembic import op
from sqlalchemy.dialects import postgresql

# revision identifiers, used by Alembic.
revision = "5af378151328"
down_revision = "70987d489b17"
down_revision = "4e2b42e69c39"
branch_labels = None
depends_on = None

Expand Down

0 comments on commit 81d2964

Please sign in to comment.