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

#165412918 Give a rating to an article #50

Merged
merged 2 commits into from
Jun 11, 2019

Conversation

kagaramag
Copy link
Collaborator

@kagaramag kagaramag commented May 22, 2019

What does this PR do?

Give a rating on the article written by others

Description of Task to be completed?

Have the following endpoint accessible to an authenticated user:

POST /api/v1/articles/:slug/rating give rating to an article identified by its slug
GET /api/v1/articles/:slug/rating Get users who rated a given article

How should this be manually tested?

  • After cloning the repo, cd into it and type npm test
  • Using Postman,
    ->make sure you signup then login to get the access token,
    ->create an article,
    -> Create a rating using this endoint: /api/v1/articles/:slug/rating
    -> Make sure you pass the access token in the header with the following key name: access-token,
    -> Finally, send a JSON object of the following structure : {"rating": 1}

Any background context you want to provide?

  • The formula used to calculate a rating of a given article
    Given,
    n: number of rating per article
    s: sum of ratings per a given article
rating: s/n

What are the relevant pivotal tracker stories?

#165412918

Screenshots (if appropriate)

N/A

Questions:

N/A

@kagaramag kagaramag force-pushed the feature/rate-article-165412918 branch 7 times, most recently from 9570bb4 to 98d8f4e Compare May 23, 2019 14:37
@kagaramag kagaramag requested review from rwajon and e-liyai May 23, 2019 14:48
@kagaramag kagaramag force-pushed the feature/rate-article-165412918 branch 6 times, most recently from 91e3e52 to 4069ffc Compare May 30, 2019 11:25
package.json Outdated
@@ -32,6 +32,7 @@
"chance": "^1.0.18",
"cheke": "^1.0.3",
"cors": "^2.8.5",
"d": "^1.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

what's this meant for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The package is no longer needed, I am removing it

src/middlewares/validation/validateRating.js Outdated Show resolved Hide resolved
export default async (data = {}) => {
const { articleId, userId, rating } = data;
// check if already exist
const result = await db.Rating.findAll({
Copy link
Contributor

Choose a reason for hiding this comment

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

there should be a file for findAll Ratings then import it here

@kagaramag kagaramag force-pushed the feature/rate-article-165412918 branch 3 times, most recently from 61b6baf to 2c39da1 Compare May 31, 2019 10:15
@kagaramag kagaramag requested a review from e-liyai May 31, 2019 11:02
import db from '../../models';

export default async (articleId) => {
const allRatings = await db.Rating.findAll({
Copy link
Contributor

Choose a reason for hiding this comment

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

import this as well

@@ -0,0 +1,63 @@
import * as queries from '../queries';
Copy link
Contributor

Choose a reason for hiding this comment

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

there should not be queries in a model file. Set static data

@kagaramag kagaramag force-pushed the feature/rate-article-165412918 branch 3 times, most recently from b671fb2 to 4637946 Compare June 4, 2019 16:41
@kagaramag
Copy link
Collaborator Author

@e-liyai made changes, you can check.

@kagaramag kagaramag force-pushed the feature/rate-article-165412918 branch 2 times, most recently from 87617c1 to 15cadae Compare June 6, 2019 20:45
@kagaramag
Copy link
Collaborator Author

@e-liyai check this PR is ready.

@kagaramag kagaramag force-pushed the feature/rate-article-165412918 branch from 15cadae to 6069e02 Compare June 9, 2019 23:08
@kagaramag kagaramag force-pushed the feature/rate-article-165412918 branch from 247fe74 to 6350219 Compare June 10, 2019 08:21
@kagaramag
Copy link
Collaborator Author

@e-liyai can you check this PR for me. it's ready 👍

const response = {};
if (result && result.length === 0) {
await db.Rating.create({ rating, articleId, userId }, { logging: false });
response.message = 'Thank you for rating this article';
Copy link
Contributor

Choose a reason for hiding this comment

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

remove message in queries, it should be in controller

{ where: { [db.Op.and]: { articleId, userId } }, logging: false }
);
response.statusCode = status.OK;
response.message = 'Your article rating has been updated';
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@kagaramag kagaramag force-pushed the feature/rate-article-165412918 branch 2 times, most recently from b74ebf6 to d5fe9e7 Compare June 10, 2019 13:35
rating: {
message:
response.statusCode === status.OK
? 'Your article rating has been updated'
Copy link
Contributor

Choose a reason for hiding this comment

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

the logic here seems flawed if the status is ok article update successful else even in the case of an error the message displayed is Thank you for rating this article

// check if already exist
const result = await findAll(articleId, userId);
const response = {};
if (result && result.length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

i see a create or update scenario here, update the filename appropriately

@kagaramag kagaramag force-pushed the feature/rate-article-165412918 branch 2 times, most recently from be36ac4 to 48de0c6 Compare June 11, 2019 09:25
@kagaramag kagaramag force-pushed the feature/rate-article-165412918 branch from 48de0c6 to 96a5daf Compare June 11, 2019 09:51
@e-liyai e-liyai merged commit 0d92d62 into develop Jun 11, 2019
@e-liyai e-liyai deleted the feature/rate-article-165412918 branch June 11, 2019 11:27
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

4 participants