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(InstallWizard): Fix exception when InstallWizard completed #14092

Merged
merged 2 commits into from
Jul 19, 2019

Conversation

BYK
Copy link
Member

@BYK BYK commented Jul 19, 2019

Since we don't have tests for the submit flow, we did not catch a regression from #13798 where the event handler was not bound to the class instance, causing an this.setState is not defined error when submitting the welcome form.

This patch fixes the issue. Will add tests to cover this soon after.

Since we don't have tests for the submit flow, we did not catch a regression from #13798 where the event handler was not bound to the class instance, causing an `this.setState` is not defined error when submitting the welcome form.

This patch fixes the issue. Will add tests to cover this soon after.
@BYK BYK requested a review from lynnagara July 19, 2019 18:09
dashed
dashed previously requested changes Jul 19, 2019
src/sentry/static/sentry/app/views/app.jsx Outdated Show resolved Hide resolved
onConfigured() {
this.setState({needsUpgrade: false});
}
onConfigured = () => this.setState({needsUpgrade: false});
Copy link
Member

Choose a reason for hiding this comment

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

👍

@BYK BYK marked this pull request as ready for review July 19, 2019 20:49
@BYK
Copy link
Member Author

BYK commented Jul 19, 2019

I have verified that this works locally. We'll need to add a dedicated acceptance test suite for the welcome page which I'll do later on.

Copy link
Member

@lynnagara lynnagara 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 fixing. Would be great to have tests /snapshots later - it's easy to inadvertently break this page

@BYK BYK dismissed dashed’s stale review July 19, 2019 20:58

Updated with the fix.

@BYK BYK merged commit cbc7b96 into master Jul 19, 2019
@BYK BYK deleted the fix-install-wizard branch July 19, 2019 20:58
HazAT added a commit that referenced this pull request Jul 23, 2019
* master: (25 commits)
  ref(onboarding): Fix install promprt URL (#14106)
  fix(app-platform): Allow GET requests for published apps (#14109)
  feat: Update Group.get_latest_event to use Snuba event (#14039)
  ref(onboarding): Rename wizardNew -> onboarding (#14104)
  feat(apm): Update props to address proptype warnings for new transaction attributes (SEN-800) (#14040)
  ref(ui): Move and codesplit `ProjectPlugins` (#13952)
  feat(typescript): Add TypeScript compatibility (#13786)
  ref(templates): Remove unused content block default (#14090)
  ref(less): Remove unused admin.less (#14097)
  ref(onobarding): Remove old onboarding experience (#14066)
  fix(ui) Fix missing conditions in tag bars (#14063)
  ref(suspect-commits): Add hook (#14057)
  ref(frontend): Segment frontend web urls (#14096)
  feat(suspect-commits): Add analytics events (#14080)
  feat(servicehooks): Update servicehook URLs (#14093)
  license: Remove license headers (#14095)
  ref(templates): Remove unused account_nav (#14091)
  fix: Disable transaction events in store (#14088)
  fix(InstallWizard): Fix exception when InstallWizard completed (#14092)
  ref(admin): Fix thrashing on stat charts (#14094)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants