Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assign users to tasks #2237

Draft
wants to merge 2 commits into
base: development
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 47 additions & 2 deletions src/backend/app/db/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from datetime import timedelta
from io import BytesIO
from re import sub
from typing import TYPE_CHECKING, Annotated, Optional, Self
from typing import TYPE_CHECKING, Annotated, List, Optional, Self
from uuid import UUID

import geojson
Expand Down Expand Up @@ -839,7 +839,7 @@ async def create(
u.profile_img
FROM inserted
JOIN users u ON u.id = inserted.user_id;
""",
""",
model_dump,
)
new_task_event = await cur.fetchone()
Expand All @@ -854,6 +854,51 @@ async def create(
return new_task_event


class DbTaskAssignment(BaseModel):
"""Table task_assignments."""

project_id: int
task_id: int
user_id: Annotated[Optional[int], Field(gt=0)] = None
assigned_at: Optional[AwareDatetime] = None

@classmethod
async def create(
cls, db: Connection, project_id: int, task_id: int, user_ids: List[int]
):
"""Create new task assignments for multiple users."""
print("user_ids in task assignments", user_ids)
async with db.cursor(row_factory=class_row(cls)) as cur:
await cur.executemany(
"""
INSERT INTO public.task_assignments (project_id, task_id, user_id)
VALUES (%(project_id)s, %(task_id)s, %(user_id)s)
ON CONFLICT (task_id, user_id) DO NOTHING;
""",
[
{"project_id": project_id, "task_id": task_id, "user_id": user_id}
for user_id in user_ids
],
)

@classmethod
async def get(cls, db: Connection, project_id: int, task_id: int):
"""Get task assignment by task ID."""
async with db.cursor(row_factory=class_row(cls)) as cur:
await cur.execute(
"""
SELECT * FROM public.task_assignments
WHERE project_id = %(project_id)s
AND task_id = %(task_id)s
""",
{
"task_id": task_id,
"project_id": project_id,
},
)
return await cur.fetchall()


class DbTask(BaseModel):
"""Table tasks."""

Expand Down
45 changes: 42 additions & 3 deletions src/backend/app/tasks/task_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#
"""Routes for FMTM tasks."""

from typing import Annotated
from typing import Annotated, List, Optional

from fastapi import APIRouter, Depends, HTTPException
from loguru import logger as log
Expand All @@ -26,8 +26,8 @@
from app.auth.auth_schemas import ProjectUserDict
from app.auth.roles import mapper, super_admin
from app.db.database import db_conn
from app.db.enums import HTTPStatus
from app.db.models import DbTask, DbTaskEvent, DbUser
from app.db.enums import HTTPStatus, TaskEvent
from app.db.models import DbTask, DbTaskAssignment, DbTaskEvent, DbUser
from app.tasks import task_crud, task_schemas

router = APIRouter(
Expand Down Expand Up @@ -95,16 +95,55 @@ async def add_new_task_event(
new_event: task_schemas.TaskEventIn,
project_user: Annotated[ProjectUserDict, Depends(mapper)],
db: Annotated[Connection, Depends(db_conn)],
assignee_id: Optional[int] = None,
):
"""Add a new event to the events table / update task status."""
user_id = project_user.get("user").id
log.info(f"Task {task_id} event: {new_event.event.name} (by user {user_id})")

new_event.user_id = user_id
new_event.task_id = task_id
if new_event.event == TaskEvent.ASSIGN:
if not assignee_id:
raise HTTPException(
status_code=HTTPStatus.BAD_REQUEST,
detail="Assignee ID is required for ASSIGN event",
)
project_id = project_user.get("project").id

# NOTE: This will save the assigned user instead of current user if event is
# ASSIGN to avoid adding a new field for it
new_event.user_id = assignee_id
await DbTaskAssignment.create(db, project_id, task_id, [assignee_id])
return await DbTaskEvent.create(db, new_event)


@router.get(
"/{task_id}/assignments", response_model=list[task_schemas.TaskAssignmentOut]
)
async def get_task_assignments(
task_id: int,
project_user: Annotated[ProjectUserDict, Depends(mapper)],
db: Annotated[Connection, Depends(db_conn)],
):
"""Get all task assignments for a task."""
project_id = project_user.get("project").id
return await DbTaskAssignment.get(db, project_id, task_id)


@router.post("/{task_id}/assignments")
async def assign_to_task(
task_id: int,
project_user: Annotated[ProjectUserDict, Depends(mapper)],
db: Annotated[Connection, Depends(db_conn)],
user_ids: List[int],
):
"""Get all task assignments for a task."""
project_id = project_user.get("project").id
await DbTaskAssignment.create(db, project_id, task_id, user_ids)
return {"message": "Users have been assigned to the task successfully."}


@router.get("/{task_id}/history", response_model=list[task_schemas.TaskEventOut])
async def get_task_event_history(
task_id: int,
Expand Down
11 changes: 10 additions & 1 deletion src/backend/app/tasks/task_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from uuid import UUID

from geojson_pydantic import Polygon
from pydantic import BaseModel, Field
from pydantic import AwareDatetime, BaseModel, Field

from app.db.enums import MappingState
from app.db.models import DbTask, DbTaskEvent
Expand Down Expand Up @@ -64,3 +64,12 @@ class TaskEventCount(BaseModel):
date: str
mapped: int
validated: int


class TaskAssignmentOut(BaseModel):
"""Task assignments."""

project_id: int
task_id: int
user_id: int
assigned_at: AwareDatetime
44 changes: 44 additions & 0 deletions src/backend/migrations/009-task-assignments.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
-- Begin transaction
BEGIN;

-- Create task_assignments table only if it does not exist
DO $$
BEGIN
IF NOT EXISTS (SELECT 1 FROM information_schema.tables WHERE table_name = 'task_assignments') THEN
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a simpler syntax in postgres for this: create table if not exists task_assignments

Checking the information schema is only needed when this is not possible, such as with enums

CREATE TABLE public.task_assignments (
project_id INTEGER NOT NULL,
task_id INTEGER NOT NULL,
user_id INTEGER NOT NULL,
assigned_at TIMESTAMPTZ NOT NULL DEFAULT now()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a new table for this?

I thought we would just have an ASSIGN event in the task events table?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding new things to the database should always be a last resort

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we rely solely on the task_events table, we'd either have multiple event records per user (causing redundancy and data bloat) or a separate field for assignees that stores a list of user IDs resulting in slower querying and an empty column for non-assignment events.

Copy link
Member

@spwoodcock spwoodcock Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - we need to reference both the assigner (eg manager) and the assigned (mapper) in one event.

Instead, how about we add a field to the tasks table assigned_to, then also make a task events entry.

The task event will have the user id for the assigner (plus a comment containing the username of who it was assigned to), while the assigned_to field will have the user id of the assigned.

Do you think that could be good?

(the task unlocking logic may have to be tweaked though to remove the assigned_to value from the tasks table though)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are to assign multiple users to a task, the field we add to the tasks table would need to be a list of integers (since the table can only have one row for each task ID). We would need to append or extend to the assigned_to field, which would be a list of user IDs. This is not the best approach.

It would be better to store it in a new task assignments table for faster querying, simple insert and delete for assignments and referential integrity with foreign keys. The assignments would also be automatically deleted without any new query code if the user, project or task were to be deleted.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fully agree - if we need to assign multiple users then this is the way to go πŸ‘

Assigning multiple users doesn't really fit into our model though. A task can only be locked by one user. The credit for mapping a task will only go to one user. I thought we always discussed team mapping as an informal process, not recorded.

Anyway, if this was discussed in my absence and agreed on, I will defer to your judgement πŸ˜„

);
ALTER TABLE public.task_assignments OWNER TO fmtm;
END IF;
END$$;

-- Add primary key constraint if it does not exist
DO $$
BEGIN
IF NOT EXISTS (SELECT 1 FROM information_schema.table_constraints WHERE table_name = 'task_assignments' AND constraint_name = 'task_assignments_pkey') THEN
ALTER TABLE ONLY public.task_assignments
ADD CONSTRAINT task_assignments_pkey PRIMARY KEY (task_id, user_id);
END IF;
END$$;

-- Add foreign key constraints if they do not exist
DO $$
BEGIN
IF NOT EXISTS (SELECT 1 FROM information_schema.table_constraints WHERE table_name = 'task_assignments' AND constraint_name = 'task_assignments_user_id_fkey') THEN
ALTER TABLE ONLY public.task_assignments
ADD CONSTRAINT task_assignments_user_id_fkey FOREIGN KEY (user_id)
REFERENCES public.users (id) ON DELETE CASCADE;
END IF;

IF NOT EXISTS (SELECT 1 FROM information_schema.table_constraints WHERE table_name = 'task_assignments' AND constraint_name = 'task_assignments_project_id_fkey') THEN
ALTER TABLE public.task_assignments
ADD CONSTRAINT task_assignments_project_id_fkey FOREIGN KEY (project_id)
REFERENCES public.projects (id) ON DELETE CASCADE;
END IF;
END$$;

-- Commit transaction
COMMIT;
18 changes: 18 additions & 0 deletions src/backend/migrations/init/fmtm_base_schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,14 @@ CREATE TABLE public.geometrylog (
);
ALTER TABLE public.geometrylog OWNER TO fmtm;

CREATE TABLE public.task_assignments (
project_id INTEGER NOT NULL,
task_id INTEGER NOT NULL,
user_id INTEGER NOT NULL,
assigned_at TIMESTAMPTZ NOT NULL DEFAULT now()
);
ALTER TABLE public.task_assignments OWNER TO fmtm;

-- nextval for primary keys (autoincrement)

ALTER TABLE ONLY public.organisations ALTER COLUMN id SET DEFAULT nextval(
Expand Down Expand Up @@ -467,6 +475,9 @@ ADD CONSTRAINT xlsforms_title_key UNIQUE (title);
ALTER TABLE ONLY public.geometrylog
ADD CONSTRAINT geometrylog_pkey PRIMARY KEY (id);

ALTER TABLE ONLY public.task_assignments
ADD CONSTRAINT task_assignments_pkey PRIMARY KEY (task_id, user_id);

-- Indexing

CREATE INDEX idx_projects_outline ON public.projects USING gist (outline);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably need an index for faster lookup. Choosing the column depends what you will query on most frequently.

Expand Down Expand Up @@ -567,6 +578,13 @@ ADD CONSTRAINT user_roles_user_id_fkey FOREIGN KEY (
user_id
) REFERENCES public.users (id);

ALTER TABLE ONLY public.task_assignments
ADD CONSTRAINT task_assignments_user_id_fkey FOREIGN KEY (user_id)
REFERENCES public.users (id) ON DELETE CASCADE;

ALTER TABLE public.task_assignments
ADD CONSTRAINT task_assignments_project_id_fkey FOREIGN KEY (project_id)
REFERENCES public.projects (id) ON DELETE CASCADE;
-- Triggers

CREATE OR REPLACE FUNCTION public.set_task_state()
Expand Down