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

front: rs-editor: manage multiple units #6671

Merged
merged 3 commits into from
Mar 7, 2024

Conversation

SharglutDev
Copy link
Contributor

@SharglutDev SharglutDev commented Feb 19, 2024

Fixes https://github.com/osrd-project/osrd-confidential/issues/183

  • update rollingStockValues state for multi units parameters to handle unit and min/max when they are changed
  • adapt checkRollingStockFormValidity tests to match the new state
  • adapt rollingstock e2e tests to match the new state and new min/max values
  • add a conversionFactors schema
  • add tests for conversionFactor and handleUnit functions
  • display an error message if the user switch to a unit which the conversion doesn't exist in the schema
  • update newRollingStockValues when creating a new rollingstock which now requires defaults values for multi units parameter
  • update the speed / effort curves error message if one of these values exceeds the max
  • update the rollingstock card details units to match the default ones in the form
  • move a css class (invalid message for resistances wasn't displayed)
  • fix the dropdown position which wasn't properly placed
  • refacto some InputGroup classenames with the proper style nomenclature for cx lib
  • remove the form valid state which was disabling the confirm button if one of the fields was invalid

Copy link

codecov bot commented Feb 19, 2024

Codecov Report

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

Project coverage is 28.28%. Comparing base (4ecdaa8) to head (4baace9).
Report is 2 commits behind head on dev.

Files Patch % Lines
front/src/modules/rollingStock/helpers/utils.ts 56.00% 51 Missing and 26 partials ⚠️
front/src/common/BootstrapSNCF/InputGroupSNCF.tsx 15.06% 62 Missing ⚠️
front/src/modules/rollingStock/types.ts 0.00% 44 Missing ⚠️
...llingStockEditor/RollingStockEditorFormHelpers.tsx 11.62% 38 Missing ⚠️
...nageTrainSchedule/Allowances/AllowancesActions.tsx 0.00% 11 Missing ⚠️
...onents/RollingStockCard/RollingStockCardDetail.tsx 18.18% 9 Missing ⚠️
...les/stdcmAllowances/components/StdcmAllowances.tsx 0.00% 8 Missing ⚠️
...Schedule/Allowances/AllowancesStandardSettings.tsx 0.00% 8 Missing ⚠️
...components/RollingStockEditor/CurveSpreadsheet.tsx 0.00% 2 Missing ⚠️
...nents/RollingStockEditor/formatSpreadSheetCurve.ts 0.00% 2 Missing ⚠️
... and 1 more
Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #6671      +/-   ##
============================================
+ Coverage     28.26%   28.28%   +0.01%     
  Complexity     2179     2179              
============================================
  Files          1051     1052       +1     
  Lines        130335   130736     +401     
  Branches       2578     2602      +24     
============================================
+ Hits          36839    36976     +137     
- Misses        92007    92247     +240     
- Partials       1489     1513      +24     
Flag Coverage Δ
core 79.44% <ø> (ø)
editoast 75.03% <ø> (-0.05%) ⬇️
front 8.74% <45.41%> (+0.12%) ⬆️
gateway 2.50% <ø> (ø)
railjson_generator 87.26% <ø> (ø)
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.

@SharglutDev SharglutDev force-pushed the pfn/front/rs-editor/manage-units branch 7 times, most recently from 69fa619 to 12744d4 Compare February 28, 2024 16:35
@SharglutDev SharglutDev force-pushed the pfn/front/rs-editor/manage-units branch from 12744d4 to fdb4922 Compare February 29, 2024 10:24
@SharglutDev SharglutDev marked this pull request as ready for review February 29, 2024 10:24
@SharglutDev SharglutDev requested a review from a team as a code owner February 29, 2024 10:24
@SharglutDev SharglutDev force-pushed the pfn/front/rs-editor/manage-units branch 3 times, most recently from 696f001 to 04b8ef8 Compare February 29, 2024 16:57
@kmer2016
Copy link
Contributor

kmer2016 commented Mar 1, 2024

Some bugs :

  • When change the unit from any parameters input, the select option keep on the previous unit but the value is well converted.
  • change unit seems to break the check of min and max bound.

@SharglutDev SharglutDev force-pushed the pfn/front/rs-editor/manage-units branch from 04b8ef8 to 22b4544 Compare March 4, 2024 06:59
Copy link
Contributor

@kmer2016 kmer2016 left a comment

Choose a reason for hiding this comment

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

LGTM

@SharglutDev SharglutDev force-pushed the pfn/front/rs-editor/manage-units branch from a6967db to 9963e39 Compare March 5, 2024 08:48
Copy link
Contributor

@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.

Nice work :) left some comments

@SharglutDev SharglutDev force-pushed the pfn/front/rs-editor/manage-units branch from aee261d to 8e50afd Compare March 7, 2024 10:10
@SharglutDev SharglutDev force-pushed the pfn/front/rs-editor/manage-units branch from 8230803 to 1f2aa16 Compare March 7, 2024 13:20
@SharglutDev SharglutDev force-pushed the pfn/front/rs-editor/manage-units branch 2 times, most recently from 70e08fe to 3d0c424 Compare March 7, 2024 13:30
Copy link
Contributor

@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.

LGTM and tested

@SharglutDev SharglutDev force-pushed the pfn/front/rs-editor/manage-units branch from 3d0c424 to db03948 Compare March 7, 2024 13:45
@SharglutDev SharglutDev force-pushed the pfn/front/rs-editor/manage-units branch from db03948 to 4baace9 Compare March 7, 2024 13:54
@SharglutDev SharglutDev added this pull request to the merge queue Mar 7, 2024
Merged via the queue into dev with commit 629612c Mar 7, 2024
22 checks passed
@SharglutDev SharglutDev deleted the pfn/front/rs-editor/manage-units branch March 7, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants