Merge branch 'course-refactor' into 'master'

Course refactor

See merge request !40

schedules/.gitignore

@@ -26,3 +26,6 @@
 /node_modules
 yarn-debug.log*
 .yarn-integrity
+
+coverage
+subjects

schedules/app/controllers/api/course_sections_controller.rb

@@ -11,9 +11,10 @@ class API::CourseSectionsController < ApplicationController
   param :instructor, String, desc: "Get course sections being taught by this instructor"
   def index
     @sections = CourseSection
-                .select('course_sections.*, courses.semester_id, instructors.name AS instructor_name')
-                .joins(:course).where('courses.semester_id = ?', @semester.id)
+                .where(semester: @semester)
+                .joins(:course)
                 .joins(:instructor)
+                .select('course_sections.*, instructors.name AS instructor_name')
 
     if params.key?(:course_id)
       @sections = @sections.where(course_id: params[:course_id])

schedules/app/controllers/api/courses_controller.rb

@@ -8,7 +8,7 @@ class API::CoursesController < ApplicationController
   param :subject, String, desc: 'Course subject, e.g. "CS" or "ACCT"'
   param :course_number, Integer, desc: 'Course number, e.g. "112"'
   def index
-    @courses = Course.where(semester_id: params[:semester_id])
+    @courses = Course.all
 
     if params.key?(:subject)
       @courses = @courses.where("UPPER(courses.subject) LIKE ?", "%#{params[:subject]}%")
