Milestone 3 - users & posts

[samgaco]

Things I have worked on in this milestone:

• Created models with associations and implement all requested features for users and posts.
• Added authentication with Devise as described in requirements.
• Added unit and integrations tests
• Did some CSS styling (still a lot left)

---

Rubocop doesn't get activated in the pull request:

I am running the linters locally because of the reasons mentioned in my last comment in the previous milestone ( #4 ) there are two offenses which I cannot solve.

Offenses:

spec/models/post_spec.rb:41:24: E: Lint/Syntax: unexpected token tRCURLY
(Using Ruby 2.6 parser; configure using TargetRubyVersion parameter, under AllCops)
expect({post.save}).to change{ post.errors.full_messages.count }
^
spec/models/user_spec.rb:63:70: E: Lint/Syntax: unexpected token tRCURLY
(Using Ruby 2.6 parser; configure using TargetRubyVersion parameter, under AllCops)
expect({@user.posts.create(content: 'bla' * 100, title: 'When')}).to change { Post.count }

I have been googling, and they might be a bug.

---

Working alone:

I am working on this project alone, allowed by student success.

---

SECOND REVIEW:

I have worked on the requested changes and I am submitting for another review.

• Run rails g devise:controllers users
• Added before_action :authenticate_user! in every controller (before non-signed in users could see pages they shouldn't be able to see, like the users' index, post index, and they could create posts)
• Some unresolved linter checks are bugs

[Oghenebrume50]

Hi @samgaco good implementation and good UI too, but we need to make more things work, it seems you are not using devise properly, for example, I cannot see the auto-generated folder by devise in the controllers' folder.
Did you run rails g devise:controllers users that?

Also currently I can access anypage on the site without signing, this is not the right behaviour. If no user is signed in, changing the url should redirect back to the signin page.
currently there are no sessions, no current_user. So this functionality cannot be provided, but devise provides them out of the box for you. Please see the devise guide on how to use these provided functionalities.

Make changes and request a re-review 🙏

[samgaco]

I have worked on the requested changes and I am submitting for another review.

• Run rails g devise:controllers users
• Added before_action :authenticate_user! in every controller (before non-signed in users could see pages they shouldn't be able to see, like the users' index, post index, and they could create posts)

---

• Some unresolved linter checks are bugs

[adriaanbd]

I really like how this project is coming along. Really nice!

Just a few things to consider before moving on (required):

• PostsController#index displays posts in a timeline

Other optional things to consider (see in comments below):

• update routes
• update FactoryBot
• more tests

In regards to testing, I'd strongly suggest covering all CRUD operations at the integration testing level (controller); therefore, your posts_controller_spec.rb should contain more tests. Do the same later on for the next milestones. Take advantage of this opportunity to get to know Rails, Rspec and Capybara while doing this project. It's a good skill and toolbox to have. Also, you'll be more clear and less error prone as your project advances and is more complex. The extra time will pay off.

Status: Request changes

[adriaanbd]

Posts need to be displayed in a timeline manner, so @posts should be sorted / ordered by date and time, preferably most recently created post first.

[REQUIRED]

[samgaco]

Hi @adriaanbd, thanks for the review

I have currently this scope in my post model (app/ models/post.rb)

default_scope { order(created_at: :desc) }

Wouldn't that be working as such?

[adriaanbd]

Oh @samgaco yes you are right, somehow I missed this. Since no dates/time is displayed on the UI I wasn't able to confirm it. You should have gotten a pass, let me tell Alvaro to correct this. Sorry about this!

[samgaco]

No problem. I will work on those test moving on, thanks a lot for your review 👍 . It was very helpful, I definitely need to improve (and widen) my testing skills.

[adriaanbd]

How about adding some integration tests in here for all CRUD operations? The tests defined in here are only Read operations on :new and :index and :show. It'd be nice to include tests for Create, Update and Delete. When doing update and delete, test that only the post creator can make an edit or delete his own post and also test the opposite (that you can't edit or delete a post you haven't created).

Doing these tests now will help you out a lot while developing your app and give you less headaches later on.

[OPTIONAL]

[samgaco]

The project was approved in one of the comments in the code:

Oh @samgaco yes you are right, somehow I missed this. Since no dates/time is displayed on the UI I wasn't able to confirm it. You should have gotten a pass, let me tell Alvaro to correct this. Sorry about this!

The option for submitting the project completion form is still not showing. That is why I am re-submitting.