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: include project and study in the endpoints responses #5981

Merged
merged 1 commit into from
Dec 7, 2023
Merged

editoast: include project and study in the endpoints responses #5981

merged 1 commit into from
Dec 7, 2023

Conversation

anglnn
Copy link
Contributor

@anglnn anglnn commented Dec 4, 2023

closes #5825

@anglnn anglnn requested a review from a team as a code owner December 4, 2023 10:59
@codecov Codecov
Copy link

codecov bot commented Dec 4, 2023 •
edited
Loading

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (f3abe90) 26.51% compared to head (332e424) 26.56%.

Files Patch % Lines
editoast/src/views/study.rs 96.00% 2 Missing ⚠️
editoast/src/models/scenario.rs 85.71% 1 Missing ⚠️
editoast/src/views/scenario.rs 98.96% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##                dev    #5981      +/-   ##
============================================
+ Coverage     26.51%   26.56%   +0.05%     
  Complexity     2117     2117              
============================================
  Files           930      930              
  Lines        122714   122798      +84     
  Branches       2669     2669              
============================================
+ Hits          32542    32626      +84     
  Misses        88595    88595              
  Partials       1577     1577
Flag Coverage Δ
core 78.97% <ø> (ø)
editoast 74.65% <97.54%> (+0.10%) ⬆️
front 9.32% <ø> (ø)
gateway 2.75% <ø> (ø)

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.

leovalais
leovalais requested changes Dec 4, 2023
View reviewed changes
Copy link
Contributor

@leovalais leovalais left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good, a few comments and some transactions are missing

editoast/src/tables.rs Outdated Show resolved Hide resolved
editoast/src/views/scenario.rs Outdated Show resolved Hide resolved
editoast/src/views/study.rs Outdated Show resolved Hide resolved
editoast/src/views/study.rs Outdated Show resolved Hide resolved
editoast/src/views/scenario.rs Outdated Show resolved Hide resolved
editoast/src/views/scenario.rs Outdated Show resolved Hide resolved
editoast/src/views/study.rs Outdated Show resolved Hide resolved
@anglnn anglnn force-pushed the akz/include-project-response-endpoints branch 3 times, most recently from 8272f9b to 1aeef0d Compare December 6, 2023 08:57
leovalais
leovalais requested changes Dec 6, 2023
View reviewed changes
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.

Thanks for adding the transactions! A few comments to tidy things up a bit before merging.

editoast/src/models/study.rs Show resolved Hide resolved
editoast/src/views/study.rs Outdated Show resolved Hide resolved
editoast/src/views/study.rs Outdated Show resolved Hide resolved
editoast/src/views/study.rs Outdated Show resolved Hide resolved
editoast/src/views/study.rs Outdated Show resolved Hide resolved
editoast/src/views/scenario.rs Outdated Show resolved Hide resolved
@anglnn anglnn force-pushed the akz/include-project-response-endpoints branch from 1aeef0d to b4b5b49 Compare December 6, 2023 10:46
leovalais
leovalais requested changes Dec 6, 2023
View reviewed changes
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.

Last batch of changes, I swear 😅
You also have a failing CI test. You have to update the integration tests to accept the new scenario response schema.

editoast/src/views/study.rs Outdated Show resolved Hide resolved
editoast/src/views/study.rs Outdated Show resolved Hide resolved
editoast/src/views/study.rs Outdated Show resolved Hide resolved
editoast/src/views/scenario.rs Outdated Show resolved Hide resolved
@anglnn anglnn force-pushed the akz/include-project-response-endpoints branch from b4b5b49 to 0fb7cbc Compare December 6, 2023 11:54
@anglnn anglnn requested a review from a team as a code owner December 6, 2023 11:54
@anglnn anglnn force-pushed the akz/include-project-response-endpoints branch 2 times, most recently from c06a670 to 1cdac85 Compare December 6, 2023 11:59
shenriotpro
shenriotpro reviewed Dec 6, 2023
View reviewed changes
Copy link
Contributor

@shenriotpro shenriotpro left a comment

Choose a reason for hiding this comment

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

Thanks

tests/tests/test_scenario.py Outdated Show resolved Hide resolved
@anglnn anglnn force-pushed the akz/include-project-response-endpoints branch from 1cdac85 to e03cfc3 Compare December 6, 2023 12:01
leovalais
leovalais approved these changes Dec 6, 2023
View reviewed changes
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.

Tested, thanks! (reviewed editoast changes)

@leovalais leovalais requested a review from shenriotpro December 6, 2023 13:58
@anglnn anglnn force-pushed the akz/include-project-response-endpoints branch from 79f3ef8 to 332e424 Compare December 6, 2023 14:33
@anglnn anglnn added this pull request to the merge queue Dec 6, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 6, 2023
@multun multun added this pull request to the merge queue Dec 7, 2023
Merged via the queue into dev with commit 0adc3bc Dec 7, 2023
17 checks passed
@multun multun deleted the akz/include-project-response-endpoints branch December 7, 2023 09:21
@clarani clarani mentioned this pull request Dec 15, 2023
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shenriotpro shenriotpro shenriotpro approved these changes

@leovalais leovalais leovalais approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

editoast: include the project and the study of a scenario in the response of the projects endpoints
4 participants
@anglnn @leovalais @shenriotpro @multun