@@ -21,7 +21,6 @@ class API::CoursesController < ApplicationController
     result = @courses.map do |c|
       {
         id: c.id,
-        semester_id: c.semester_id,
         subject: c.subject,
         course_number: c.course_number,
         description: c.description,

schedules/app/controllers/api/semesters_controller.rb

+class API::SemestersController < ApplicationController
+  def index
+    result = Semester.all.map do |s|
+      {
+        id: s.id,
+        season: s.season,
+        year: s.year
+      }
+    end
+
+    render json: result
+  end
+end

schedules/app/controllers/courses_controller.rb

@@ -2,18 +2,6 @@ class CoursesController < ApplicationController
   def show
     # Load the course with the id passed in the URL.
     @course = Course.find_by_id(params[:id])
-
-    # If the user changes the semester while looking at a course,
-    # we need to display the page for the course with the same subject and course number
-    # in that semester.
-    unless @course.semester == @semester
-      # find the course we should redirect to
-      @course = Course.find_by(subject: @course.subject, course_number: @course.course_number, semester: @semester)
-
-      # redirect to the url for that course
-      redirect_to course_url(@course)
-    end
-
-    @sections = @course.course_sections
+    @sections = @course.course_sections.where(semester: @semester)
   end
 end

schedules/app/controllers/instructors_controller.rb

@@ -7,7 +7,7 @@ class InstructorsController < ApplicationController
     @instructor = Instructor.find_by_id(params[:id])
 
     # find the courses being taught this semester
-    sections = CourseSection.where(instructor: @instructor).joins(course: :semester).where("semesters.id = ?", @semester.id)
+    sections = CourseSection.where(instructor: @instructor, semester: @semester)
     @courses = Course.build_set(sections)
 
     # build the list of courses the instructor has taught in the past

schedules/app/helpers/api/semesters_helper.rb

+module API::SemestersHelper
+end

schedules/app/helpers/search_helper.rb

@@ -27,15 +27,15 @@ module SearchHelper
     def self.fetchall(search_string, sort_mode: :auto, semester: :fall2018)
       query_data = GenericQueryData.new(search_string, sort_mode, semester)
       models = []
-      models += fetch_instructors query_data
-      models += fetch_courses query_data
+      models += fetch_instructors(query_data)
+      models += fetch_courses(query_data)
       build_list(models)
     end
 
     def self.fetch_instructors(query_data)
       Instructor.from_name(Instructor.select("instructors.*, COUNT(courses.id) AS section_count").from("course_sections"), query_data.search_string)
                 .joins("LEFT OUTER JOIN instructors ON instructors.id = course_sections.instructor_id")
-                .joins("LEFT OUTER JOIN courses ON courses.id = course_sections.course_id AND courses.semester_id = #{query_data.semester.id}")
+                .joins("LEFT OUTER JOIN courses ON courses.id = course_sections.course_id AND course_sections.semester_id = #{query_data.semester.id}")
                 .group("instructors.id").all
     end
 
@@ -46,7 +46,7 @@ module SearchHelper
       base_query = Course.select("courses.*, count(course_sections.id) AS section_count")
                          .left_outer_joins(:course_sections)
                          .having("count(course_sections.id) > 0")
-                         .where("courses.semester_id = ?", query_data.semester)
+                         .where("course_sections.semester_id = ?", query_data.semester)
                          .group("courses.id")
 
       subj = nil

schedules/app/models/course.rb

 # Contains logic regarding the +Course+ model.
 class Course < ApplicationRecord
-  # Each course belongs to a +Semester+
-  belongs_to :semester
   has_many :course_sections
 
   # Ensure all necessary are fields present.
   validates :course_number, presence: true
   validates :subject, presence: true
-  validates :semester_id, presence: true
 
   def full_name
     "#{subject} #{course_number}"

schedules/app/models/course_section.rb

@@ -4,11 +4,15 @@ class CourseSection < ApplicationRecord
   belongs_to :course
   belongs_to :instructor
 
+  # Each course belongs to a +Semester+
+  belongs_to :semester
+
   # Ensure all necessary fields are present.
   validates :name, presence: true
   validates :crn, presence: true
   validates :title, presence: true
   validates :course_id, presence: true
+  validates :semester_id, presence: true
 
   def overlaps?(other)
     t1_start, t1_end = Time.parse(start_time), Time.parse(end_time)
@@ -18,7 +22,7 @@ class CourseSection < ApplicationRecord
   end
 
   def self.latest_by_crn(crn)
-    where(crn: crn).min_by { |s| s.course.semester.id }
+    where(crn: crn).min_by { |s| s.semester.id }
   end
 
   # Select all course sections that have an instructor that matches the given name

schedules/app/models/schedule.rb

@@ -95,7 +95,7 @@ class Schedule
   # @return [Array]
   def exdates_for_section(section)
     # Generate exdates for all closures in a semester
-    exdates = Closure.where(semester: section.course.semester).map { |closure|
+    exdates = Closure.where(semester: section.semester).map { |closure|
       generate_exdate(closure.date.to_formatted_s(:number), section.start_time)
     }
 

schedules/app/views/shared/_course.html.erb

 <% expanded = false unless defined? expanded %>
 
+<% if defined?(@instructor) %>
+    <% sections = course.course_sections.where(instructor: @instructor, semester: @semester).order(:name) %>
+<% else %>
+    <% sections = course.course_sections.where(semester: @semester).order(:name) %>
+<% end %>
+
 <div class="card course-card" id="course-<%= course.id %>">
     <div class="card-header">
 	<div class="row">
@@ -24,7 +30,7 @@
 		<div class="icon">
 		    <i class="fa fa-bars"></i>
 		</div>
-		 <%= course.course_sections.count %> sections
+		 <%= sections.count %> sections
 	    </div>
 	</div>
 	<p class="description"><%= course.description %></p>
@@ -42,11 +48,7 @@
 	
 	<!-- List of Course Sections -->
 	<div class="list-group list-group-flush sections" style="display: <%= expanded ? "flex" : "none" %>">
-	    <% if defined?(@instructor) %>
-		<%= render partial: 'shared/section', collection: course.course_sections.where(instructor: @instructor).order(:name), locals: { course: course } %>
-	    <% else %>
-		<%= render partial: 'shared/section', collection: course.course_sections.order(:name), locals: { course: course } %>
-	    <% end %>
+	    <%= render partial: 'shared/section', collection: sections, locals: { course: course } %>
 	</div>
     </div>
 </div>

schedules/config/routes.rb

@@ -12,6 +12,7 @@ Rails.application.routes.draw do
   get 'schedule/view', to: 'schedules#view', as: 'view_schedule'
 
   scope :api, module: 'api' do # Register /api routes
+    resources :semesters, only: [:index], as: 'api_semesters'
     resources :courses, only: [:index, :show], as: 'api_courses'
     resources :course_sections, only: [:index], as: 'api_course_sections'
     resources :instructors, only: [:index, :show], as: 'api_instructors'

schedules/db/migrate/20190210152552_remove_semester_fk_from_course.rb

+class RemoveSemesterFkFromCourse < ActiveRecord::Migration[5.1]
+  def change
+    remove_column :courses, :semester_id
+    add_reference :course_sections, :semester, foreign_key: true
+  end
+end

schedules/db/schema.rb

@@ -10,14 +10,11 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180927140017) do
-
-  # These are extensions that must be enabled in order to support this database
-  enable_extension "plpgsql"
+ActiveRecord::Schema.define(version: 20190210152552) do
 
   create_table "closures", force: :cascade do |t|
     t.date "date"
-    t.bigint "semester_id"
+    t.integer "semester_id"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.index ["semester_id"], name: "index_closures_on_semester_id"
@@ -38,25 +35,25 @@ ActiveRecord::Schema.define(version: 20180927140017) do
     t.string "campus"
     t.string "notes"
     t.integer "size_limit"
-    t.bigint "course_id"
+    t.integer "course_id"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
-    t.bigint "instructor_id"
+    t.integer "instructor_id"
+    t.integer "semester_id"
     t.index ["course_id"], name: "index_course_sections_on_course_id"
     t.index ["instructor_id"], name: "index_course_sections_on_instructor_id"
+    t.index ["semester_id"], name: "index_course_sections_on_semester_id"
   end
 
   create_table "courses", force: :cascade do |t|
     t.string "subject"
     t.string "course_number"
-    t.bigint "semester_id"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.string "description"
     t.string "credits"
     t.string "title"
     t.string "prereqs"
-    t.index ["semester_id"], name: "index_courses_on_semester_id"
   end
 
   create_table "instructors", force: :cascade do |t|
@@ -72,8 +69,4 @@ ActiveRecord::Schema.define(version: 20180927140017) do
     t.datetime "updated_at", null: false
   end
 
-  add_foreign_key "closures", "semesters"
-  add_foreign_key "course_sections", "courses"
-  add_foreign_key "course_sections", "instructors"
-  add_foreign_key "courses", "semesters"
 end

schedules/db/seeds.rb

@@ -22,7 +22,7 @@ def parse_courses(subjects)
   courses
 end
 
-def load_courses(courses, semester)
+def load_courses(courses)
   insert_hashes = courses.map do |course|
     {
       subject: course[:subject],
@@ -31,7 +31,6 @@ def load_courses(courses, semester)
       credits: course[:credits],
       description: course[:description],
       prereqs: course[:prereqs],
-      semester: semester
     }
   end
 
@@ -64,8 +63,7 @@ def load_sections(sections_in, semester)
       end
 
       course = Course.find_or_create_by!(subject: section[:subj],
-                                         course_number: section[:course_number],
-                                         semester: semester)
+                                         course_number: section[:course_number])
 
       instructor = Instructor.find_or_create_by!(name: section[:instructor])
 
