Skip to content

Commit

Permalink
Merge pull request #15083 from jntullo/bulk/incorrect_href_bulk_tag
Browse files Browse the repository at this point in the history
Return correct href when bulk assigning/unassigning tags
(cherry picked from commit da20831)

https://bugzilla.redhat.com/show_bug.cgi?id=1460294
  • Loading branch information
abellotti authored and simaishi committed Jun 9, 2017
1 parent eea703a commit f9deb1d
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
6 changes: 4 additions & 2 deletions app/controllers/api/base_controller/results.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ def action_result(success, message = nil, options = {})
res[:message] = message if message.present?
res[:result] = options[:result] unless options[:result].nil?
add_task_to_result(res, options[:task_id]) if options[:task_id].present?
add_parent_href_to_result(res, options[:parent_id]) if options[:parent_id].present?
res
end

Expand All @@ -16,8 +17,9 @@ def add_href_to_result(hash, type, id)
hash
end

def add_parent_href_to_result(hash)
hash[:href] = "#{@req.api_prefix}/#{@req.collection}/#{@req.c_id}"
def add_parent_href_to_result(hash, parent_id = nil)
return if hash[:href].present?
hash[:href] = "#{@req.api_prefix}/#{@req.collection}/#{parent_id ? parent_id : @req.c_id}"
hash
end

Expand Down
7 changes: 3 additions & 4 deletions app/controllers/api/subcollections/tags.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ def tag_subcollection_action(tag_spec)
else
result = action_result(false, "Missing tag category or name")
end

add_parent_href_to_result(result)
add_parent_href_to_result(result) unless tag_spec[:href]
add_tag_to_result(result, tag_spec)
log_result(result)
result
Expand Down Expand Up @@ -125,7 +124,7 @@ def ci_set_tag(ci, tag_spec)
Classification.classify(ci, tag_spec[:category], tag_spec[:name])
success = ci_is_tagged_with?(ci, tag_spec)
end
action_result(success, desc)
action_result(success, desc, :parent_id => ci.id)
rescue => err
action_result(false, err.to_s)
end
Expand All @@ -139,7 +138,7 @@ def ci_unset_tag(ci, tag_spec)
desc = "Not tagged with #{tag_ident(tag_spec)}"
success = true
end
action_result(success, desc)
action_result(success, desc, :parent_id => ci.id)
rescue => err
action_result(false, err.to_s)
end
Expand Down
10 changes: 10 additions & 0 deletions spec/requests/api/tag_collections_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -543,9 +543,11 @@ def expect_resource_has_tags(resource, tag_names)
expected = {
'results' => [
a_hash_including('success' => true,
'href' => a_string_including(vms_url(vm1.id)),
'tag_category' => tag1[:category],
'tag_name' => tag1[:name]),
a_hash_including('success' => true,
'href' => a_string_including(vms_url(vm2.id)),
'tag_category' => tag2[:category],
'tag_name' => tag2[:name])
]
Expand Down Expand Up @@ -573,9 +575,11 @@ def expect_resource_has_tags(resource, tag_names)
'results' => [
a_hash_including('success' => false, 'message' => a_string_including("Couldn't find Vm")),
a_hash_including('success' => false,
'href' => a_string_including(vms_url(vm2.id)),
'tag_category' => bad_tag[:category],
'tag_name' => bad_tag[:name]),
a_hash_including('success' => true,
'href' => a_string_including(vms_url(vm2.id)),
'tag_category' => tag1[:category],
'tag_name' => tag1[:name])
]
Expand Down Expand Up @@ -607,9 +611,11 @@ def expect_resource_has_tags(resource, tag_names)
expected = {
'results' => [
a_hash_including('success' => true,
'href' => a_string_including(vms_url(vm1.id)),
'tag_category' => tag1[:category],
'tag_name' => tag1[:name]),
a_hash_including('success' => true,
'href' => a_string_including(vms_url(vm2.id)),
'tag_category' => tag2[:category],
'tag_name' => tag2[:name])
]
Expand Down Expand Up @@ -818,9 +824,11 @@ def expect_resource_has_tags(resource, tag_names)
expected = {
'results' => [
a_hash_including('success' => true,
'href' => a_string_including(services_url(service1.id)),
'tag_category' => tag1[:category],
'tag_name' => tag1[:name]),
a_hash_including('success' => true,
'href' => a_string_including(services_url(service2.id)),
'tag_category' => tag2[:category],
'tag_name' => tag2[:name])
]
Expand All @@ -844,9 +852,11 @@ def expect_resource_has_tags(resource, tag_names)
expected = {
'results' => [
a_hash_including('success' => true,
'href' => a_string_including(services_url(service1.id)),
'tag_category' => tag1[:category],
'tag_name' => tag1[:name]),
a_hash_including('success' => true,
'href' => a_string_including(services_url(service2.id)),
'tag_category' => tag2[:category],
'tag_name' => tag2[:name])
]
Expand Down

0 comments on commit f9deb1d

Please sign in to comment.