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

WINDUP-1896: advanced options in analysis config are updated immediat… #566

Merged
merged 3 commits into from Apr 19, 2018

Conversation

m-brophy
Copy link
Contributor

…ely without use of page save, cancel etc buttons

…ely without use of page save, cancel etc buttons
@jsight
Copy link
Member

jsight commented Mar 28, 2018

This change doesn't seem quite right to me. This disables the dirty checking for advanced options, which disables the unsaved changes warning, and by extension of that it does disable the saving of advanced options.

However, if you change any other option (for example, the cloud readiness checkbox), the warning is still displayed and the changes are still incorrectly persisted.

I think instead of this fix, the dialog box itself is the problem. It seems to always call "save" when "yes" is clicked in the confirmation dialog. But in reality, this dialog is being overloaded with two different functions. In one case, "yes" should save, and in the other it should not.

Dialog and subscription:
https://github.com/windup/windup-web/blob/master/ui/src/main/webapp/src/app/analysis-context/analysis-context-form.component.ts#L107

Subscription Behavior:
https://github.com/windup/windup-web/blob/master/ui/src/main/webapp/src/app/analysis-context/analysis-context-form.component.ts#L134

Dialog where proceed should save:
https://github.com/windup/windup-web/blob/master/ui/src/main/webapp/src/app/analysis-context/analysis-context-form.component.ts#L308

Dialog where proceed should NOT save:
https://github.com/windup/windup-web/blob/master/ui/src/main/webapp/src/app/shared/confirm-deactivate.guard.ts#L14

Because these end up using the same dialog, the first subscription is called resulting in a save operation. Oops!

This is the part that seems to need a fix, IMO.

…ely without use of page save, cancel etc buttons
@m-brophy
Copy link
Contributor Author

m-brophy commented Apr 4, 2018

Adjusted to use the dialog subscription model

@jsight
Copy link
Member

jsight commented Apr 4, 2018

This fixes the problem with that cancellation dialog, but introduces a new problem. Steps to reproduce:

  1. Go to the project list and click "New Project"
  2. Give the project a name and click "Next"
  3. Upload a file or two (preferably a relatively large file) and click "Next"
  4. Click "Save & Continue" before the packages finish loading.
  5. A dialog box will appear, but nothing will allow you to finish the save and continue as this dialog is now broken.

@m-brophy
Copy link
Contributor Author

m-brophy commented Apr 9, 2018

The two dialogs no longer interfere with each other

@jsight jsight merged commit 03c8a3c into windup:master Apr 19, 2018
@m-brophy m-brophy deleted the WINDUP-1896 branch May 11, 2018 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants