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.

Sign up for GitHub

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

Jump to bottom

Specify a target and resource when refreshing a dialog field #233

Merged
merged 2 commits into from Dec 4, 2017
Merged

Specify a target and resource when refreshing a dialog field #233

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0cbec91
Specify a target and resource when refreshing a dialog field
eclarizio Dec 1, 2017
574e63d
Ensure dialog passed in and resource action dialog are the same
eclarizio Dec 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 14 additions & 13 deletions app/controllers/api/service_dialogs_controller.rb
View file
Open in desktop
Expand Up @@ -11,12 +11,12 @@ def refresh_dialog_fields_resource(type, id = nil, data = nil)
service_dialog = resource_search(id, type, klass)
api_log_info("Refreshing Dialog Fields for #{service_dialog_ident(service_dialog)}")

refresh_dialog_fields_service_dialog(service_dialog, data)
refresh_dialog_fields_service_dialog(data)
end
end

def fetch_service_dialogs_content(resource)
target, resource_action = validate_dialog_content_params
target, resource_action = validate_dialog_content_params(params)
resource.content(target, resource_action, true)
end

Expand Down Expand Up @@ -49,8 +49,9 @@ def copy_resource(type, id, data)

private

def validate_dialog_content_params
return unless CONTENT_PARAMS.detect { |param| params.include?(param) }
def validate_dialog_content_params(params, required = false)
return unless CONTENT_PARAMS.detect { |param| params.include?(param) } || required

raise BadRequestError, "Must specify all of #{CONTENT_PARAMS.join(',')}" unless (CONTENT_PARAMS - params.keys).count.zero?
target_type = params['target_type'].pluralize.to_sym
target = resource_search(params['target_id'], target_type, collection_class(target_type))
Expand All @@ -62,26 +63,26 @@ def set_additional_attributes
@additional_attributes = %w(content) if attribute_selection == "all"
end

def refresh_dialog_fields_service_dialog(service_dialog, data)
def refresh_dialog_fields_service_dialog(data)
data ||= {}
dialog_fields = Hash(data["dialog_fields"])
refresh_fields = data["fields"]
return action_result(false, "Must specify fields to refresh") if refresh_fields.blank?

define_service_dialog_fields(service_dialog, dialog_fields)
service_dialog = define_service_dialog(dialog_fields, data)
Copy link
Member

Choose a reason for hiding this comment

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

with the service_dialog no longer passed in as parameter (which was RBAC checked earlier for the user via resource_search), is it possible we'll be updating a different service_dialog (data driven and not URL) ?

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 thought about that too, but the way the classic-ui was doing the refresh before was pulling the dialog off of the resource_action, since it belongs to it. So I guess I could envision a scenario where someone could call, say, /api/service_dialogs/123 with action set to refresh, and pass in a bogus resource_action_id that doesn't correspond to the dialog with the id of 123. We could compare ids, and return an error if they don't match, maybe?

Copy link
Member

Choose a reason for hiding this comment

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

maybe pass the service_dialog to refresh_dialog_fields_service_dialog and have it check there against the one defined in data and raise an error accordingly, (with added test for that). Thanks.


refresh_dialog_fields_action(service_dialog, refresh_fields, service_dialog_ident(service_dialog))
rescue => err
action_result(false, err.to_s)
end

def define_service_dialog_fields(service_dialog, dialog_fields)
ident = service_dialog_ident(service_dialog)
dialog_fields.each do |key, value|
dialog_field = service_dialog.field(key)
raise BadRequestError, "Dialog field #{key} specified does not exist in #{ident}" if dialog_field.nil?
dialog_field.value = value
end
def define_service_dialog(dialog_fields, data)
target, resource_action = validate_dialog_content_params(data, true)

workflow = ResourceActionWorkflow.new({}, User.current_user, resource_action, :target => target)

dialog_fields.each { |key, value| workflow.set_value(key, value) }
workflow.dialog
end

def service_dialog_ident(service_dialog)
Expand Down
51 changes: 47 additions & 4 deletions spec/requests/service_dialogs_spec.rb
View file
Open in desktop
Expand Up @@ -63,7 +63,7 @@
expect_result_to_have_keys(%w(content))
end

it "requires both target_id, target_type, and resource_action" do
it "requires all of target_id, target_type, and resource_action" do
api_basic_authorize action_identifier(:service_dialogs, :read, :resource_actions, :get)

get(api_service_dialog_url(nil, dialog1), :params => { :target_id => 'id' })
Expand Down Expand Up @@ -360,16 +360,59 @@ def init_dialog
api_basic_authorize action_identifier(:service_dialogs, :refresh_dialog_fields)
init_dialog

post(api_service_dialog_url(nil, dialog1), :params => gen_request(:refresh_dialog_fields, "fields" => %w(bad_field)))

post(api_service_dialog_url(nil, dialog1), :params => gen_request(
:refresh_dialog_fields,
"fields" => %w(bad_field),
"resource_action_id" => ra1.id,
"target_id" => template.id,
"target_type" => "service_template"
))
expect_single_action_result(:success => false, :message => /unknown dialog field bad_field/i)
end

it "requires all of resource_action_id, target_id, and target_type" do
api_basic_authorize action_identifier(:service_dialogs, :refresh_dialog_fields)
init_dialog

post(api_service_dialog_url(nil, dialog1), :params => gen_request(
:refresh_dialog_fields,
"fields" => %w(text1)
))

expect_single_action_result(:success => false, :message => a_string_including('Must specify all of'))
end

it "supports refresh when passing in resource_action_id, target_id, and target_type" do
api_basic_authorize action_identifier(:service_dialogs, :refresh_dialog_fields)
init_dialog

post(api_service_dialog_url(nil, dialog1), :params => gen_request(
:refresh_dialog_fields,
"fields" => %w(text1),
"resource_action_id" => ra1.id,
"target_id" => template.id,
"target_type" => "service_template"
))

expect(response.parsed_body).to include(
"success" => true,
"message" => a_string_matching(/refreshing dialog fields/i),
"href" => api_service_dialog_url(nil, dialog1),
"result" => hash_including("text1")
)
end

it "supports refresh dialog fields of valid fields" do
api_basic_authorize action_identifier(:service_dialogs, :refresh_dialog_fields)
init_dialog

post(api_service_dialog_url(nil, dialog1), :params => gen_request(:refresh_dialog_fields, "fields" => %w(text1)))
post(api_service_dialog_url(nil, dialog1), :params => gen_request(
:refresh_dialog_fields,
"fields" => %w(text1),
"resource_action_id" => ra1.id,
"target_id" => template.id,
"target_type" => "service_template"
))

expect(response.parsed_body).to include(
"success" => true,
Expand Down