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
Refs #35236 - ak update requires the existence of organization #10414
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well!
Before:
$ hammer activation-key update --name=dev --organization-id=1 --release-version=8
Could not update the activation key:
undefined method `library' for nil:NilClass
After:
$ hammer activation-key update --name=dev --organization-id=1 --release-version=8
Activation key updated.
$ hammer activation-key update --name=dev --organization-id=1 --release-version="hoo"
Could not update the activation key:
Invalid release version: [hoo]
I'd also like to see a test added that reflects this change.
c78b630
to
ac6cf9a
Compare
ac6cf9a
to
cf4f033
Compare
activation_key = ActivationKey.new( | ||
:name => 'new key', :organization => @organization | ||
) | ||
assert activation_key.save |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're not testing the model's validity here, so I'd say the assert .. save
isn't needed. (It's probably not needed in the previous test either.) I'd just go with ActivationKey.create!
cf4f033
to
219a307
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lfu Test looks good, but there's a Rubocop issue.
APJ 👍
219a307
to
b25b238
Compare
What are the changes introduced in this pull request?
Make sure
organization
exists to avoid an error like:Considerations taken when implementing this change?
Followup of #10200
What are the testing steps for this pull request?
hammer activation-key update --name=test5 --organization-id=1 --release-version='willthiswork'
Before
After