nho: add feedback for BW project2

Gemfile

@@ -47,9 +47,11 @@ group :development do
   # Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring
   gem 'spring'
 
-  group :production do
-    gem 'rails_12factor'
-  end
+end
 
+# NHO: Make sure to move this group outside of the development group!
+group :production do
+  gem 'rails_12factor'
 end
-  gem 'devise'
+
+gem 'devise'

app/controllers/comments_controller.rb

@@ -13,11 +13,13 @@ def create
   end
 
   def edit
+    # NHO: How could we lock down these actions to protect users from editing other users comments?
     @post = Post.find(params[:id])
     @comment = Comment.find(params[:id])
   end
 
   def update
+    # NHO: How could we lock down these actions to protect users from editing other users comments?
     @comment = Comment.find(params[:id])
     @comment.update(comment_params.merge(user:current_user))
     redirect_to posts_path

app/controllers/posts_controller.rb

@@ -1,14 +1,14 @@
 class PostsController < ApplicationController
-  load_and_authorize_resource
+  load_and_authorize_resource # NHO: This helper can be utilized to DRY up our controller code, let's take advantage!
 
   def index
-    @posts = Post.order('updated_at DESC').all
-    @post = Post.where(params[:id])
+    @posts = Post.order('updated_at DESC') # NHO: this works! Have also seen. Post.all.order('updated_at DESC')
+    @post = Post.where(params[:id]) # NHO: I think we want `Post.find` to grab the one post here.
     @comment = Comment.new
   end
 
   def show
-    @post = Post.find(params[:id])
+    @post = Post.find(params[:id]) # NHO: Line no longer necessary because of load part of load_and_authorize_resource, same with edit, update, destroy
   end
 
   def new

app/models/ability.rb

@@ -5,7 +5,7 @@ def initialize(user)
     can :read, Post
     if user
       can :create, Post
-      can [:update, :destroy], Post, :user => user
+      can [:update, :destroy], Post, :user => user # NHO: How could we implement the same rules for comments as well?
     end
   end
 end

app/models/comment.rb

@@ -1,6 +1,7 @@
 class Comment < ActiveRecord::Base
   belongs_to :user
   belongs_to :post
+  # Nice validations!
   validates :content, presence: true,
                     length: { minimum: 1 }
 end

app/models/post.rb

@@ -1,4 +1,4 @@
 class Post < ActiveRecord::Base
   belongs_to :user
-  has_many :comments, dependent: :destroy
+  has_many :comments, dependent: :destroy # NHO: great use of dependent destroy
 end

app/views/comments/_new.html.erb

@@ -5,3 +5,5 @@
     <%= f.submit %>
 
     <% end %>
+
+<!-- NHO: What's the difference between `_new` and `_form`? -->

app/views/posts/_form.html.erb

@@ -1,16 +1,17 @@
 <div class="container">
   <div class="post">
-<%= form_for @post do |f| %>
-  <%= f.label :title, class: "form_title" %>
-  <%= f.text_area :title, class: "form_title"%>
+    <%= form_for @post do |f| %>
+      <%= f.label :title, class: "form_title" %>
+      <%= f.text_area :title, class: "form_title"%>
 
-  <%= f.label :body, class: "form_body" %>
-  <%= f.text_field :body, class: "form_body" %>
+      <%= f.label :body, class: "form_body" %>
+      <%= f.text_field :body, class: "form_body" %>
 
-  <%#= f.label :img_url, class: "form_body" %>
-  <%#= f.text_area :img_url, class: "form_body" %>
+      <!-- NHO: remember to remove commented out/unused code  -->
+      <%#= f.label :img_url, class: "form_body" %>
+      <%#= f.text_area :img_url, class: "form_body" %>
 
-  <%= f.submit %>
-<% end %>
+      <%= f.submit %>
+    <% end %>
   </div>
 </div>

app/views/posts/index.html.erb

@@ -1,43 +1,45 @@
+<!-- NHO: ran code through atom's beautify package to format indentation -->
 <div class="container">
-<div class="title post element">
-<%= link_to 'New Post', new_post_path(@post) %>
-</div>
-
-<% @posts.each do |post| %>
-<div class="post">
-  <div class="title element">
-
-    <div class="create"><%= post.user.email %> said: </div><%= post.title %>
+  <div class="title post element">
+    <%= link_to 'New Post', new_post_path(@post) %>
   </div>
-  <div class="body">
-
-    <%#= image_tag post.img_url %>
-
-    <%= post.body%>
 