@@ -82,7 +80,8 @@ def load_sections(sections_in, semester)
                         end_time: section[:end_time],
                         location: section[:location],
                         course: course,
-                        instructor: instructor)
+                        instructor: instructor,
+                        semester: semester)
     end
 
     all_sections.each { |s| CourseSection.find_or_create_by! s }
@@ -120,21 +119,19 @@ def main
                 parser.parse_semesters[0..6]
               end
 
-  courses = nil
+  puts "\tParsing subjects..."
+  subjects = parser.parse_subjects(semesters.first[:value])
+
+  puts "\tParsing courses from catalog.gmu.edu..."
+  courses = parse_courses(subjects) if courses.nil?
+
+  puts "\tLoading courses..."
+  load_courses(courses)
 
   semesters.each do |semester|
     puts "#{semester[:season]} #{semester[:year]}"
     db_semester = Semester.find_or_create_by!(season: semester[:season], year: semester[:year])
 
-    puts "\tParsing subjects..."
-    subjects = parser.parse_subjects(semester[:value])
-
-    puts "\tParsing courses from catalog.gmu.edu..."
-    courses = parse_courses(subjects) if courses.nil?
-
-    puts "\tLoading courses..."
-    load_courses(courses, db_semester)
-
     puts "\tParsing sections from Patriot Web..."
     sections_in = parse_sections(semester[:value], subjects)
 

