Add check for conflicting events before creation #129
Conversation
vetlek
force-pushed
the
add_verification_pose_event
branch
2 times, most recently
from
7200116
to
079bddd
Compare
|
I think this looks good. When my comments above are solved I will approve. |
vetlek
force-pushed
the
add_verification_pose_event
branch
from
079bddd
to
6100203
Compare
| if overlapping_events: | ||
| raise HTTPException( | ||
| status_code=HTTPStatus.CONFLICT.value, | ||
| detail=f"Conflict with already existing event in the same time period. Events with id: {','.join(str(event.id) for event in overlapping_events)}", |
There was a problem hiding this comment.
Will all users be authorized to see all events for a given robot?
If not, this might be giving out too much information, maybe the first statement is enough?
I'm not sure, open for discussion
There was a problem hiding this comment.
I would say that if the user is authorized to post events (which I believe should have a "higher" access level than just read events) they will be allowed to see all events.
| @@ -62,11 +63,24 @@ async def post_event( | |||
| ) -> Event: | |||
| """Add a new event to the robot schedule""" | |||
| try: | |||
| end_time = event_request.start_time + DEFAULT_EVENT_DURATION | |||
There was a problem hiding this comment.
Will we always use the default duration? Maybe the user should have an event_duration optional field to this post endpoint so they can define it themselves?
There was a problem hiding this comment.
Currently a default value is enough in my opinion. I don't think the user should be able to set the event_duration, but rather be based on the number of tags and/or based on data from previous missions with the same id. Can make a card for setting the event_duration based on number of tags.
vetlek
force-pushed
the
add_verification_pose_event
branch
from
6100203
to
c8d0925
Compare
Closes #95