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

[FEATURE] Adding comments to a post in prep for api release #1

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
1 participant
@getaclue
Copy link
Owner

getaclue commented Oct 20, 2017

  • want to fetch all of the comments prior to rendering the show action of the
    posts controller
  • want to display html unchanged
  • want to show json with all of the data, neatly in one call as two
    entities: 1) post data, 2) all post comments data in an array
  • tested it locally and everything works as expected.
[FEATURE] Adding comments to a post in prep for api release
- want to fetch all of the  comments prior to rendering the show action of the
  posts controller
- want to display html unchanged
- want to show json with all of the data, neatly in one call as two
  entities: 1) post data, 2) all post comments data in an array
- tested it locally and everything works as expected.

@getaclue getaclue self-assigned this Oct 20, 2017

@getaclue
Copy link
Owner Author

getaclue left a comment

I noticed an area of code that could use some improvements. I think you should surround that piece of code in a try-catch clause. If you need more information, then please feel free to reach out to me and I will suggest catches for common errors in this area. Thanks, Alex

@@ -10,6 +10,12 @@ def index
# GET /posts/1
# GET /posts/1.json
def show

This comment has been minimized.

@getaclue

getaclue Oct 20, 2017

Author Owner

Hey,
Are you trying to display all of the comments relevant to a post?
I see a potential area of improvement here.
What happens if the user visits a post that is not found in the database?
As it appears currently, the code could fail through.
I think it would be beneficial to wrap this block into a try-catch clause.
This area is usually prone to ActiveRecordNotFound errors.
You can do the following:

begin
  <code block>
rescue Exception => e
  <code to rescue>
end

If this is not clear, I would be more than happy to point you to the most common errors that occur here.
Cheers,
Alex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment