🐛 Reset wizard form after analysis run #1267

ibolton336 · 2023-08-08T17:35:08Z

Resolves Sometimes wizard state is persisted between analysis runs #1256
When closing the wizard, the taskgroup state hangs around between analysis runs causing problems. This PR cleans up the react state when closing/submitting the analysis wizard

codecov · 2023-08-08T17:42:57Z

Codecov Report

Patch coverage: 8.33% and project coverage change: -0.18% ⚠️

Comparison is base (e47565e) 43.28% compared to head (f1a9151) 43.11%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1267      +/-   ##
==========================================
- Coverage   43.28%   43.11%   -0.18%     
==========================================
  Files         143      143              
  Lines        4288     4291       +3     
  Branches      999     1000       +1     
==========================================
- Hits         1856     1850       -6     
- Misses       2354     2361       +7     
- Partials       78       80       +2

Flag	Coverage Δ
client	`43.11% <8.33%> (-0.18%)`	⬇️
server	`∅ <ø> (∅)`

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

Files Changed	Coverage Δ
...s/applications/analysis-wizard/analysis-wizard.tsx	`62.99% <8.33%> (-5.56%)`	⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

gitdallas

although would it make sense to call handleClose here? does both reset() and onClose(), i'm unsure about the delete task group stuff though.

ibolton336 · 2023-08-08T18:21:55Z

although would it make sense to call handleClose here? does both reset() and onClose(), i'm unsure about the delete task group stuff though.

You're right on the money. Our delete task group stuff was very wrong and needs to be refactored.

ibolton336 · 2023-08-08T18:40:44Z

@gitdallas Updated - good idea to consolidate that logic.

gitdallas · 2023-08-08T18:48:10Z

client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx

    if (id && stepIdReached < (id as number)) setStepIdReached(id as number);
