[api] expose Request Workflow class name #13441
Conversation
|
@imtayadeway I've consolidated all attribute related specs under one spec in ec1872b9fc839ba2c2a2490e3569dab55fe7348d. |
|
@miq-bot add_label wip |
AparnaKarve
changed the title
AparnaKarve
force-pushed
the
api_expose_wf_class
branch
2 times, most recently
from
6490fb4 to
6b21ab1
Compare
| expect(response.parsed_body).to match(expected) | ||
| expect(response.parsed_body).to match(expected_workflow) | ||
| expect(response.parsed_body).to match(expected_tags) | ||
| expect(response.parsed_body).to match(expected_class) |
There was a problem hiding this comment.
Might be better to have a single expect() for all the above since we're retesting id anyways, might something like:
expected_response = a_hash_including(....)
expect(response.parsed_body).to match(expected_response)
| virtual_column :request_type_display, :type => :string | ||
| virtual_column :resource_type, :type => :string | ||
| virtual_column :state, :type => :string | ||
|
|
||
| delegate :allowed_tags, :to => :workflow, :prefix => :v | ||
| delegate :class, :to => :workflow, :prefix => :v |
There was a problem hiding this comment.
feels odd, too close to class, I know it has the v_ prefix, but I wonder if this should be named differently, maybe workflow_class ?
AparnaKarve
force-pushed
the
api_expose_wf_class
branch
from
6b21ab1 to
f9f3f43
Compare
AparnaKarve
force-pushed
the
api_expose_wf_class
branch
from
f9f3f43 to
5c94cfa
Compare
| virtual_column :request_type_display, :type => :string | ||
| virtual_column :resource_type, :type => :string | ||
| virtual_column :state, :type => :string | ||
|
|
||
| delegate :allowed_tags, :to => :workflow, :prefix => :v | ||
| delegate :class, :to => :workflow, :prefix => :v |
There was a problem hiding this comment.
This could lead to some surprising behavior because it's overriding MiqRequest's built-in class method - would klass be OK for this?
There was a problem hiding this comment.
I've changed the virtual_column name to v_workflow_class Does that work?
|
|
||
| expected = a_hash_including("id" => request.id, "v_allowed_tags" => [a_hash_including("children")]) | ||
| expected_workflow = a_hash_including("id" => request.id, "workflow" => a_hash_including("values")) |
There was a problem hiding this comment.
This doesn't seem quite right, as you're checking the same attribute "id" in each expectation. Perhaps something like:
expected = {
"id" => request.id,
"workflow" => a_hash_including("values"),
"v_allowed_tags" => [a_hash_including("children")],
"v_class" => {"etc" => "etc"}
}
expected(response.parsed_body).to include(expected)There was a problem hiding this comment.
Yes, I took care of that. Thanks.
AparnaKarve
force-pushed
the
api_expose_wf_class
branch
from
5c94cfa to
a9f1c84
Compare
There was a problem hiding this comment.
@AparnaKarve changes look good, if you can just fix the rubocop issue I think this is GTG 🍰
AparnaKarve
force-pushed
the
api_expose_wf_class
branch
from
a9f1c84 to
af354f5
Compare
|
Checked commits AparnaKarve/manageiq@b6d57bd~...af354f5 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 |
AparnaKarve
changed the title
|
@miq-bot remove_label wip @imtayadeway Thanks! |
|
👍 Thanks Aparna. |
Created a virtual_column for the workflow object's class name which is required in order to populate the
Catalogtab in the Request Workflow in SUI.api/requests/<id>?attributes=v_workflow_classhttps://www.pivotaltracker.com/n/projects/1914499/stories/137409675