Merge pull request #28 from jmkoni/minor-bugfixes

add some minor bugfixes and query optimizations

app/controllers/courses_controller.rb

@@ -21,6 +21,7 @@ def index
   # GET /courses/1.json
   def show
     @reviews = @course.reviews.preload(:user, course: [:school])
+    @url = school_course_reviews_path(school_id: @school, course_id: @course)
   end
 
   # GET /courses/new

app/controllers/reviews_controller.rb

@@ -12,7 +12,7 @@ class ReviewsController < ApplicationController
   def index
     if params[:course_id].to_i.zero?
       if params[:school_id].to_i.zero?
-        @reviews = Review.all
+        @reviews = Review.preload(:user, course: [:school]).all
       else
         set_school
         @reviews = Review.preload(:user, course: [:school]).joins(:course).where('courses.school_id = (?)', @school.id)
@@ -42,6 +42,7 @@ def edit
   # POST /schools/1/courses/1/reviews
   # POST /schools/1/courses/1/reviews.json
   def create
+    @url = school_course_reviews_path
     @review = Review.new(review_params)
     respond_to do |format|
       if @review.save
@@ -83,7 +84,11 @@ def update
   def destroy
     @review.destroy
     respond_to do |format|
-      format.html { redirect_to school_course_reviews_url, notice: 'Review was successfully destroyed.' }
+      format.html do
+        redirect_to school_course_reviews_url(school_id: @school,
+                                              course_id: @course),
+                    notice: 'Review was successfully destroyed.'
+      end
       format.json { head :no_content }
     end
   end

app/controllers/users_controller.rb

@@ -59,7 +59,7 @@ def destroy
   # PUT /users/1/promote
   def promote
     @user.update(is_admin: true)
-    redirect_to users_url, notice: "#{@user.email} was successfully demoted from admin."
+    redirect_to users_url, notice: "#{@user.email} was successfully promoted to admin."
   end
 
   # PUT /users/1/demote

app/models/ability.rb

@@ -15,7 +15,7 @@ def initialize(user)
     can [:update, :delete], Review do |r|
       r.user == user
     end
-    can [:update, :delete], User do |u|
+    can [:read, :update, :delete], User do |u|
       u == user
     end
     return unless user.admin?

app/views/courses/edit.html.erb

@@ -2,4 +2,4 @@
 
 <%= render 'form', course: @course %>
 
-<%= link_to 'Show', school_course_path(school_id: @school.id, id: @course.id) %> |
+<%= link_to 'Show', school_course_path(school_id: @school.id, id: @course.id) %>

app/views/courses/index.html.erb

@@ -1,5 +1,3 @@
-<p id="notice"><%= notice %></p>
-
 <h1>Courses</h1>
 <% if can? :create, Course %>
   <% if @school %>
@@ -8,30 +6,34 @@
     <%= link_to 'Select a school first to add a new course', schools_path %>
   <% end %>
 <% end %>
-
-<table class="table table-hover table-sm table-responsive">
-  <thead class="thead-light">
-    <tr>
-      <th scope="col">Name</th>
-      <th scope="col">Number</th>
-      <th scope="col">Department</th>
-      <th scope="col">School</th>
-      <th scope="col" colspan="3"></th>
-    </tr>
-  </thead>
-
-  <tbody>
-    <% @courses.each do |course| %>
+<br/>
+<% if @courses.load.empty? %>
+  No courses at this time!
+<% else %>
+  <table class="table table-hover table-sm table-responsive">
+    <thead class="thead-light">
       <tr>
-        <td><%= link_to course.name, school_course_path(id: course.id, school_id: course.school.id) %></td>
-        <td><%= course.number %></td>
-        <td><%= course.department %></td>
-        <td><%= link_to course.school.name, school_path(id: course.school) %><td>
-        <% if can? :edit, course %>
-          <td><%= link_to 'Edit', edit_school_course_path(id: course.id, school_id: course.school.id) %></td>
-          <td><%= link_to 'Destroy', school_course_path(id: course.id, school_id: course.school.id), method: :delete, data: { confirm: 'Are you sure?' } %></td>
-        <% end %>
+        <th scope="col">Name</th>
+        <th scope="col">Number</th>
+        <th scope="col">Department</th>
+        <th scope="col">School</th>
+        <th scope="col" colspan="3"></th>
       </tr>
-    <% end %>
-  </tbody>
-</table>
+    </thead>
+
+    <tbody>
+      <% @courses.each do |course| %>
+        <tr>
+          <td><%= link_to course.name, school_course_path(id: course.id, school_id: course.school.id) %></td>
+          <td><%= course.number %></td>
+          <td><%= course.department %></td>
+          <td><%= link_to course.school.name, school_path(id: course.school) %><td>
+          <% if can? :edit, course %>
+            <td><%= link_to 'Edit', edit_school_course_path(id: course.id, school_id: course.school.id) %></td>
+            <td><%= link_to 'Destroy', school_course_path(id: course.id, school_id: course.school.id), method: :delete, data: { confirm: 'Are you sure?' } %></td>
+          <% end %>
+        </tr>
+      <% end %>
+    </tbody>
+  </table>
+<% end %>

app/views/courses/show.html.erb

@@ -1,7 +1,7 @@
 <h2>
     <%= @course.name %> at <%= link_to @course.school.name, @course.school %>
 </h2> <% if can? :edit, @course %>
-<%= link_to 'Edit', edit_school_course_path(@course) %>
+<%= link_to 'Edit', edit_school_course_path(school_id: @school.id, course_id: @course.id) %>
 <% end %>
 <p>
   <strong>Number:</strong>
@@ -15,6 +15,9 @@
 
 <p>
   <h4>Reviews</h4>
-  <%= link_to 'New Review', new_school_course_review_path(school_id: @school.id, course_id: @course.id) %>
+  <% if can? :create, Review %>
+    <%= link_to 'New Review', new_school_course_review_path(school_id: @school.id, course_id: @course.id) %>
+  <% end %>
+  <br/>
   <%= render 'reviews/review_index', reviews: @reviews %>
 </p>

app/views/reviews/_review_index.html.erb

@@ -1,4 +1,4 @@
-<% if reviews.empty? %>
+<% if reviews.load.empty? %>
 No reviews at this time!
 <% else %>
   <table class="table table-hover table-sm table-responsive">

app/views/users/index.html.erb

@@ -1,7 +1,6 @@
-<p id="notice"><%= notice %></p>
-
-<h1>Users</h1>
-
+<% if can? :manage, User %>
+<h1>Users</h1><!--
+<%= link_to 'New User', new_user_path %> -->
 <table class="table table-hover table-sm table-responsive">
   <thead class="thead-light">
     <tr>
@@ -29,7 +28,4 @@
     <% end %>
   </tbody>
 </table>
-
-<br>
-
-<%= link_to 'New User', new_user_path %>
+<% end %>

coverage/.resultset.json

@@ -120,6 +120,7 @@
         null,
         1,
         2,
+        2,
         null,
         null,
         null,
@@ -251,6 +252,7 @@
         5,
         5,
         5,
+        5,
         2,
         2,
         null,
@@ -413,7 +415,7 @@
         null,
         null,
         1,
-        4,
+        5,
         null,
         null,
         null,
@@ -664,6 +666,6 @@
         null
       ]
     },
-    "timestamp": 1559940230
+    "timestamp": 1560033163
   }
 }