schedules/test/controllers/api/course_sections_controller_test.rb

@@ -2,14 +2,15 @@ require 'test_helper'
 
 class API::CourseSectionsControllerTest < ActionDispatch::IntegrationTest
   test 'should get index' do
-    get api_course_sections_url course_id: courses(:cs112).id, semester_id: semesters(:fall2018).id
+    get api_course_sections_url(course_id: courses(:cs112).id,
+                                semester_id: semesters(:fall2018).id)
+
     assert_response :success
 
     sections_returned = JSON.parse @response.body
     num_sections = CourseSection
-                   .joins(course: :semester)
-                   .where('semesters.id = ?', semesters(:fall2018).id)
-                   .where(course_id: courses(:cs112).id).count
+                   .where(course_id: courses(:cs112).id)
+                   .where(semester: semesters(:fall2018)).count
 
     assert_equal num_sections, sections_returned.count
   end

schedules/test/controllers/api/courses_controller_test.rb

@@ -2,30 +2,30 @@ require 'test_helper'
 
 class API::CoursesControllerTest < ActionDispatch::IntegrationTest
   test '#index should return all courses' do
-    get api_courses_url semester_id: semesters(:fall2018).id
+    get api_courses_url
     assert_response :success
 
     courses_returned = JSON.parse @response.body
-    courses_count = Course.where(semester_id: semesters(:fall2018).id).count
+    courses_count = Course.all.count
     assert_equal courses_count, courses_returned.count
   end
 
   test '#index should return filtered by subject case insensitive' do
-    get api_courses_url subject: "Cs", semester_id: semesters(:fall2018).id
+    get api_courses_url(subject: "Cs")
     assert_response :success
 
     courses_returned = JSON.parse @response.body
-    courses_count = Course.where(subject: "CS", semester_id: semesters(:fall2018).id).count
+    courses_count = Course.where(subject: "CS").count
 
     assert_equal courses_count, courses_returned.count
   end
 
   test '#index should return filtered by subject and course number' do
-    get api_courses_url subject: "CS", course_number: "112", semester_id: semesters(:fall2018).id
+    get api_courses_url(subject: "CS", course_number: "112")
     assert_response :success
 
     courses_returned = JSON.parse @response.body
-    courses_count = Course.where(subject: "CS", course_number: "112", semester_id: semesters(:fall2018).id).count
+    courses_count = Course.where(subject: "CS", course_number: "112").count
 
     assert_equal courses_count, courses_returned.count
   end
@@ -33,7 +33,7 @@ class API::CoursesControllerTest < ActionDispatch::IntegrationTest
   test '#show should return course_sections for course' do
     cs_112_id = courses(:cs112).id
 
-    get api_course_url id: cs_112_id, semester_id: semesters(:fall2018).id
+    get api_course_url(id: cs_112_id)
     assert_response :success
 
     sections_returned = JSON.parse @response.body

schedules/test/controllers/api/semesters_controller_test.rb

+require 'test_helper'
+
+class API::SemestersControllerTest < ActionDispatch::IntegrationTest
+  # test "the truth" do
+  #   assert true
+  # end
+end

schedules/test/controllers/courses_controller_test.rb

@@ -7,6 +7,6 @@ class CoursesControllerTest < ActionDispatch::IntegrationTest
     assert_response :success
 
     # assert every course section is displayed
