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

#162727478 Add search functionality #40

Merged
merged 9 commits into from
Feb 24, 2019

Conversation

daniellamarr
Copy link
Contributor

What does this PR do?

  • This PR integrates search functionality for the app

Description of Task to be completed?

  • A user should be able to search for articles based on categories
  • A user should be able to search for articles based on keywords passed
  • A user can search for other users

How should this be manually tested?

  • Run npm install, then npm start
  • Access the route /api/v1/search/categories/:categoryId to search arts based on the category id passed.
  • Access the route /api/v1/search/:keyword to search arts based on the keyword passed.
  • Access the route /api/v1/search/users/:user to search for available users on the platform.

Any background context you want to provide?

  • N/A

What are the relevant pivotal tracker stories?

#162727478

Screenshots (if appropriate)

screen shot 2019-01-31 at 5 33 03 pm

screen shot 2019-01-31 at 5 33 13 pm

screen shot 2019-01-31 at 5 33 19 pm

Questions:

N/A

A user can search for an art by its category
A user can search for an art by keyword
A user can search for other users by their firstname and lastname
[Finishes #162727478]
Fix merge conflict
[Fixes #162727478]
@@ -0,0 +1,69 @@
import chai from 'chai';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,22 @@
import express from 'express';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,185 @@
import { Op } from 'sequelize';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

Copy link
Contributor

@abejide001 abejide001 left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link
Contributor

@Proception Proception left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Contributor

@wombolo wombolo left a comment

Choose a reason for hiding this comment

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

Looks Good

* @memberof SearchController
* @returns {object} arts matching the specified category
*/
static async searchByCategory(req, res) {

Choose a reason for hiding this comment

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

This is searching by the category id. Searching is mostly done by name or title

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, so the idea is on the frontend the users get to see the category URL but on the backend, the id is being queried.

Choose a reason for hiding this comment

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

Then the function should be named differently. Something like getArtsByCategory.

@andela-dbamidele
Copy link

Please look at the comment and fix the merge conflict @daniellamarr

@andela-dbamidele andela-dbamidele merged commit 2684200 into staging Feb 24, 2019
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.

None yet

6 participants