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.

Sign up for GitHub

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

Jump to bottom

#167891593 Implement Search Functionality #53

Merged
merged 1 commit into from
Sep 11, 2019
Merged

#167891593 Implement Search Functionality #53

merged 1 commit into from
Sep 11, 2019

Conversation

max-wel
Copy link
Collaborator

@max-wel max-wel commented Sep 8, 2019 •
edited
Loading

What does this PR do?

Adds search functionality

Description of Task to be completed?

  • Create search controller
  • Create pagination helper class
  • Add validations for query parameters

How should this be manually tested?

  • Pull this branch ft-search-functionality-167891593
  • Run npm run migrate
  • Run npm run devstart command in the terminal
  • On postman, make a GET request to localhost:3000/api/v1/search?keyword=pending

Any background context you want to provide?

N/A

What are the relevant pivotal tracker stories?

#167891593

Screenshots (if appropriate)

Screenshot (149)

@max-wel max-wel added the wip label Sep 8, 2019
@max-wel max-wel force-pushed the ft-search-functionality-167891593 branch 2 times, most recently from f6337f0 to 5a8bf6e Compare September 10, 2019 07:19
@max-wel max-wel force-pushed the ft-search-functionality-167891593 branch from 5a8bf6e to 64d0b63 Compare September 10, 2019 09:28
daniellamarr
daniellamarr suggested changes Sep 10, 2019
View reviewed changes
Copy link
Contributor

@daniellamarr daniellamarr left a comment

Choose a reason for hiding this comment

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

Nice work on this PR, I feel you can make some changes to your implementation in order to give room for efficiency in your application. You can use your switch statement to determine what search query you need to make.

@max-wel max-wel force-pushed the ft-search-functionality-167891593 branch 2 times, most recently from db55649 to f9ab0a1 Compare September 10, 2019 15:05
@max-wel
Copy link
Collaborator Author

max-wel commented Sep 10, 2019

Nice work on this PR, I feel you can make some changes to your implementation in order to give room for efficiency in your application. You can use your switch statement to determine what search query you need to make.

Thanks, I've implemented the feedback

@max-wel max-wel force-pushed the ft-search-functionality-167891593 branch 2 times, most recently from f636de2 to a0a74a4 Compare September 11, 2019 08:58
@max-wel
feature(search): implement search functionality
67683f2 
- write tests
- add pagination helper class
- add route and controller logic

[Starts #167891593]
@max-wel max-wel force-pushed the ft-search-functionality-167891593 branch from a0a74a4 to 67683f2 Compare September 11, 2019 09:11
daniellamarr
daniellamarr approved these changes Sep 11, 2019
View reviewed changes
Copy link
Contributor

@daniellamarr daniellamarr left a comment

Choose a reason for hiding this comment

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

Great job on your implementation!🎊🎊🎊

@daniellamarr daniellamarr merged commit 4b41a8d into develop Sep 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daniellamarr daniellamarr daniellamarr approved these changes

@tvpeter tvpeter Awaiting requested review from tvpeter

@femitj femitj Awaiting requested review from femitj

@meetKazuki meetKazuki Awaiting requested review from meetKazuki

@Pomile Pomile Awaiting requested review from Pomile

Assignees
No one assigned
Labels
ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@max-wel @daniellamarr