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

editoast: work-schedules: add endpoints for easier edition and viewing #9785

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

eckter
Copy link
Contributor

@eckter eckter commented Nov 19, 2024

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:

  1. POST /work_schedules/group: create a new group (name can be empty and autofilled (the unique name thing is just painful))
  2. GET /work_schedules/group: lists all the group ids
  3. PUT /work_schedules/group/42: insets a work schedule into an existing group
  4. GET /work_schedules/group/42: lists all the work schedules in the group
  5. DELETE /workçschedules/group/42: deletes the group and its work schedules

What I have not added but could have been nice:

  1. More details in GET /work_schedules/group (name, creation date, ...)
  2. GET /work_schedules to list them all
  3. Any kind of endpoint dealing with individual work schedules

@codecov-commenter
Copy link

codecov-commenter commented Nov 19, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 80.83333% with 46 lines in your changes missing coverage. Please review.

Project coverage is 38.22%. Comparing base (7527a8e) to head (da16772).
Report is 10 commits behind head on dev.

Files with missing lines Patch % Lines
editoast/src/views/work_schedules.rs 84.10% 31 Missing ⚠️
front/src/common/api/generatedEditoastApi.ts 65.78% 13 Missing ⚠️
editoast/src/views/operational_studies.rs 66.66% 2 Missing ⚠️

❗ 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              
Flag Coverage Δ
editoast 73.30% <83.66%> (+0.02%) ⬆️
front 20.24% <65.78%> (+0.03%) ⬆️
gateway 2.18% <ø> (ø)
osrdyne 3.28% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 86.74% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@eckter eckter force-pushed the ech/work-schedule-endpoints branch 5 times, most recently from 7525cf6 to 7cb0332 Compare November 19, 2024 15:44
@github-actions github-actions bot added the area:editoast Work on Editoast Service label Nov 19, 2024
@eckter eckter force-pushed the ech/work-schedule-endpoints branch 6 times, most recently from 8f1bb19 to 5f2eca1 Compare November 20, 2024 14:28
@eckter eckter marked this pull request as ready for review November 20, 2024 14:47
@eckter eckter requested review from a team as code owners November 20, 2024 14:47
@eckter eckter force-pushed the ech/work-schedule-endpoints branch 2 times, most recently from 53adda9 to 2c360cb Compare November 21, 2024 08:55
Copy link
Contributor

@woshilapin woshilapin left a 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.

editoast/src/views/work_schedules.rs Outdated Show resolved Hide resolved
editoast/src/views/work_schedules.rs Outdated Show resolved Hide resolved
editoast/src/views/work_schedules.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@leovalais leovalais left a 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 :)

editoast/src/views/work_schedules.rs Outdated Show resolved Hide resolved
editoast/src/views/work_schedules.rs Outdated Show resolved Hide resolved
editoast/src/views/work_schedules.rs Outdated Show resolved Hide resolved
editoast/src/views/work_schedules.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@Sh099078 Sh099078 left a 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

editoast/src/views/work_schedules.rs Outdated Show resolved Hide resolved
editoast/src/views/work_schedules.rs Outdated Show resolved Hide resolved
editoast/src/views/work_schedules.rs Outdated Show resolved Hide resolved
editoast/src/views/work_schedules.rs Show resolved Hide resolved
Copy link
Contributor

@SharglutDev SharglutDev left a 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)

front/public/locales/en/errors.json Outdated Show resolved Hide resolved
@github-actions github-actions bot added the area:front Work on Standard OSRD Interface modules label Nov 22, 2024
@eckter eckter force-pushed the ech/work-schedule-endpoints branch from 62011da to 61426f6 Compare November 22, 2024 09:43
Copy link
Contributor

@leovalais leovalais left a 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!

editoast/src/views/work_schedules.rs Outdated Show resolved Hide resolved
editoast/src/views/work_schedules.rs Outdated Show resolved Hide resolved
@eckter eckter force-pushed the ech/work-schedule-endpoints branch 2 times, most recently from fadd260 to b27ef13 Compare November 22, 2024 13:51
Copy link
Contributor

@woshilapin woshilapin left a 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.

@eckter eckter force-pushed the ech/work-schedule-endpoints branch from f214017 to da16772 Compare November 26, 2024 10:37
@eckter eckter enabled auto-merge November 26, 2024 10:37
@eckter eckter added this pull request to the merge queue Nov 26, 2024
Merged via the queue into dev with commit 68c3e14 Nov 26, 2024
28 checks passed
@eckter eckter deleted the ech/work-schedule-endpoints branch November 26, 2024 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:editoast Work on Editoast Service area:front Work on Standard OSRD Interface modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants