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

Elastic search assets and users 165965388 #316

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

raheemazeezabiodun
Copy link
Collaborator

What does this PR do?

Allow full text search across different fields from the viewset

Description of Task to be completed?

How should this be manually tested?

  1. Create assets
  2. search assets with this endpoint http://127.0.0.1:8000/api/v1/assets?search=whatever_you_want_to_search

Any background context you want to provide?

What are the relevant pivotal tracker stories?

https://www.pivotaltracker.com/n/projects/2146417/stories/165965388

Postman Documentation

Screenshots (If applicable)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.003%) to 91.006% when pulling d2b419c on elastic-search-assets-and-users-165965388 into cd5bbca on develop.

@@ -387,3 +387,63 @@ def test_asset_allocation_history_has_assigner(self, mock_verify_id_token):
)
self.assertIn("assigner", response.data["allocation_history"][0].keys())
self.assertEqual(response.status_code, 200)

@patch("api.authentication.auth.verify_id_token")
def test_asset_search_endpoint(self, mock_verify_id_token):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Asset search is more of a functionality than an endpoint. However, there's an asset endpoint. A better description could be, "test_search_funx_on_asset_endpoint".

response.data.get('results')[0].get('uuid'), str(asset_two.uuid)
)

response = client.get(
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be more appropriate if the tests are independent to show different functionalities rather than grouping them together. Different tests cases could should show different fields the search query string reaches.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The test objects created could be done in a helper function/method and called when need be.

@kmazi kmazi force-pushed the elastic-search-assets-and-users-165965388 branch from d2b419c to 52bc506 Compare August 28, 2019 11:33
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.

3 participants