-  </div>
+  <% @posts.each do |post| %>
+    <div class="post">
+      <div class="title element">
+        <div class="create"><%= post.user.email %>
+          said:
+        </div><%= post.title %>
+      </div>
+      <div class="body">
 
-  <% post.comments.each do |comment| %>
-  <div class="comments">
-  <%= comment.user.email + " said : "%><%= comment.content + "   "%><%= link_to 'Edit', edit_post_comment_path(post, comment) %><br>
-  </div>
+        <%#= image_tag post.img_url %>  <!-- NHO: Did we ever get this working? Would be happy to pair on this if you want! -->
 
-  <% end %>
-  <div class="body">
+        <%= post.body%>
 
-    <%= form_for([post, @comment]) do |f| %>
-      <div class="flex">
-        <%= f.text_field :content %>
-        <%= f.submit 'Comment', class: 'button element' %>
       </div>
-        <% end %>
-  <% if can? :update, post %>
-  <%= link_to 'Edit Post', edit_post_path(post)%>
-  <% end %>
-  </div>
-</div>
 
-<% end %>
+      <% post.comments.each do |comment| %>
+      <div class="comments">
+        <!-- NHO: how could we use CanCanCan's view helpers or logic to hide this link from those without permission?  -->
+        <%= comment.user.email + " said : "%><%= comment.content + "   "%><%= link_to 'Edit', edit_post_comment_path(post, comment) %><br>
+      </div>
 
+      <% end %>
+      <div class="body">
 
+        <%= form_for([post, @comment]) do |f| %>
+          <div class="flex">
+            <%= f.text_field :content %>
+            <%= f.submit 'Comment', class: 'button element' %>
+          </div>
+        <% end %>
+          <!-- NHO: nice! -->
+        <% if can? :update, post %>
+          <%= link_to 'Edit Post', edit_post_path(post)%>
+        <% end %>
+      </div>
+    </div>
+  <% end %>
 </div>
+<!-- NHO: would recommend commenting your code to improve readibility, visual seperation of concerns  -->

app/views/posts/show.html.erb

@@ -7,10 +7,9 @@
     <%= image_tag @post.img_url %>
     <%= @post.body %>
   </div>
+  <!-- NHO: How could we use CanCanCan's view helpers to hide these links depending on user permissions -->
   <%= link_to 'Edit Post', edit_post_path(@post)%>
-  <%= link_to 'Destroy', post_path(@post),
-              method: :delete,
-              data: { confirm: 'Are you sure?' } %>
+  <%= link_to 'Destroy', post_path(@post), method: :delete, data: { confirm: 'Are you sure?' } %>
 </div>
   <%= link_to 'Back to Posts', posts_path %>
 </div>

config/routes.rb

@@ -3,7 +3,7 @@
   root to: "posts#index"
 
   resources :posts do
-    resources :comments, except: [:index, :show]
+    resources :comments, except: [:index, :show] # NHO: doesnt look like you are using a destroy action either
   end
 
 end

feedback_nho.md

@@ -0,0 +1,34 @@
+# Feedback
+
+## Project Workflow
+
+**Exceeds Expectations**
+
+Great job with the planning process! Nice wirefames, and user stories are clear, and accomplishable!  Would be really cool to see more commentary in your `readme` about your process, things you would like to implemnt! Also, please include set-up instructions for this application.
+
+## Technical Requirements
+
+**Meets Expectations**
+
+Nice job implementing user authentication, and locking down your app's authorization with CanCanCan!
+Demonstrates utilizing nested resources, and at least two models of CRUD.
+
+## Creativity / Interface
+
+**Exceeds Expectations**
+
+Really enjoyed your approach with the visual design, and your creativity was evident in your CSS and layout!
+Loved how most of the app's action occurs on the index page, would like to see you eventually turn this into a Single Page app via partials and/or front-end JS/ajax in the future!
+
+## Code Quality
+
+**Meets Expectations**
+
+Please review [inline code comments](https://github.com/brittonwalker/project_two/compare/master...nolds9:feedback)
+prefixed with my initials: `NHO` for detailed feedback.
+
+## Deployment and Functionality
+
+**Meets Expectations**
+
+Great job hitting your MVP, app is functional, and deployed, though currently I think there might be an issue with the latest version last time I checked. Would like to see you challenge yourself to come back to this an tackle your gold user stories! Make sure to put the link to the deployed url in your repo.