-
Notifications
You must be signed in to change notification settings - Fork 45
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
editoast: work-schedules: add endpoints for easier edition and viewing #9785
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #9785 +/- ##
==========================================
+ Coverage 38.13% 38.22% +0.08%
==========================================
Files 995 995
Lines 91675 91891 +216
Branches 1193 1193
==========================================
+ Hits 34963 35128 +165
- Misses 56258 56309 +51
Partials 454 454
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
7525cf6
to
7cb0332
Compare
8f1bb19
to
5f2eca1
Compare
53adda9
to
2c360cb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, just some comments about if we should do more transactional operations or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, a few non-blocking comments :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great PR ! I added a few comments but mostly nitpicks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm for front part (not tested)
62011da
to
61426f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the changes!
fadd260
to
b27ef13
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the improvements.
Signed-off-by: Eloi Charpentier <[email protected]>
f214017
to
da16772
Compare
Would fix 2/3 of https://github.com/osrd-project/osrd-confidential/issues/838, the rest is changing the etl for batched imports.
I've added a few more endpoints than just what was in the ticket, to have a complete workflow:
POST /work_schedules/group
: create a new group (name can be empty and autofilled (the unique name thing is just painful))GET /work_schedules/group
: lists all the group idsPUT /work_schedules/group/42
: insets a work schedule into an existing groupGET /work_schedules/group/42
: lists all the work schedules in the groupDELETE /workçschedules/group/42
: deletes the group and its work schedulesWhat I have not added but could have been nice:
GET /work_schedules/group
(name, creation date, ...)GET /work_schedules
to list them all