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

Fixes #24489 - add a command for report templates #396

Merged
merged 3 commits into from
Nov 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/hammer_cli_foreman.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ def self.exception_handler_class
:warning => _('%{report} command is deprecated and will be removed in one of the future versions. Please use %{config_report} command instead.') % {:report => 'report', :config_report => 'config-report'}
)

HammerCLI::MainCommand.lazy_subcommand('report-template', _("Manipulate report templates"),
'HammerCLIForeman::ReportTemplate', 'hammer_cli_foreman/report_template',
)

HammerCLI::MainCommand.lazy_subcommand('config-report', _("Browse and read reports"),
'HammerCLIForeman::ConfigReport', 'hammer_cli_foreman/config_report'
)
Expand Down
45 changes: 45 additions & 0 deletions lib/hammer_cli_foreman/commands.rb
Original file line number Diff line number Diff line change
Expand Up @@ -519,4 +519,49 @@ def self.success_message(msg = nil)
super(msg) || default_message(_('The %{resource_name} has been disassociated.'))
end
end

class DownloadCommand < HammerCLIForeman::SingleResourceCommand
Copy link
Member

Choose a reason for hiding this comment

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

Do you plan to re-use this command somewhere else?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what other API endpoints use this patterns. @ares may know.

Copy link
Member

@tstrachota tstrachota Nov 6, 2018

Choose a reason for hiding this comment

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

Well I see some potential in using it for partition tables and provisioning templates commands. I just wanted to know whether there are any plans for such future refactoring.

action :download

def self.command_name(name = nil)
super(name) || "download"
end

def request_options
{ :response => :raw }
end

option "--path", "PATH", _("Path to directory where downloaded content will be saved"),
:attribute_name => :option_path

def execute
response = send_request
if option_path
filepath = store_response(response)
print_message(_('The response has been saved to %{path}s.'), {:path => filepath})
else
puts response.body
end
return HammerCLI::EX_OK
end

def default_filename
"Downloaded-#{Time.new.strftime("%Y-%m-%d")}.txt"
end

private

def store_response(response)
if response.headers.key?(:content_disposition)
suggested_filename = response.headers[:content_disposition].match(/filename="(.*)"/)
Copy link
Member

Choose a reason for hiding this comment

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

👍 Elegant solution for removing the quotes ;)

end
filename = suggested_filename ? suggested_filename[1] : default_filename
Copy link
Member

Choose a reason for hiding this comment

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

Nice, thanks for adding the default name.

path = option_path.dup
path << '/' unless path.end_with? '/'
raise _("Cannot save file: %s does not exist") % path unless File.directory?(path)
filepath = path + filename
File.write(filepath, response.body)
filepath
end
end
end
161 changes: 161 additions & 0 deletions lib/hammer_cli_foreman/report_template.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
module HammerCLIForeman

class ReportTemplate < HammerCLIForeman::Command
resource :report_templates

class ListCommand < HammerCLIForeman::ListCommand
output do
field :id, _("Id")
field :name, _("Name")
end

build_options
end

class InfoCommand < HammerCLIForeman::InfoCommand
output ListCommand.output_definition do
field :locked, _("Locked"), Fields::Boolean
field :default, _("Default"), Fields::Boolean
HammerCLIForeman::References.timestamps(self)
HammerCLIForeman::References.taxonomies(self)
collection :inputs, _("Template inputs") do
field :id, _("Id"), Fields::Id
field :name, _('Name')
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to show input's id?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sort of, just if you want to delete the inputs using template-input delete command. It is minor case, so id doesn't have to be there if it improve UX

Copy link
Member

Choose a reason for hiding this comment

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

I'd be for adding IDs as we do it everywhere else - with type Id. This way it won't be shown unless users pass --show-ids.

field :description, _('Description')
field :required, _("Required"), Fields::Boolean
field :options, _('Options'), Fields::List, :hide_blank => true
end
end

def extend_data(template)
template[:inputs] = load_inputs
template
end

build_options

private

def load_inputs
template_inputs_api = HammerCLIForeman.foreman_api.resource(:template_inputs)
params = {:template_id => options['option_id']}
params[:organization_id] = options['option_organization_id'] if options['option_organization_id']
params[:location_id] = options['option_location_id'] if options['option_locations_id']
HammerCLIForeman.collection_to_common_format(template_inputs_api.call(:index, params))
end
end

class DumpCommand < HammerCLIForeman::InfoCommand
command_name "dump"
desc _("View report content")

def print_data(report_template)
puts report_template["template"]
end

build_options
end

class GenerateCommand < HammerCLIForeman::DownloadCommand
command_name "generate"
action :generate
desc _("Generate report")

option '--inputs', 'INPUTS', N_('Specify inputs'),
:format => HammerCLI::Options::Normalizers::KeyValueList.new

