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: migrate endpoint to create a rolling_stock livery #3892

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

clarani
Copy link
Contributor

@clarani clarani commented Apr 13, 2023

closes #3498

⚠️ this new endpoint is not tested because TestRequest does not support multipart for the moment. Here is a ticket #3931 to add tests on this specific endpoint (in the future).

To test this endpoint, you can use postman with this configuration (select form-data for the body, use the images in editoast/src/tests if you want). You can add several images for a single request.
image

@clarani clarani force-pushed the cni/editoast/migrate-create-livery branch 2 times, most recently from f544e4c to 2faece8 Compare April 18, 2023 08:04
Copy link
Contributor Author

@clarani clarani left a comment

Choose a reason for hiding this comment

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

Ready for review

@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Merging #3892 (394be8b) into dev (46afe1c) will decrease coverage by 0.34%.
The diff coverage is 27.95%.

❗ Current head 394be8b differs from pull request most recent head 3404a48. Consider uploading reports for the commit 3404a48 to get more accurate results

@@             Coverage Diff              @@
##                dev    #3892      +/-   ##
============================================
- Coverage     69.83%   69.50%   -0.34%     
  Complexity     2026     2026              
============================================
  Files           441      439       -2     
  Lines         22432    22332     -100     
  Branches       1672     1672              
============================================
- Hits          15665    15521     -144     
- Misses         5962     6006      +44     
  Partials        805      805              
Flag Coverage Δ
editoast 72.84% <5.97%> (-1.95%) ⬇️
front 59.33% <84.61%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
editoast/src/models/mod.rs 37.03% <ø> (ø)
editoast/src/models/rolling_stock/mod.rs 96.87% <ø> (ø)
...t/src/models/rolling_stock/rolling_stock_livery.rs 33.33% <ø> (ø)
editoast/src/tables.rs 0.00% <ø> (ø)
editoast/src/views/rolling_stocks.rs 35.35% <5.97%> (-61.62%) ⬇️
front/src/common/api/osrdEditoastApi.ts 91.30% <84.61%> (-0.33%) ⬇️

... and 10 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@clarani clarani force-pushed the cni/editoast/migrate-create-livery branch 3 times, most recently from 3f9ac9f to de931da Compare April 18, 2023 10:28
@clarani clarani added the area:editoast Work on Editoast Service label Apr 18, 2023
@clarani clarani force-pushed the cni/editoast/migrate-create-livery branch from 394be8b to c11afc6 Compare April 18, 2023 11:06
@clarani clarani marked this pull request as ready for review April 18, 2023 11:06
@clarani clarani requested a review from a team as a code owner April 18, 2023 11:06
@clarani clarani force-pushed the cni/editoast/migrate-create-livery branch from c11afc6 to 07e5455 Compare April 18, 2023 13:01
Copy link
Contributor

@younesschrifi younesschrifi left a comment

Choose a reason for hiding this comment

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

LGTM !! Nice PR :)

@clarani clarani force-pushed the cni/editoast/migrate-create-livery branch 2 times, most recently from ec1badf to dc06556 Compare April 18, 2023 14:43
Copy link
Contributor

@flomonster flomonster 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 this PR! This looks good to me. I've suggested changes to slightly improve the code.

editoast/src/views/rolling_stocks.rs Outdated Show resolved Hide resolved
editoast/src/views/rolling_stocks.rs Outdated Show resolved Hide resolved
editoast/src/views/rolling_stocks.rs Outdated Show resolved Hide resolved
@clarani clarani removed the request for review from Tguisnet April 18, 2023 15:16
@clarani clarani force-pushed the cni/editoast/migrate-create-livery branch 2 times, most recently from 2c36bd2 to 434872b Compare April 18, 2023 15:48
@clarani clarani force-pushed the cni/editoast/migrate-create-livery branch from 434872b to 3404a48 Compare April 18, 2023 15:48
@clarani clarani merged commit 5cfdb30 into dev Apr 18, 2023
@clarani clarani deleted the cni/editoast/migrate-create-livery branch April 18, 2023 16:11
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

migrate POST /rolling-stock/{id}/livery endpoint to editoast
4 participants