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: drop tsv2 switch #8217

Merged
merged 2 commits into from
Jul 30, 2024
Merged

front: drop tsv2 switch #8217

merged 2 commits into from
Jul 30, 2024

Conversation

SharglutDev
Copy link
Contributor

@SharglutDev SharglutDev commented Jul 26, 2024

Operational studies and stdcm both now use tsv2 model by default

e2e tests have been adapted and some removed (see 2nd commit) in accordance with @Maymanaf

close #8059

@codecov-commenter
Copy link

codecov-commenter commented Jul 26, 2024

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

Codecov Report

Attention: Patch coverage is 3.76344% with 179 lines in your changes missing coverage. Please review.

Project coverage is 27.24%. Comparing base (4388f9d) to head (5cbbfb5).
Report is 2 commits behind head on dev.

Files Patch % Lines
...ainSchedule/ManageTrainScheduleMap/RenderPopup.tsx 0.00% 31 Missing ⚠️
...les/scenario/components/AddOrEditScenarioModal.tsx 0.00% 25 Missing ⚠️
...ules/rollingStock/components/RollingStockCurve.tsx 0.00% 23 Missing ⚠️
...o/components/ScenarioExplorer/ScenarioExplorer.tsx 0.00% 17 Missing and 1 partial ⚠️
...e/components/ManageTrainSchedule/TrainSettings.tsx 0.00% 16 Missing ⚠️
front/src/applications/stdcm/views/StdcmConfig.tsx 0.00% 12 Missing ⚠️
...ponents/ScenarioExplorer/ScenarioExplorerModal.tsx 0.00% 10 Missing ⚠️
...ainschedule/components/ManageTrainSchedule/Map.tsx 0.00% 10 Missing ⚠️
...nents/RollingStockCard/RollingStockCardButtons.tsx 0.00% 8 Missing ⚠️
front/src/applications/stdcm/hooks/useStdcm.ts 0.00% 5 Missing ⚠️
... and 9 more

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

Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #8217      +/-   ##
============================================
+ Coverage     27.18%   27.24%   +0.05%     
  Complexity     2127     2127              
============================================
  Files          1310     1309       -1     
  Lines        157758   157328     -430     
  Branches       3256     3254       -2     
============================================
- Hits          42887    42863      -24     
+ Misses       112891   112487     -404     
+ Partials       1980     1978       -2     
Flag Coverage Δ
core 75.49% <ø> (ø)
editoast 64.21% <ø> (-0.04%) ⬇️
front 10.41% <3.76%> (+0.02%) ⬆️
gateway 2.03% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 73.18% <ø> (ø)

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/drop-tsv2-switch branch 2 times, most recently from 1c80d71 to 390f484 Compare July 26, 2024 12:14
@SharglutDev SharglutDev marked this pull request as ready for review July 26, 2024 12:27
@SharglutDev SharglutDev requested a review from a team as a code owner July 26, 2024 12:27
Copy link
Contributor

@Uriel-Sautron Uriel-Sautron left a comment

Choose a reason for hiding this comment

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

Review and tested on operational studie and stdcm no problem for me.
I'll keep testing.

@SharglutDev SharglutDev force-pushed the pfn/front/drop-tsv2-switch branch from 8d14ac1 to 2589d39 Compare July 26, 2024 16:57
Copy link
Contributor

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

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.

The code LGTM, I still need to test:

  • Eex
  • stdcm
  • rolling stock editor

@SharglutDev SharglutDev force-pushed the pfn/front/drop-tsv2-switch branch from 774cfbb to 5274097 Compare July 29, 2024 13:31
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.

Code looks good to me, only found 2 bugs:

  • when I select a rolling stock with heating or air conditioning and reopen the rolling stock selector, then the selected rolling stock is correct but has not the correct mode (it has standard)
  • when editing a rolling stock, in the effort curves tab, there is a missing translation for the air conditioning tag

@SharglutDev
Copy link
Contributor Author

Should be fixed :)

@SharglutDev SharglutDev requested a review from clarani July 29, 2024 16:39
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, tested ✅

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.

Last bug: if I duplicate a rollingStock (tgv 2N2 for instance) and I edit it, there is a missing translation:
image

Operational studies and stdcm both now use tsv2 model by default
- e2e tests are now using tsv2 model
- remove allowances e2e tests as they are deprecated
- rolling stock comfort type properly uses tsv2 model
@SharglutDev SharglutDev force-pushed the pfn/front/drop-tsv2-switch branch from 7da8861 to 5cbbfb5 Compare July 30, 2024 12:29
@SharglutDev SharglutDev enabled auto-merge July 30, 2024 12:37
@SharglutDev SharglutDev added this pull request to the merge queue Jul 30, 2024
Merged via the queue into dev with commit c891ab9 Jul 30, 2024
20 checks passed
@SharglutDev SharglutDev deleted the pfn/front/drop-tsv2-switch branch July 30, 2024 12:47
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.

Drop TSV2 switch
5 participants