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.

Sign up for GitHub

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

Jump to bottom

editoast: remove serde default from project and study #6949

Merged
merged 1 commit into from
Apr 16, 2024
Merged

editoast: remove serde default from project and study #6949

merged 1 commit into from
Apr 16, 2024

Conversation

Wadjetz
Copy link
Member

@Wadjetz Wadjetz commented Mar 20, 2024 •
edited
Loading

  • The frontend already send empty strings
  • Fix openapi
  • Fix tests

close #6773

@Wadjetz Wadjetz self-assigned this Mar 20, 2024
@codecov Codecov
Copy link

codecov bot commented Mar 20, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 70.00000% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 28.94%. Comparing base (481083c) to head (36fdd62).

Files Patch % Lines
editoast/src/modelsv2/study.rs 0.00% 4 Missing ⚠️
editoast/src/modelsv2/projects.rs 0.00% 3 Missing ⚠️
...dules/project/components/AddOrEditProjectModal.tsx 0.00% 3 Missing ⚠️
...c/modules/study/components/AddOrEditStudyModal.tsx 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##                dev    #6949      +/-   ##
============================================
- Coverage     28.95%   28.94%   -0.01%     
  Complexity     2244     2244              
============================================
  Files          1070     1070              
  Lines        132528   132528              
  Branches       2713     2713              
============================================
- Hits          38371    38365       -6     
- Misses        92580    92586       +6     
  Partials       1577     1577
Flag Coverage Δ
core 79.81% <ø> (ø)
editoast 74.81% <66.66%> (-0.03%) ⬇️
front 9.29% <73.68%> (ø)
gateway 2.45% <ø> (ø)
railjson_generator 87.15% <ø> (ø)
tests 83.07% <ø> (ø)

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.

@Wadjetz Wadjetz force-pushed the ebe/editoast-project-study-remove-default branch from 03c75ac to 5126409 Compare March 20, 2024 14:32
@Wadjetz Wadjetz marked this pull request as ready for review March 20, 2024 14:32
@Wadjetz Wadjetz requested a review from a team as a code owner March 20, 2024 14:32
@Wadjetz Wadjetz force-pushed the ebe/editoast-project-study-remove-default branch 3 times, most recently from 8a4548b to 6775dfd Compare March 20, 2024 15:49
@Wadjetz Wadjetz requested a review from a team as a code owner March 20, 2024 15:49
@Wadjetz Wadjetz force-pushed the ebe/editoast-project-study-remove-default branch 2 times, most recently from 6f34c1a to e937be7 Compare March 20, 2024 16:23
@Wadjetz Wadjetz requested a review from a team as a code owner March 20, 2024 16:23
@Wadjetz Wadjetz requested a review from bloussou March 20, 2024 16:23
younesschrifi
younesschrifi approved these changes Mar 20, 2024
View reviewed changes
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

Tguisnet
Tguisnet requested changes Mar 21, 2024
View reviewed changes
Copy link
Contributor

@Tguisnet Tguisnet left a comment

Choose a reason for hiding this comment

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

You need to Adapt OpenAPI and tests, otherwise it looks good to me for the editoast part.

@Wadjetz Wadjetz force-pushed the ebe/editoast-project-study-remove-default branch from e937be7 to 2e47538 Compare March 21, 2024 09:39
hamz2a
hamz2a approved these changes Mar 21, 2024
View reviewed changes
Copy link
Contributor

@hamz2a hamz2a left a comment

Choose a reason for hiding this comment

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

LGTM.

@Wadjetz Wadjetz force-pushed the ebe/editoast-project-study-remove-default branch 3 times, most recently from 9eb0f13 to 36fdd62 Compare March 28, 2024 20:10
@Wadjetz Wadjetz requested a review from Tguisnet March 28, 2024 21:07
@Wadjetz Wadjetz force-pushed the ebe/editoast-project-study-remove-default branch from 36fdd62 to 43a6acc Compare April 16, 2024 07:46
@Wadjetz Wadjetz requested review from a team as code owners April 16, 2024 07:46
@codecov-commenter
Copy link

codecov-commenter commented Apr 16, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 70.00000% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 28.18%. Comparing base (bde4844) to head (b050e4b).

Files Patch % Lines
editoast/src/modelsv2/study.rs 0.00% 4 Missing ⚠️
editoast/src/modelsv2/projects.rs 0.00% 3 Missing ⚠️
...dules/project/components/AddOrEditProjectModal.tsx 0.00% 3 Missing ⚠️
...c/modules/study/components/AddOrEditStudyModal.tsx 0.00% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@             Coverage Diff              @@
##                dev    #6949      +/-   ##
============================================
- Coverage     28.19%   28.18%   -0.01%     
  Complexity     2273     2273              
============================================
  Files          1128     1128              
  Lines        139439   139439              
  Branches       2804     2804              
============================================
- Hits          39309    39302       -7     
- Misses        98498    98505       +7     
  Partials       1632     1632
Flag Coverage Δ
core 78.58% <ø> (ø)
editoast 70.09% <66.66%> (-0.03%) ⬇️
front 9.10% <73.68%> (ø)
gateway 2.45% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 83.98% <ø> (ø)

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.

@Wadjetz Wadjetz force-pushed the ebe/editoast-project-study-remove-default branch from 43a6acc to 3fa6a1e Compare April 16, 2024 07:47
Tguisnet
Tguisnet approved these changes Apr 16, 2024
View reviewed changes
Copy link
Contributor

@Tguisnet Tguisnet left a comment

Choose a reason for hiding this comment

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

lgtm

woshilapin
woshilapin approved these changes Apr 16, 2024
View reviewed changes
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 improvement (and bug fix) 🎉

@Wadjetz Wadjetz force-pushed the ebe/editoast-project-study-remove-default branch from 3fa6a1e to ecef139 Compare April 16, 2024 12:17
@Wadjetz Wadjetz force-pushed the ebe/editoast-project-study-remove-default branch from ecef139 to b050e4b Compare April 16, 2024 13:30
SharglutDev
SharglutDev approved these changes Apr 16, 2024
View reviewed changes
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

@Wadjetz Wadjetz added this pull request to the merge queue Apr 16, 2024
Merged via the queue into dev with commit 0fa03a0 Apr 16, 2024
17 checks passed
@Wadjetz Wadjetz deleted the ebe/editoast-project-study-remove-default branch April 16, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@woshilapin woshilapin woshilapin approved these changes

@Tguisnet Tguisnet Tguisnet approved these changes

@hamz2a hamz2a hamz2a approved these changes

@younesschrifi younesschrifi younesschrifi approved these changes

@SharglutDev SharglutDev SharglutDev approved these changes

@bloussou bloussou Awaiting requested review from bloussou bloussou was automatically assigned from osrd-project/test-maintainers

Assignees

@Wadjetz Wadjetz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Certain fields must not have default values in studies and projects
7 participants
@Wadjetz @codecov-commenter @woshilapin @Tguisnet @hamz2a @younesschrifi @SharglutDev