+    if (id === 2) {


What is 2? Any way we can use a step property or something instead?

Looks like id is a string, but line 302 is comparing it as a number.

Updated to use the enum to identify the step as

StepId.SetTargets (2)

Looks like the string/number type is coming from PF.

gildub

Please see inline.

client/src/app/pages/applications/applications-table-analyze/applications-table-analyze.tsx

Sorry but some changes are needed

gitdallas · 2023-08-08T19:44:36Z

client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx

@@ -286,28 +280,37 @@ export const AnalysisWizard: React.FC<IAnalysisWizard> = ({
  };

  const isModeValid = applications.every((app) => isModeSupported(app, mode));
+  const resetWizard = () => {


as we are only using this on close and reset doesn't intuitively mean it will close something, i wonder if it should be replacing the close handler as handleClose ("close" including reset logic is more intuitive imo).. and perhaps have the delete stuff as a parameter flag, or have another deleteAndClose that does the delete and then calls this

@ibolton336 - actually the delete is more of a cancel right? maybe just handleCancel which could delete and then call the close function

Ah that is a good idea @gitdallas.

Signed-off-by: ibolton336 <ibolton@redhat.com>

- Improve handling of taskgroup state by using contextAPI - Async create the taskgroup before uploading the custom binary in the single app use case. This is currently broken because we require a task group to exist before navigating to the upload file screen. This regression was caused by the move away from useEffect here #1267 Resolves https://issues.redhat.com/browse/MTA-1175 Signed-off-by: ibolton336 <ibolton@redhat.com>

ibolton336 force-pushed the reset-form-after-analysis-run branch 2 times, most recently from 86cf127 to c777e34 Compare August 8, 2023 17:35

gitdallas previously approved these changes Aug 8, 2023

View reviewed changes

ibolton336 marked this pull request as draft August 8, 2023 18:21

ibolton336 marked this pull request as ready for review August 8, 2023 18:25

ibolton336 force-pushed the reset-form-after-analysis-run branch from a002422 to bfd1bd3 Compare August 8, 2023 18:25

ibolton336 force-pushed the reset-form-after-analysis-run branch from 680cf97 to 55b59ad Compare August 8, 2023 18:41

gitdallas self-requested a review August 8, 2023 18:41

gitdallas reviewed Aug 8, 2023

View reviewed changes

ibolton336 force-pushed the reset-form-after-analysis-run branch from aaf2f70 to f1a062c Compare August 8, 2023 18:55

ibolton336 requested a review from gitdallas August 8, 2023 18:57

gildub reviewed Aug 8, 2023

View reviewed changes

gitdallas reviewed Aug 8, 2023

View reviewed changes

ibolton336 force-pushed the reset-form-after-analysis-run branch 2 times, most recently from ec75830 to 652497c Compare August 8, 2023 20:25

🐛 Cleanup wizard state on cancel / submit

f1a9151

Signed-off-by: ibolton336 <ibolton@redhat.com>

ibolton336 force-pushed the reset-form-after-analysis-run branch from 652497c to f1a9151 Compare August 8, 2023 20:27

ibolton336 requested review from gildub and gitdallas August 8, 2023 20:30

gitdallas approved these changes Aug 8, 2023

View reviewed changes

ibolton336 merged commit 3d72ff9 into konveyor:main Aug 8, 2023

ibolton336 deleted the reset-form-after-analysis-run branch August 8, 2023 23:26

ibolton336 mentioned this pull request Aug 11, 2023

🐛 Add taskgroup when uploading binary #1280

Merged

konveyor-release-bot mentioned this pull request Nov 22, 2023

operator konveyor-operator (0.3.0-beta.3) k8s-operatorhub/community-operators#3570

Merged

konveyor-release-bot mentioned this pull request Nov 22, 2023

operator konveyor-operator (0.3.0-beta.3) redhat-openshift-ecosystem/community-operators-prod#3652

Merged

This was referenced Jan 23, 2024

operator konveyor-operator (0.3.0) k8s-operatorhub/community-operators#3793

Merged

operator konveyor-operator (0.3.0) redhat-openshift-ecosystem/community-operators-prod#3927

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Reset wizard form after analysis run #1267

🐛 Reset wizard form after analysis run #1267

ibolton336 commented Aug 8, 2023 •

edited

Loading

codecov bot commented Aug 8, 2023 •

edited

Loading

gitdallas left a comment

ibolton336 commented Aug 8, 2023

ibolton336 commented Aug 8, 2023

gitdallas Aug 8, 2023 •

edited

Loading

ibolton336 Aug 8, 2023 •

edited

Loading

ibolton336 Aug 8, 2023

gildub left a comment

gitdallas Aug 8, 2023

gitdallas Aug 8, 2023

ibolton336 Aug 8, 2023

ibolton336 Aug 8, 2023

		if (id && stepIdReached < (id as number)) setStepIdReached(id as number);
		if (id === 2) {

🐛 Reset wizard form after analysis run #1267

🐛 Reset wizard form after analysis run #1267

Conversation

ibolton336 commented Aug 8, 2023 • edited Loading

codecov bot commented Aug 8, 2023 • edited Loading

Codecov Report

gitdallas left a comment

Choose a reason for hiding this comment

ibolton336 commented Aug 8, 2023

ibolton336 commented Aug 8, 2023

gitdallas Aug 8, 2023 • edited Loading

Choose a reason for hiding this comment

ibolton336 Aug 8, 2023 • edited Loading

Choose a reason for hiding this comment

ibolton336 Aug 8, 2023

Choose a reason for hiding this comment

gildub left a comment

Choose a reason for hiding this comment

gitdallas Aug 8, 2023

Choose a reason for hiding this comment

gitdallas Aug 8, 2023

Choose a reason for hiding this comment

ibolton336 Aug 8, 2023

Choose a reason for hiding this comment

ibolton336 Aug 8, 2023

Choose a reason for hiding this comment

ibolton336 commented Aug 8, 2023 •

edited

Loading

codecov bot commented Aug 8, 2023 •

edited

Loading

gitdallas Aug 8, 2023 •

edited

Loading

ibolton336 Aug 8, 2023 •

edited

Loading