-
Notifications
You must be signed in to change notification settings - Fork 118
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
Added vm_retire_extend method for new email. #366
Added vm_retire_extend method for new email. #366
Conversation
@miq-bot add_label bug |
@tinaafitz Please review |
c644e63
to
ba28fe9
Compare
end | ||
|
||
def retirement_extend_days | ||
@vm_retire_extend_days ||= @handle.object['vm_retire_extend_days'] || 14 |
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.
@billfitzgerald0120
Can you add a constant for 14 at the top of the class
DEFAULT_EXTENSION_DAYS = 14
And then in the method you could use
``
@vm_retire_extend_days ||= @handle.object['vm_retire_extend_days'] || DEFAULT_EXTENSION_DAYS
@billfitzgerald0120 |
ba28fe9
to
02bf557
Compare
@mkanoor Added a constant, changed vm.name to vm in multiple places. Coverage is 100% except file statement in method. |
def extend_retirement | ||
@handle.log("info", "Number of days to extend: <#{retirement_extend_days}>") | ||
@handle.log("info", "VM: <#{vm}> current retirement date is #{vm.retires_on}") | ||
@handle.log("info", "Extending retirement <#{retirement_extend_days}> days for VM: <#{vm}>") |
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.
@billfitzgerald0120 this still needs to be vm.name otherwise you are dealing with the whole vm object.
|
||
def extend_retirement | ||
@handle.log("info", "Number of days to extend: <#{retirement_extend_days}>") | ||
@handle.log("info", "VM: <#{vm}> current retirement date is #{vm.retires_on}") |
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.
@billfitzgerald0120 vm should be vm.name
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.
@mkanoor Changed vm back to vm.name.
Updated System/Request/vm_retire_extend instance to include vm_retire_extend method for new email. Created tests Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1603881 Adding tests Updated method
02bf557
to
1d76f6d
Compare
Checked commit billfitzgerald0120@1d76f6d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
@tinaafitz Please review |
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.
@billfitzgerald0120 Looks good.
@mkanoor Please review.
Updated System/Request/vm_retire_extend instance to include vm_retire_extend method.
Created tests
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1603881