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

Fix three user provided service create/edit stepper bugs #3545

Merged
merged 3 commits into from
Apr 30, 2019

Conversation

richard-cox
Copy link
Contributor

@richard-cox richard-cox commented Apr 29, 2019

  • Stepper/Navigation
    • Previously the user couldn't return to the 'specify service details' step after going back to the 'select cf/org/space' step. This has now been fixed. Issue revolved around a guard in place to ensure navigation only happens after we've fetched the apps. This caused the onNext process to just wait forever - fixes User provided service instance: Create: Cannot return to details step #3543
    • when arriving at the 'specify service details` step we were calling a non-existent onEnter
  • 'specify service details' step - when editing
    • We now correctly allow next once the tags are changed
    • We now only allow next if the values have changed

- current implementation was put in place to ensure skipApps$ resolves before we continue to the next step
- there was a bug where if user returns to step then continues again, as nothing had changed, onNext would never resolve
- now move appsEmitted to more core observable
@cfdreddbot
Copy link

✅ Hey richard-cox! The commit authors and yourself have already signed the CLA.

@codecov
Copy link

codecov bot commented Apr 29, 2019

Codecov Report

Merging #3545 into v2-master will decrease coverage by 0.01%.
The diff coverage is 57.57%.

@@              Coverage Diff              @@
##           v2-master    #3545      +/-   ##
=============================================
- Coverage      51.76%   51.75%   -0.02%     
=============================================
  Files            720      720              
  Lines          20201    20217      +16     
  Branches        3605     3610       +5     
=============================================
+ Hits           10458    10463       +5     
- Misses          9743     9754      +11

Copy link
Contributor

@nwmac nwmac left a comment

Choose a reason for hiding this comment

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

LGTM

@nwmac nwmac merged commit 0dc5583 into v2-master Apr 30, 2019
KlapTrap added a commit that referenced this pull request Apr 30, 2019
* v2-master: (722 commits)
  When no space quota has been assigned fall back on more realistive quota (#3541)
  Fix three user provided service create/edit stepper bugs (#3545)
  Fix SSO connect button not enabled
  Return default unknown endpoint type (#3517)
  Metrics chart is in the SUSE repository
  Ensure we have docker registry variables
  Fix registry change
  Final change
  Fix nav icon
  Ensure PR uses new branch
  Last fix
  Remove upstream specific #4
  Add helm chart PR creation
  Remove upstream specific #3
  Remove upstream specific #2
  Remove upstream specific #1
  Tweak side nav icon padding to align with tabs header
  Don't log key
  Fix upload of helm chart
  More tweaks
  ...
@nwmac nwmac deleted the fix-upsi-tags branch October 16, 2020 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User provided service instance: Create: Cannot return to details step
3 participants