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

Use the #id reader instead of the nil @id variable #90

Merged

Conversation

erran
Copy link
Contributor

@erran erran commented Nov 5, 2014

When template ID argument finally made it to url encoding when String#gsub was called on @id the variable would be nil. This change uses the id getter method instead of the absent @id instance variable (ScanTemplate never sets it while ScanTemplateSummary does).

@@ -423,7 +423,7 @@ def self.copy(nsc, id)
# @param [Connection] nsc API connection to a Nexpose console.
#
def delete(nsc)
nsc.delete_scan_template(@id)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdaines-r7 Should ScanTemplate set @id? ScanTemplateSummary does. The use case is different since we read/write to the XML every time for the id of the full scan template class though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, your fix is what it should do. Surprised this wasn't noticed earlier, but guess people were using the other method.

@erran-r7
Copy link
Contributor

erran-r7 commented Nov 5, 2014

@mdaines-r7 @AdamBunn Can we push a gem release for #89 and this PR?

@abunn-r7
Copy link
Contributor

abunn-r7 commented Nov 5, 2014

You're correct. That is the issue here.

mdaines-r7 added a commit that referenced this pull request Nov 5, 2014
…e-delete

Use the #id reader instead of the nil @id variable
@mdaines-r7 mdaines-r7 merged commit 8154199 into rapid7:master Nov 5, 2014
@mdaines-r7
Copy link
Contributor

@erran-r7 erran-r7 deleted the fix-no-method-error-in-scan_template-delete branch November 5, 2014 21:27
@erran-r7
Copy link
Contributor

erran-r7 commented Nov 5, 2014

@mdaines-r7 Awesome thanks! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants