[api] expose allowed tags for a request workflow

[AparnaKarve]

Created a virtual_column for allowed_tags which is required in order to populate the tags tree in the Request Workflow in SUI

api/requests/<id>?attributes=v_allowed_tags

https://www.pivotaltracker.com/n/projects/1914499/stories/137139937

[AparnaKarve]

@imtayadeway Can you please review this when you get a chance? Thanks!

[AparnaKarve]

/cc @chriskacerguis

[abellotti]

In general I'm ok with this, short of the rubocop warning above.

Just curious, could you get that information today without this PR via:

  GET /api/requests/:id?attributes=workflow.allowed_tags

Thanks.

[AparnaKarve]

GET /api/requests/:id?attributes=workflow.allowed_tags

@abellotti Interesting! However, that did not work. I got this --

{
  "error": {
    "kind": "internal_server_error",
    "message": "undefined method `has_attribute?' for ManageIQ::Providers::Vmware::InfraManager::ProvisionWorkflow:Class\nDid you mean?  class_attribute",
    "klass": "NoMethodError"
  }
}

[AparnaKarve]

@abellotti As far as the rubocop warning goes, I think we could ignore it.
I have deliberately added the spacing for readability like all the other virtual_columns on top that use :uses

 virtual_column  :reason,               :type => :string,   :uses => :miq_approvals
 virtual_column  :v_approved_by,        :type => :string,   :uses => :miq_approvals
 virtual_column  :v_approved_by_email,  :type => :string,   :uses => {:miq_approvals => :stamper}
 virtual_column  :stamped_on,           :type => :datetime, :uses => :miq_approvals

Maybe I could move this newly added virtual column on top with the others that have :uses

[abellotti]

@AparnaKarve thanks for checking.

This LGTM!!

@imtayadeway thoughts/input ?

[imtayadeway]

I think you could do this in one line:

delegate :allowed_tags, :to => :workflow, :prefix => :v

[AparnaKarve]

Yeah, that would work 👍 . Thanks.

[kbrock]

yay!

[imtayadeway]

I'm a bit divided over this test, although it is written perfectly well! I'm just thinking that since we should have ample coverage that the API can deliver virtual attributes, is it needed? Perhaps a model test would suffice?

[AparnaKarve]

So what you're saying is, instead of writing a test on the API side we should write it only on the model side? ...which kinda makes sense in this situation.
Although the only reason I added a virtual_column was because I have this requirement to access it via the API. Without this requirement in the picture, the model does not need any changes.

How about we include both tests?

[imtayadeway]

I'm leaning towards only making the model test. @abellotti any thoughts here?

[abellotti]

Both good, though my preference is on the API side since that is the intended usage and it tests both model and the API exposing it.

[AparnaKarve]

@abellotti @imtayadeway I have added tests in both places -- model and API.

If having both specs seems redundant, I can get rid of this commit d041b96, which is the model spec.

[miq-bot]

Checked commits AparnaKarve/manageiq@21d9103~...d041b96 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 1 offense detected

app/models/miq_request.rb

• ❗ - Line 41, Col 59 - Style/ExtraSpacing - Unnecessary spacing detected.

[abellotti]

👍