Skip to content

Danebook#35

Open
lynchd2 wants to merge 31 commits into
vikingeducation:masterfrom
lynchd2:master
Open

Danebook#35
lynchd2 wants to merge 31 commits into
vikingeducation:masterfrom
lynchd2:master

Conversation

@lynchd2

@lynchd2 lynchd2 commented Aug 12, 2016

Copy link
Copy Markdown

No description provided.

@hannahsquier

Copy link
Copy Markdown

comment test

@matthinea

Copy link
Copy Markdown

Woooo, comments!!! Yeah

@hannahsquier

Copy link
Copy Markdown

When I logout and then go to the timeline page, I get an error

Comment thread config/routes.rb Outdated
get 'comments/new'

resources :users do
resource :profiles

@hannahsquier hannahsquier Aug 12, 2016

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

use shallow: true if you can
should resource be resources?

@hannahsquier

Copy link
Copy Markdown

when i try to go to a page that doesnt have a user, i get redirected to the home page which is nice, but then i dont have a way to get back to the timeline

@hannahsquier

Copy link
Copy Markdown

the search button breaks the app (as exepected because we haven't done that yet!)

Comment thread app/controllers/posts_controller.rb Outdated
def destroy
@post = Post.find_by_id(params[:id])
@post.destroy
redirect_to current_user

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

probably makes more sense to redirect to the profile you are currently on, not your own profile

Comment thread config/routes.rb
resources :users do
resource :profiles, only: [:edit, :show, :update]
resources :posts, only: [:create, :destroy]
resources :photos

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can make this a shallow route, for both posts and photos.

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.

4 participants