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

Add Classification to asset API #538

Merged
merged 2 commits into from
May 18, 2017
Merged

Add Classification to asset API #538

merged 2 commits into from
May 18, 2017

Conversation

michaeljs1990
Copy link
Contributor

@michaeljs1990 michaeljs1990 commented Apr 18, 2017

This adds support for finding what classification a nodeclass belongs to using the API.
The tests are a little bit... a lot a bit messy however they cover the two cases well I
believe.

Closes #482
Docs #541

This adds support for finding what classification a nodeclass belongs to using the API.
The tests are a little bit... a lot a bit messy however they cover the two cases well I
believe.
Didn't break the test but changed one back to it's original style since
this was not the goal of this PR.
"create a configuration asset and check that an asset matches" in new WithApplication(FakeApplication(
additionalConfiguration = Map(
"solr.enabled" -> false))) with AssetApiHelper {
override val assetTag = "node_classy"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only have nodes with the utmost class.

Copy link
Contributor

Choose a reason for hiding this comment

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

"classification":"bourgeoise"

Copy link
Contributor

@byxorna byxorna left a comment

Choose a reason for hiding this comment

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

Wow, this looks awesome to me! Nice work!

@michaeljs1990
Copy link
Contributor Author

Just a bump on this 😆 we merged the docs but not the code.... that has to be a first for me.

@byxorna byxorna merged commit 4691ca5 into tumblr:master May 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants