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

After cloning new project, go to project when done #1810

Merged
merged 1 commit into from
May 7, 2024

Conversation

rmunn
Copy link
Collaborator

@rmunn rmunn commented May 6, 2024

Fixes #1806

Description

The "Clone new project" page was setting a locationChange event handler to cancel the clone-status timer. However, since the clone-status page counts as a different locaiton, the event was firing as soon as the user pressed the "Get Started" button, cancelling the clone-status timer too soon so the "cloning new project" page was never detecting that the clone completed.

By simply removing the locationChange event handler, we can let the clone status proceed. It changes from "Getting initial data" to "syncing" pretty soon, but that's fine. Once the status goes to idle, the existing code already cancels the timer and loads the project page, so there's no need for any further changes.

Checklist

  • I have labeled my PR with: bug, feature, engineering, security fix or testing
  • I have performed a self-review of my own code
  • I have reviewed the title & description of this PR which I will use as the squashed PR commit message
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have enabled auto-merge (optional)

Testing

Testers, use the following instructions against our staging environment. Post your findings as a comment and include any meaningful screenshots, etc.

Describe how to verify your changes and provide any necessary test data.

  • Test A

The locationchange event was firing too early, cancelling the
clone-status timer too soon so the "cloning new project" page was
never detecting that the clone completed.
@rmunn rmunn added the bug An existing problem with our app in production label May 6, 2024
@rmunn rmunn self-assigned this May 6, 2024
@rmunn rmunn requested a review from megahirt May 6, 2024 06:11
Copy link

github-actions bot commented May 6, 2024

Unit Test Results

362 tests   362 ✅  12s ⏱️
 37 suites    0 💤
  1 files      0 ❌

Results for commit d71270b.

Copy link
Collaborator

@myieye myieye left a comment

Choose a reason for hiding this comment

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

  • I synced a new project and then
  • synced the same project again

Both cases worked perfectly 🥳.
Wow, it's really satisfying having this fixed!
Nice work @rmunn!

Copy link
Collaborator

@megahirt megahirt left a comment

Choose a reason for hiding this comment

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

Agree with @myieye - way to go on fixing this!

@megahirt megahirt merged commit 80ade38 into develop May 7, 2024
15 of 17 checks passed
@megahirt megahirt deleted the bug/go-to-project-after-cloning branch May 7, 2024 10:17
@megahirt
Copy link
Collaborator

megahirt commented May 7, 2024

merged this PR and bypassed the e2e checks which prevented the automerge. The e2e failing checks are firefox only and are not real errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An existing problem with our app in production
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

bug: initial clone dialog never updates when finished cloning
3 participants