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

Update task states for each task. #62

Merged
merged 11 commits into from
Jul 10, 2024
Merged

Update task states for each task. #62

merged 11 commits into from
Jul 10, 2024

Conversation

nrjadkry
Copy link
Collaborator

@nrjadkry nrjadkry commented Jul 8, 2024

Update Task State Transitions

Description

This pull request introduces changes to the task state transitions, enabling tasks to move seamlessly between different states based on specific actions. The following task states and actions are defined:

Task States

  1. UNLOCKED_TO_MAP
  2. LOCKED_FOR_MAPPING
  3. UNLOCKED_TO_VALIDATE
  4. LOCKED_FOR_VALIDATION
  5. UNLOCKED_DONE

Following actions can be taken to change the state of task.

  1. MAP
  2. FINISH
  3. VALIDATE
  4. GOOD
  5. BAD
  6. ASSIGN
  7. COMMENT

Database design.

Here is the database table designed for this task_actions
`class TaskEvent(Base):
tablename = "task_events"

event_id = cast(str, Column(UUID(as_uuid=True), primary_key=True))
project_id = cast(
    str, Column(UUID(as_uuid=True), ForeignKey("projects.id"), nullable=False)
)
task_id = cast(
    str, Column(UUID(as_uuid=True), ForeignKey("tasks.id"), nullable=False)
)
user_id = cast(str, Column(String(100), ForeignKey("users.id"), nullable=False))
comment = cast(str, Column(String))

state = cast(State, Column(Enum(State), nullable=False))
created_at = cast(datetime, Column(DateTime, default=timestamp))

__table_args__ = (
    Index("idx_task_event_composite", "task_id", "project_id"),
    Index("idx_task_event_project_id_user_id", "user_id", "project_id"),
)`

API Endpoints

  1. api/tasks/update-event/. This endpoint is created to update the state of tasks.
    It requires following payload.
{
  "event": "finish",
  "project_id": "492d8c6a-0e47-4c65-a6ad-d8348c195759",
  "task_id": "13afb2f6-f1ca-469f-88f9-178b1e15d377"
}
  1. api/tasks/states/<project_id>/ . This endpoint provides the state of each tasks within the project.
    Example response:
[
  {
    "project_id": "492d8c6a-0e47-4c65-a6ad-d8348c195759",
    "task_id": "13afb2f6-f1ca-469f-88f9-178b1e15d377",
    "state": "LOCKED_FOR_MAPPING"
  },
  {
    "project_id": "492d8c6a-0e47-4c65-a6ad-d8348c195759",
    "task_id": "680aaab3-2915-4438-a47f-37609fbf39d3",
    "state": "LOCKED_FOR_MAPPING"
  },
  {
    "project_id": "492d8c6a-0e47-4c65-a6ad-d8348c195759",
    "task_id": "f9bb41be-5a9c-4b01-8717-cbe887542a39",
    "state": "LOCKED_FOR_MAPPING"
  }
]

@nrjadkry
Copy link
Collaborator Author

nrjadkry commented Jul 8, 2024

@spwoodcock I have started designing the project/task/history schemas and relationships with taking reference from the suggestions you made similar to the one for fmtm you have mentioned in this issue.
hotosm/fmtm#1610

@nrjadkry nrjadkry requested a review from spwoodcock July 8, 2024 05:26
@nrjadkry nrjadkry marked this pull request as ready for review July 8, 2024 09:00
@spwoodcock
Copy link
Member

spwoodcock commented Jul 8, 2024

Will check this over soon!

Can this be refactored to use url path params though (REST)?

api/projects/project_id/tasks/task/id/event

  • The project and task id can be removed from the json payload.
  • update-event suggests it's an update endpoint, when it's actually a create endpoint. I think event is more appropriate

Sidenote something I didn't consider with UUID for project IDs is the awful URL it will produce if we use it in the frontend page path. UUID is essential for task events, but not necessarily for projects and tasks IDs. Either we revert to SERIAL (not ideal) or generate a slug field we can use in the URL

@nrjadkry
Copy link
Collaborator Author

nrjadkry commented Jul 8, 2024

Will check this over soon!

Can this be refactored to use url path params though (REST)?

api/projects/project_id/tasks/task/id/event

  • The project and task id can be removed from the json payload.
  • update-event suggests it's an update endpoint, when it's actually a create endpoint. I think event is more appropriate

Sidenote something I didn't consider with UUID for project IDs is the awful URL it will produce if we use it in the frontend page path. UUID is essential for task events, but not necessarily for projects and tasks IDs. Either we revert to SERIAL (not ideal) or generate a slug field we can use in the URL

Sure, we could remove project_id and task_id from json payload and use them in the url params.

And for the frontend url, you are right. I also did not think of that. I think we should use slug rather than reverting back to ID( Serial)

@nrjadkry
Copy link
Collaborator Author

nrjadkry commented Jul 8, 2024

@spwoodcock
One major diference in this project from FMTM and TM is that user will request to fly in the task (LOCKED FOR MAPPING) and then, when it is accepted by the project creator, then only user can fly the drone in this task.
How do you think we should approach this ?

@spwoodcock
Copy link
Member

spwoodcock commented Jul 9, 2024

Should we add a new state REQUESTED_MAPPING between unlocked to map and locked for mapping?

Then probably an event REQUEST to put the task in this state? The even would trigger notifying the project admin

@nrjadkry
Copy link
Collaborator Author

nrjadkry commented Jul 9, 2024

This might be a good idea. I have thought of the workflow like this for all the mapping process:
unlocked to map -> request for map -> locked for map-> unlock to validate-> lock for validation-> unlock done

And then for bad tasks back to unlock to map from lock_for_validation

@nrjadkry nrjadkry merged commit caaeea0 into main Jul 10, 2024
0 of 2 checks passed
@nrjadkry nrjadkry deleted the task_events branch July 22, 2024 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants