Add Author to Article Show Page #19

AKidd95 · 2018-03-18T19:56:57Z

PT Story: https://www.pivotaltracker.com/story/show/155970828

Description

Changes proposed in this pull request:

Add author field to new article
display author article show page
add associations between author and article
add database table for authors

What I have learned working on this feature:

How to add references to an existing table, and their benefits
how to add associations and fix the errors they create

add author column to article db display author on article show page validate and sanitze author params

add validation for author add tests for associations

add author ref to article fix author tests

rspec all green

tochman

You have to rethink the entire approach to this

tochman · 2018-03-18T19:59:27Z

app/controllers/articles_controller.rb

@@ -8,7 +8,7 @@ def create
    @article = Article.new(article_params)
    if @article.save
      flash[:success] = "Article was successfully created."
-      redirect_to @article
+      redirect_to article_path(@article)


No need to make this change

tochman · 2018-03-18T20:00:07Z

app/models/author.rb

+  validates :author, presence: true
+
+  has_many :articles
+end


No need to create an Author class. User can do

tochman · 2018-03-18T20:01:24Z

app/views/articles/new.html.haml

@@ -3,6 +3,10 @@
    = form.label :title
    %br/
    = form.text_field :title, id: :article_title
+  %p
+    = form.label :author
+    %br/


No need to add this field. Whoever’s logged in should be assigned as the author of the article. You can deal with it in the controller

i thought that the requirements for the author part of the assignment was to be able to assign multiple authors to a piece. Would using an author form field be a bad way approaching this?

tochman · 2018-03-18T20:02:09Z

app/views/articles/show.html.haml

@@ -1,6 +1,9 @@
 %p
  %strong Title:
  = @article.title
+%p
+  %strong Authors:
+  [email protected]


If it is authors then use plural form

tochman · 2018-03-18T20:03:11Z

db/migrate/20180318173006_add_author_ref_to_article.rb

@@ -0,0 +1,5 @@
+class AddAuthorRefToArticle < ActiveRecord::Migration[5.2]
+  def change
+    add_reference :articles, :author, foreign_key: true


You probably want to have a has_many relationship between Article and User

tochman · 2018-03-18T20:03:33Z

spec/models/article_spec.rb

    end
  end

  describe 'Validations' do
    it { is_expected.to validate_presence_of :title }
    it { is_expected.to validate_presence_of :content }
  end
+
+  describe 'Associations' do
+    it { should belong_to :author }


change unnecessary code

make correct assocations between user and articles

add user logged in to create article tests

all tests green

take out show page from tests

add optional user assocation with article

diraulo

... Try to be a bit descriptive with your PR titles. When you say Author? what about it? 🤔

diraulo · 2018-03-20T09:58:47Z

spec/models/article_spec.rb

    end
  end

  describe 'Validations' do
    it { is_expected.to validate_presence_of :title }
    it { is_expected.to validate_presence_of :content }
  end
+
+  describe 'Associations' do
+    it { should belong_to :user }


Prefer expect syntax over should

diraulo · 2018-03-20T09:58:53Z

spec/models/user_spec.rb

@@ -11,4 +11,8 @@
      expect(create(:user)).to be_valid
    end
  end
+
+  describe 'Associations' do
+    it { should have_many :articles }


Prefer expect syntax over should

edit rspec syntax to is expected add author feature test

holgertidemand · 2018-03-22T09:50:21Z

spec/models/article_spec.rb

@@ -8,12 +8,16 @@

  describe 'Factory' do
    it 'should have valid Factory' do
-      expect(create(:article)).to be_valid
+      expect(create(:user)).to be_valid


This should be article. You are testing the article factory :)
If you are getting an error maybe you forgot to add user to the factory?

Looked at the article factory and it's missing the user. :) See comment above.

holgertidemand · 2018-03-22T09:52:13Z

spec/models/article_spec.rb

@@ -8,12 +8,16 @@

  describe 'Factory' do
    it 'should have valid Factory' do
-      expect(create(:article)).to be_valid
+      expect(create(:user)).to be_valid


Looked at the article factory and it's missing the user. :) See comment above.

AKidd95 added 6 commits March 18, 2018 16:27

add author field to new article

823c6d8

add author column to article db display author on article show page validate and sanitze author params

add author and article associations

899d977

add validation for author add tests for associations

create author table

4115a2d

add author ref to article fix author tests

create author table

4242955

add author ref to article fix author tests

fix factory bot tests

e9bdf75

rspec all green

edit article path in controller

43c3821

tochman requested changes Mar 18, 2018

View reviewed changes

AKidd95 added 8 commits March 19, 2018 10:23

rollback author db changes

b4f7f63

change unnecessary code

delete author class

adf0782

generate migration for user ref to articles

88cf87c

make correct assocations between user and articles

delete author field

77ec4b5

add user logged in to create article tests

set articles creator to current user

c3d613e

all tests green

set author name to user email address

dd938b9

take out show page from tests

fix merge conflict

ae54e9f

fix merge conflicts

1df15f3

add optional user assocation with article

diraulo reviewed Mar 20, 2018

View reviewed changes

AKidd95 changed the title ~~[WIP] Author~~ [WIP] Add Author to Article Show Page Mar 21, 2018

fix merge conflict

d44b104

edit rspec syntax to is expected add author feature test

holgertidemand reviewed Mar 22, 2018

View reviewed changes

holgertidemand suggested changes Mar 22, 2018

View reviewed changes

AKidd95 added 4 commits March 22, 2018 11:34

edit factory create

9dc83cf

fix author display

8943240

delete chrome driver binary

92d36d6

remove comma

9d66c96

AKidd95 changed the title ~~[WIP] Add Author to Article Show Page~~ Add Author to Article Show Page Mar 22, 2018

resolve merge conflicts

7ac0835

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Author to Article Show Page #19

Add Author to Article Show Page #19

AKidd95 commented Mar 18, 2018 •

edited

Loading

tochman left a comment

tochman Mar 18, 2018

tochman Mar 18, 2018

tochman Mar 18, 2018

AKidd95 Mar 19, 2018

tochman Mar 18, 2018

tochman Mar 18, 2018

tochman Mar 18, 2018

diraulo left a comment •

edited

Loading

diraulo Mar 20, 2018

diraulo Mar 20, 2018

holgertidemand Mar 22, 2018 •

edited

Loading

holgertidemand Mar 22, 2018 •

edited

Loading

holgertidemand Mar 22, 2018 •

edited

Loading

Add Author to Article Show Page #19

Are you sure you want to change the base?

Add Author to Article Show Page #19

Conversation

AKidd95 commented Mar 18, 2018 • edited Loading

PT Story: https://www.pivotaltracker.com/story/show/155970828

Description

What I have learned working on this feature:

tochman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diraulo left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holgertidemand Mar 22, 2018 • edited Loading

Choose a reason for hiding this comment

holgertidemand Mar 22, 2018 • edited Loading

Choose a reason for hiding this comment

holgertidemand Mar 22, 2018 • edited Loading

Choose a reason for hiding this comment

AKidd95 commented Mar 18, 2018 •

edited

Loading

diraulo left a comment •

edited

Loading

holgertidemand Mar 22, 2018 •

edited

Loading

holgertidemand Mar 22, 2018 •

edited

Loading

holgertidemand Mar 22, 2018 •

edited

Loading