Grades #20

Merged
merged 5 commits into from May 14, 2013

Conversation

Projects
None yet
4 participants
Owner

jmabry111 commented May 6, 2013

No description provided.

@@ -24,4 +25,8 @@ def current_semester
def next_semester
@next_semester ||= Semester.where("starts_on < ? AND ends_on > ?", Time.now + 6.months, Time.now + 6.months).first
end
+
+ def current_grading_period
+ @current_grading_period ||= GradingPeriod.where("starts_on < ? AND ends_on > ?", Time.now + 4.months, Time.now + 4.months).first
@drapergeek

drapergeek May 6, 2013

Collaborator

These should be moved to methods on the model.

  GradingPeriod.for_dates(Time.now+ 4.month, Time.now + 4.moths).first
app/controllers/grades_controller.rb
+ def new
+ @student = Student.find_by_id(params[:student_id])
+ @section = Section.find_by_id(params[:section_id])
+ @current_semester = GradingPeriod.where("semester_id = ?", current_semester.id)
@drapergeek

drapergeek May 6, 2013

Collaborator

You should never have a 'where' caluse in a controller. There should all be methods on the models.

app/controllers/grades_controller.rb
+ def index
+ @student = Student.find_by_id(params[:student_id])
+ @section = Section.find_by_id(params[:section_id])
+ @grades = Grade.where("student_section_enrollment_id = ?", @section.id)
@drapergeek

drapergeek May 6, 2013

Collaborator

Model method.

@@ -0,0 +1,14 @@
+class Instructor::SectionsController < ApplicationController
+
+ skip_before_filter :authenticate_user!
@drapergeek

drapergeek May 6, 2013

Collaborator

Are you sure you want to skip authentication?

+
+ belongs_to :student_section_enrollment
+ belongs_to :grading_period
+ delegate :student, to: :student_section_enrollment, allow_nil: true
@drapergeek

drapergeek May 6, 2013

Collaborator

YAY!

@@ -30,7 +30,7 @@
<tbody>
<% @sections.each do |s| %>
@drapergeek

drapergeek May 6, 2013

Collaborator

Really recommend against using single letter variables.

@@ -2,7 +2,7 @@
<h2 class="left"><%= @student.to_s %></h2>
<div class="row">
<div class="span6">
- <%= link_to "View current schedule", student_schedule_path(@student, 1) %>
+ <%= link_to "View current schedule", student_schedule_path(@student) %>
@drapergeek

drapergeek May 6, 2013

Collaborator

YAY for not passing arbitrary numbers!

@@ -0,0 +1,171 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
@drapergeek

drapergeek May 6, 2013

Collaborator

I think I'd remove this from my repo since you can generate it on the fly. It will be quickly outdated.

Collaborator

drapergeek commented May 6, 2013

Also there are no tests.

Coverage Status

Changes Unknown when pulling cdf4b6e on grades into * on master*.

Btw you need to have this I believe for it to actually work

Coveralls.wear!('rails')
Owner

jmabry111 replied May 8, 2013

Collaborator

drapergeek replied May 8, 2013

Geez @bigbam505, way to burst his bubble!

@jmabry111 jmabry111 merged commit ff068b8 into master May 14, 2013

1 check failed

default The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment