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

Create a virtual column for archived for using in the API #17509

Merged
merged 1 commit into from
Jun 1, 2018

Conversation

AparnaKarve
Copy link
Contributor

@AparnaKarve AparnaKarve commented Jun 1, 2018

Adding a virtual column for archived? enables us to query it in the API as shown below -

/api/service_templates/<id>?attributes=archived?

/cc @agrare

@@ -411,6 +412,10 @@ def add_resource(rsc, options = {})
set_service_type
end

def archived
archived?
Copy link
Member

Choose a reason for hiding this comment

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

why is this needed? can virtual columns not have a ?

Copy link
Member

Choose a reason for hiding this comment

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

Oh I guess it wouldn't work well with the URI then, still new to me so bear with me.
You should add one for active also then if we want to use that in the future

Copy link
Contributor Author

@AparnaKarve AparnaKarve Jun 1, 2018

Choose a reason for hiding this comment

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

? has a special meaning in the API context - so I was just making sure to keep things simple in the context of the API
Although I have verified that this does work in the API -

virtual_column   :archived?,                     :type => :boolean
/api/service_templates/<id>?attributes=archived?

LMK if you want me to change it.

Copy link
Member

Choose a reason for hiding this comment

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

The thing is these are defined in the archived mixin so we would need to add these to every model that includes the mixin, might be better to either use the methods that the mixin provides or add aliases and virtual-columns to the mixin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, let's go with archived?, since the API can handle it

@AparnaKarve AparnaKarve force-pushed the add_virtual_col_archived branch from 6394758 to b4d64ff Compare June 1, 2018 17:21
@miq-bot
Copy link
Member

miq-bot commented Jun 1, 2018

Checked commit AparnaKarve@b4d64ff with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍰

@agrare agrare merged commit c3b703d into ManageIQ:master Jun 1, 2018
@AparnaKarve AparnaKarve deleted the add_virtual_col_archived branch June 1, 2018 17:53
@AparnaKarve
Copy link
Contributor Author

@agrare Thanks for merging...

And apologies I didn't see this comment #17509 (comment) before -- saw it just now after I refreshed the page.

archived? returns true or false, so false would mean active ?
I think we are good for now, what do you think?

@agrare agrare added this to the Sprint 87 Ending Jun 4, 2018 milestone Jun 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants