-
Notifications
You must be signed in to change notification settings - Fork 357
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
Dialog copy - remove ids pointing to the original dialog #5856
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When copying a dialog, we load the original, allow the user to change it, and save that data as a new dialog. But, the API just saves the fields, groups and tabs, then saves the dialog, and only after that updates those tabs to point to the new dialog. No transactions. Which means that when dialog validation fails, all the copied tabs still point to the original dialog, leading to duplicate tabs appearing in the original. This does not fix the API bug or attempt to wrap it in a transaction, this just makes sure the UI won't send any obsolete ids when copying, thus limiting the impact of the API bug to extra unused tabs remaining in the db. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1713100
Checked commit https://github.com/himdel/manageiq-ui-classic/commit/4962568f5586895f95fc7d0b7093737e4d3de041 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 |
simaishi
pushed a commit
that referenced
this pull request
Jul 25, 2019
Dialog copy - remove ids pointing to the original dialog (cherry picked from commit 9c80759) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1713100
Ivanchuk backport details:
|
simaishi
pushed a commit
that referenced
this pull request
Jul 25, 2019
Dialog copy - remove ids pointing to the original dialog (cherry picked from commit 9c80759) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1733375
Hammer backport details:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When copying a dialog, we load the original, allow the user to change it, and save that data as a new dialog.
But, the API just saves the fields, groups and tabs, then saves the dialog, and only after that updates those tabs to point to the new dialog. No transactions.
Which means that when dialog validation fails, all the copied tabs still point to the original dialog, leading to duplicate tabs appearing in the original.
This does not fix the API bug or attempt to wrap it in a transaction,
this just makes sure the UI won't send any obsolete ids when copying,
thus limiting the impact of the API bug to extra unused tabs remaining in the db.
(And created ManageIQ/manageiq-api#630.)
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1713100