-    assert_select '.section-item', c.course_sections.count
+    assert_select '.section-item', c.course_sections.where(semester: semesters(:fall2018)).count
   end
 end

schedules/test/fixtures/course_sections.yml

@@ -12,6 +12,7 @@ cs112001:
   location: Innovation Hall 204
   course: cs112
   instructor: kinga
+  semester: fall2018
 
 cs112002:
   name: CS 112 002
@@ -25,6 +26,7 @@ cs112002:
   end_time: 2:00 pm
   course: cs112
   instructor: luke
+  semester: fall2018
 
 cs211001:
   name: CS 211 001
@@ -36,8 +38,9 @@ cs211001:
   start_time: 2:30 pm
   end_time: 3:00 pm
   location: ENGR 200
-  course: cs211spring
+  course: cs211
   instructor: otten
+  semester: spring2018
 
 cs112001spring:
   name: CS 112 001
@@ -49,8 +52,9 @@ cs112001spring:
   start_time: 12:00 pm
   end_time: 1:00 pm
   location: Innovation Hall 204
-  course: cs112spring
+  course: cs112
   instructor: kinga
+  semester: spring2018
 
 acct110001:
   name: ACCT 110 001
@@ -62,5 +66,6 @@ acct110001:
   start_time: 12:00 pm
   end_time: 1:00 pm
   location: Innovation Hall 204
-  course: acct110spring
+  course: acct110
   instructor: business_man
+  semester: spring2018

schedules/test/fixtures/courses.yml

@@ -3,24 +3,13 @@
 cs112:
   subject: CS
   course_number: 112
-  semester: fall2018
 
 cs211:
   subject: CS
   course_number: 211
-  semester: fall2018
 
-acct110spring:
+acct110:
   subject: ACCT
   course_number: 110
-  semester: spring2018
 
-cs110spring:
-  subject: CS
-  course_number: 110
-  semester: spring2018
-
-cs112spring:
-  subject: CS
-  course_number: 112
-  semester: spring2018
+  

schedules/test/models/course_section_test.rb

@@ -13,8 +13,9 @@ class CourseSectionTest < ActiveSupport::TestCase
     CourseSection.create! name: 'Test section',
                           crn: '12345',
                           title: 'Test title',
-                          course_id: courses(:cs211).id,
-                          instructor_id: instructors(:luke).id
+                          course: courses(:cs211),
+                          instructor: instructors(:luke),
+                          semester: semesters(:fall2018)
   end
 
   test '#with_instructor filters correctly' do
@@ -24,6 +25,6 @@ class CourseSectionTest < ActiveSupport::TestCase
 
   test '#latest_by_crn sorts correctly' do
     s = CourseSection.latest_by_crn(70192)
-    assert_equal semesters(:fall2018).id, s.course.semester.id
+    assert_equal semesters(:fall2018).id, s.semester.id
   end
 end

schedules/test/models/course_test.rb

@@ -3,15 +3,15 @@ require 'test_helper'
 class CourseTest < ActiveSupport::TestCase
   test 'fails with improper data' do
     assert_raise do
-      Course.create! course_number: nil, subject: nil, semester_id: nil
+      Course.create! course_number: nil, subject: nil
     end
   end
 
   test 'creates with proper data' do
-    Course.create! course_number: '112', subject: 'CS', semester_id: semesters(:fall2018).id
+    Course.create! course_number: '112', subject: 'CS'
   end
 
   test 'has correct number of sections' do
-    assert_equal 2, courses(:cs112).course_sections.count
+    assert_equal 3, courses(:cs112).course_sections.count
   end
 end

schedules/test/models/semester_test.rb

@@ -10,8 +10,4 @@ class SemesterTest < ActiveSupport::TestCase
   test 'create successful' do
     Semester.create!(season: 'Test', year: 'Test')
   end
-
-  test 'semester has correct number of courses' do
-    assert_equal 2, semesters(:fall2018).courses.count
-  end
 end