def default_filename
"Report-#{Time.new.strftime("%Y-%m-%d")}.txt"
end

def request_params
params = super
params['input_values'] = option_inputs || {}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe use default value in the option definition?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd prefer not to to distinguish inputs were not provided by the user

params
end

build_options
end


class CreateCommand < HammerCLIForeman::CreateCommand
option ['--interactive', '-i'], :flag, _('Open empty template in an $EDITOR. Upload the result')
option "--file", "LAYOUT", _("Path to a file that contains the report template content"),
:attribute_name => :option_template, :format => HammerCLI::Options::Normalizers::File.new

validate_options do
any(:option_interactive, :option_template).required
end

def request_params
params = super
if option_interactive?
params['report_template']['template'] = HammerCLI.open_in_editor(
'', content_type: 'report_template', suffix: '.erb')
end
params
end

success_message _("Report template created.")
failure_message _("Could not create the report template")

build_options :without => [:template]
end


class UpdateCommand < HammerCLIForeman::UpdateCommand
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't one of --interactive and --file be required here too?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so, you can update also other attributes (name, default)

option ['--interactive', '-i'], :flag, _('Dump existing template and open it in an $EDITOR. Update with the result')
option '--file', 'REPORT', _("Path to a file that contains the report template content"), :attribute_name => :option_template,
:format => HammerCLI::Options::Normalizers::File.new

def request_params
params = super
if option_interactive?
template = load_template
params['report_template']['template'] = HammerCLI.open_in_editor(
template['template'], content_type: 'report_template', suffix: '.erb')
end
params
end

success_message _("Report template updated.")
failure_message _("Could not update the report template")

build_options :without => [:template]

private

def load_template
template_api = HammerCLIForeman.foreman_api.resource(:report_templates)
params = {:id => options['option_id']}
params[:organization_id] = options['option_organization_id'] if options['option_organization_id']
params[:location_id] = options['option_location_id'] if options['option_locations_id']
HammerCLIForeman.record_to_common_format(template_api.call(:show, params))
end
end

class CloneCommand < HammerCLIForeman::UpdateCommand
action :clone
command_name 'clone'

success_message _('Report template cloned.')
failure_message _('Could not clone the report template')

validate_options do
option(:option_new_name).required
end

build_options
end

class DeleteCommand < HammerCLIForeman::DeleteCommand
success_message _("Report template deleted.")
failure_message _("Could not delete the report template")

build_options
end

autoload_subcommands
end

end
1 change: 1 addition & 0 deletions test/data/1.20/foreman_api.json

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions test/functional/hostgroup/create_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ module HammerCLIForeman
api_expects(:hostgroups, :create).with_params({
:hostgroup => {
:name => 'hg1',
:parent_id => '1'
:parent_id => 1
}
})
run_cmd(%w(hostgroup create --name hg1 --parent-id 1))
Expand Down Expand Up @@ -207,7 +207,7 @@ module HammerCLIForeman
api_expects(:hostgroups, :create).with_params({
:hostgroup => {
:name => 'hg1',
:puppet_ca_proxy_id => '1'
:puppet_ca_proxy_id => 1
}
})
run_cmd(%w(hostgroup create --name hg1 --puppet-ca-proxy-id 1))
Expand Down Expand Up @@ -250,7 +250,7 @@ module HammerCLIForeman
api_expects(:hostgroups, :create).with_params({
:hostgroup => {
:name => 'hg1',
:puppet_proxy_id => '1'
:puppet_proxy_id => 1
}
})
run_cmd(%w(hostgroup create --name hg1 --puppet-proxy-id 1))
Expand Down
6 changes: 3 additions & 3 deletions test/functional/hostgroup/update_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ module HammerCLIForeman
it 'allows parent hostgroup id' do
api_expects(:hostgroups, :update).with_params({
:id => '1',
:hostgroup => { :parent_id => '1' }
:hostgroup => { :parent_id => 1 }
})
run_cmd(%w(hostgroup update --id 1 --parent-id 1))
end
Expand Down Expand Up @@ -204,7 +204,7 @@ module HammerCLIForeman
it 'allows puppet ca proxy id' do
api_expects(:hostgroups, :update).with_params({
:id => '1',
:hostgroup => { :puppet_ca_proxy_id => '1' }
:hostgroup => { :puppet_ca_proxy_id => 1 }
})
run_cmd(%w(hostgroup update --id 1 --puppet-ca-proxy-id 1))
end
Expand Down Expand Up @@ -245,7 +245,7 @@ module HammerCLIForeman
it 'allows puppet proxy id' do
api_expects(:hostgroups, :update).with_params({
:id => '1',
:hostgroup => { :puppet_proxy_id => '1' }
:hostgroup => { :puppet_proxy_id => 1 }
})
run_cmd(%w(hostgroup update --id 1 --puppet-proxy-id 1))
end
Expand Down
Loading