Skip to content

Conversation

@abulojoshua1
Copy link

What does this PR do?

  • Enables a user to create an article on Authors Haven

Description of Task to be completed?

  • With these new changes, a user is able to create an article on authors haven.

How should this be manually tested?

  • make a post request to the route /api/articles/
  • The post request made should be in the form { "article": { "title": "THE ARTICLE TITLE", "body": "THE ARTICLE BODY", "description": "THE ARTICLE DESCRIPTION" } }

Any background context you want to provide?

  • I created a new django app named articles.

What are the relevant pivotal tracker stories?

@abulojoshua1 abulojoshua1 force-pushed the ft-create-user-articles-159082379 branch 2 times, most recently from 474b7a9 to 22c8987 Compare August 7, 2018 18:09
@abulojoshua1 abulojoshua1 requested review from geofrocker, lytes20 and ssewilliam and removed request for lytes20 August 7, 2018 18:13
@coveralls
Copy link

Coverage Status

Coverage increased (+0.9%) to 93.506% when pulling 22c8987 on ft-create-user-articles-159082379 into 8ca30af on develop.

Copy link
Contributor

@ssewilliam ssewilliam left a comment

Choose a reason for hiding this comment

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

@abulojoshua1

  • This looks good, it just needs to clean up
  • Also you can provide more clarity on what you did

pass
# As mentioned about, we will let the default JSONRenderer handle
# rendering errors.
# return super(UserJSONRenderer, self).render(data)
Copy link
Contributor

Choose a reason for hiding this comment

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

@abulojoshua1

  • Is this Commented code
  • I think you should return the line
    return super(ArticlesJSONRenderer, self).render(data)
    I think what that does is return the actual error if the class's render() itself has an error

Copy link
Author

@abulojoshua1 abulojoshua1 Aug 8, 2018

Choose a reason for hiding this comment

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

Thanks for Identifying that @ssewilliam, Sharp eye there! I will work on it asap.

updatedAt = models.DateTimeField(auto_now=True)

# This makes identifies the owner of the publication
user = models.ForeignKey(User, on_delete=models.CASCADE)
Copy link
Contributor

Choose a reason for hiding this comment

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

@abulojoshua1

  • You could have also called this user as user_id

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I will work on that as soon as possible.

self.article_to_create = {
"article": {
"title": "hahahhaha hahaha hahah ahhah",
"body": "<p>this is a body that is here hahahhahaha\n <p",
Copy link
Contributor

Choose a reason for hiding this comment

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

@abulojoshua1
Can you clarify on why you are passing in raw HTML here

Copy link
Contributor

@lytes20 lytes20 Aug 8, 2018

Choose a reason for hiding this comment

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

i think this is some data to be used in the test and it should be okay to write some raw html @ssewilliam , maybe @abulojoshua1 could also say something

Copy link
Author

Choose a reason for hiding this comment

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

I would like to mimic the presence on an article that has HTML attributes like Bolding, paragraphs, etc in an article body. But as for now, that for tests.

@abulojoshua1 abulojoshua1 force-pushed the ft-create-user-articles-159082379 branch from 22c8987 to 076f9ce Compare August 8, 2018 09:12
published = models.BooleanField(default=False)

# this takes the time stamp of when the article was made
createdAt = models.DateTimeField(auto_now_add=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

use underscores for variable naming

Copy link
Author

Choose a reason for hiding this comment

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

Ok, @geofrocker. I will do that ASAP.

@abulojoshua1 abulojoshua1 force-pushed the ft-create-user-articles-159082379 branch 2 times, most recently from 67e45c4 to 864a4ce Compare August 8, 2018 09:51
@abulojoshua1 abulojoshua1 force-pushed the ft-create-user-articles-159082379 branch from 864a4ce to 31c48fa Compare August 8, 2018 10:52
@geofrocker geofrocker merged commit 52fdee4 into develop Aug 8, 2018
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.

6 participants