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

166840926 article comments #56

Merged
merged 1 commit into from
Aug 19, 2019
Merged

Conversation

olorunwalawrence
Copy link
Contributor

What does this PR do?

create article comment

Description of the task to be completed

  • create article comment functionality
  • create the endpoint for the article

How to manually test this PR

  • clone the repo
  • on your terminal, run 166840926-article-comments
  • create a user model and migration
  • on your terminal, run npm install to install dependencies
  • on your terminal, run npm run start:dev to start the application.

What is the story identity?

166840926

@@ -0,0 +1,51 @@
import express from 'express';
Copy link

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,40 @@
import responseGerator from './responseGenerator';
Copy link

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'

return Comments;
};

export default comments;
Copy link

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,46 @@
module.exports = {
Copy link

Choose a reason for hiding this comment

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

'module' is not defined no-undef

@@ -0,0 +1,140 @@
/* eslint-disable require-jsdoc */
import models from '../database/models';
Copy link

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'

@olorunwalawrence olorunwalawrence force-pushed the 166840926-article-comments branch 7 times, most recently from 022d8ff to 6d09cbd Compare August 5, 2019 08:30
Copy link
Contributor

@tohbay tohbay left a comment

Choose a reason for hiding this comment

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

Good job @olorunwalawrence 👍

Copy link
Contributor

@timi-codes timi-codes left a comment

Choose a reason for hiding this comment

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

Good Job! @lawarence, I left you a comment. Kindly review and let's get the merged as soon as possible

});
});

describe('PUT api/v1/comments/:commentId', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hello Good @olorunwalawrence. However, None of your tests here catered for a successful comment update. Kindly revisit this.

Copy link
Contributor

Choose a reason for hiding this comment

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

No success case in the tests. All the tests account for only for failure cases

Copy link
Contributor

@segunolalive segunolalive left a comment

Choose a reason for hiding this comment

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

make the suggested changes including the grammatical errors in test descriptions.

This is good to go afterward.

expect((res.body.data.rows[0] = 1));
});

it('should not list when comment token is invalid', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This behaviour is not right. Users should be able to see comments whether or not they're logged in.

expect(res.status).to.equal(401);
expect(res.body.message).to.be.equal('Token is not valid');
});
it('User should not be able to comment if not authenticated', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

should not be able to update a comment if not authenticated.

});
});

describe('PUT api/v1/comments/:commentId', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

No success case in the tests. All the tests account for only for failure cases

@olorunwalawrence olorunwalawrence force-pushed the 166840926-article-comments branch 3 times, most recently from 9dd58cc to 3eee5c3 Compare August 6, 2019 22:04
@olorunwalawrence olorunwalawrence force-pushed the 166840926-article-comments branch 3 times, most recently from bdb31e0 to dbd9c0f Compare August 15, 2019 11:28
- create article comment functionality
- create artilce comment  test and validations

[Finishes #166840926]
@olorunwalawrence olorunwalawrence merged commit 544b7b2 into develop Aug 19, 2019
codexempire pushed a commit that referenced this pull request Aug 21, 2019
timi-codes pushed a commit that referenced this pull request Aug 27, 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

4 participants