fixes #8867 - fixes activation key dynflow update passing in nil variables #4904
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
cfouant
force-pushed
the
actkey-dynflow-fix
branch
5 times, most recently
from
56e717d
to
3f63e5f
Compare
| @@ -15,15 +15,27 @@ module Katello | |||
| module ActivationKey | |||
| class Update < Actions::EntryAction | |||
| def plan(activation_key, activation_key_params) | |||
| cp_call = cp_changed?(activation_key.auto_attach, activation_key_params[:auto_attach]) || | |||
| cp_changed?(activation_key.service_level, activation_key_params[:service_level]) || | |||
| cp_changed?(activation_key.release_version, activation_key_params[:release_version]) | |||
There was a problem hiding this comment.
I might wrap this in a function call itself as well, so that further down you could just do:
if update_candlepin?(activation_key, params)
....
...
end
This encapsulates the logic and allows anyone reading the code to skip over this chunk if they don't need to see the details of it.
|
A test for the dynflow action would be nice to provide basic confirmation that the Candlepin update task is called in the right scenario and not called in the other scenario. |
ehelms
added
Waiting on contributor
Needs testing
and removed
Needs testing
Not yet reviewed
labels
cfouant
force-pushed
the
actkey-dynflow-fix
branch
from
3f63e5f
to
917cff8
Compare
theforeman-bot
added
Needs testing
Needs re-review
and removed
Waiting on contributor
labels
|
@ehelms - I added tests for the update actions in Katello, wrote tests for all three Dynflow case actions in Candlepin, encapsulated (and commented) code inside the Dynflow update function. Let me know if you have any other questions/suggestions! |
cfouant
force-pushed
the
actkey-dynflow-fix
branch
from
917cff8
to
f8a3551
Compare
|
Tested and worked great for me! ACK |
cfouant
added a commit
that referenced
this pull request
Jan 30, 2015
fixes #8867 - fixes activation key dynflow update passing in nil variables
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.
No description provided.