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 ongoing workspace creation shouldn't be interrupted if the users close the window too early #94

Merged
merged 5 commits into from
Oct 18, 2023

Conversation

SRombautsU
Copy link

@SRombautsU SRombautsU commented Oct 13, 2023

  • Split OnSourceControlOperationComplete() methods in dedicated delegates
  • Move workspace creation parameters into a dedicated structure
  • Move the workspace creation logic into a dedicated class: the instance is hold by the module, and kept alive when the Login window is closed (this is the actual fix)

image

@SRombautsU SRombautsU added the bug label Oct 13, 2023
@SRombautsU SRombautsU self-assigned this Oct 13, 2023
For the menu, only do that when it requires some additional work
@SRombautsU SRombautsU force-pushed the 1002462-ongoing-workspace-creation branch from a882cf0 to 66cfd6b Compare October 13, 2023 13:58
@SRombautsU SRombautsU force-pushed the 1002462-ongoing-workspace-creation branch 2 times, most recently from 18aaf56 to 453fcf0 Compare October 16, 2023 08:52
@SRombautsU SRombautsU marked this pull request as ready for review October 16, 2023 14:02
Copy link

@danielhompanera danielhompanera left a comment

Choose a reason for hiding this comment

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

LGTM
I'm interested on knowing why the dedicated delegates do the trick. Even it seems like a much more appropiated approach, I'm curious to know why it wasn't working previously. Wasn't OnSourceControlOperationComplete being called correctly when the settings dialog is was closed?

{
OnSourceControlOperationComplete(InOperation, InResult);

// Launch the following asynchronous operation

Choose a reason for hiding this comment

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

You mean "next" not "following" I think

@SRombautsU SRombautsU force-pushed the 1002462-ongoing-workspace-creation branch from f9960d3 to 5328549 Compare October 18, 2023 06:06
The single instance of FPlasticSourceControlWorkspaceCreation is hold by the Module,
and as such it is kept alive when the Login window is closed (this is the actual fix)
@SRombautsU SRombautsU force-pushed the 1002462-ongoing-workspace-creation branch from 5328549 to c5a92d1 Compare October 18, 2023 06:09
@SRombautsU
Copy link
Author

SRombautsU commented Oct 18, 2023

LGTM I'm interested on knowing why the dedicated delegates do the trick. Even it seems like a much more appropriate approach, I'm curious to know why it wasn't working previously. Wasn't OnSourceControlOperationComplete being called correctly when the settings dialog is was closed?

Indeed, this is just a refactor to make things cleaner while I was at the task, it's not the fix.
OnSourceControlOperationComplete (or any delegate) was not called if the "Login/workspace creation Window" was closed too early as the delegate was attached to the Windows itself

So the fix is in fact in the following commit:

Move the workspace creation logic into a dedicated class

The single instance of FPlasticSourceControlWorkspaceCreation is held by the Module,
and as such it is kept alive when the Login window is closed

@SRombautsU SRombautsU merged commit 769ead3 into master Oct 18, 2023
@SRombautsU SRombautsU deleted the 1002462-ongoing-workspace-creation branch October 18, 2023 06:14
@SRombautsU SRombautsU added this to the 1.9.0 milestone May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants