From c01902c3d14b66f5b53efb04539f0071f6e90600 Mon Sep 17 00:00:00 2001 From: Zac Wood Date: Sat, 8 Feb 2020 11:13:39 -0500 Subject: [PATCH 1/8] start cleaning up --- .../app/controllers/concerns/by_semester.rb | 4 ++-- .../controllers/course_sections_controller.rb | 4 +++- .../app/controllers/courses_controller.rb | 21 ++++++++++++------- schedules/app/models/course.rb | 4 ++-- schedules/app/models/semester.rb | 4 ++-- schedules/config/routes.rb | 2 +- schedules/db/load_course_ratings.rb | 1 - schedules/test/fixtures/semesters.yml | 16 ++++++++++++++ schedules/test/models/semester_test.rb | 14 +++++++++++-- 9 files changed, 52 insertions(+), 18 deletions(-) diff --git a/schedules/app/controllers/concerns/by_semester.rb b/schedules/app/controllers/concerns/by_semester.rb index 9323dba..48951a1 100644 --- a/schedules/app/controllers/concerns/by_semester.rb +++ b/schedules/app/controllers/concerns/by_semester.rb @@ -7,13 +7,13 @@ module BySemester end # This page needs to know what semester it should load data from. - # set_semester checks both the semester_id query parameter and the current session + # set_semester checks the semester_id query parameter # to look for a semester id and loads whatever it finds into @semester. # # By default, load the most recent semester. def set_semester @semester = if params.key?(:semester_id) - Semester.find_by_id(params[:semester_id]) + Semester.find(params[:semester_id]) else Semester.sorted_by_date.first end diff --git a/schedules/app/controllers/course_sections_controller.rb b/schedules/app/controllers/course_sections_controller.rb index 6adfbc8..98b95b2 100644 --- a/schedules/app/controllers/course_sections_controller.rb +++ b/schedules/app/controllers/course_sections_controller.rb @@ -1,4 +1,5 @@ class CourseSectionsController < ApplicationController + # index renders the view that is asyncronously fetched for the /schedule page def index crns = params[:crns].split(',') @sections = crns.map { |crn| CourseSection.latest_by_crn(crn) } @@ -26,10 +27,11 @@ def index end end + # Don't render the normal HTML head, just render what's in the view file render(layout: false) end def show - @section = CourseSection.find_by_id(params[:id]) + @section = CourseSection.find(params[:id]) end end diff --git a/schedules/app/controllers/courses_controller.rb b/schedules/app/controllers/courses_controller.rb index 0cd32c6..ed1ce71 100644 --- a/schedules/app/controllers/courses_controller.rb +++ b/schedules/app/controllers/courses_controller.rb @@ -4,28 +4,35 @@ def show @course = Course.find_by_id(params[:id]) @rating = @course.rating - semester_ids = Set.new(@course.course_sections.map(&:semester_id)).to_a + semester_ids = @course.course_sections.pluck(:semester_id).uniq @semesters = Semester.where(id: semester_ids) @semesters = Semester.sorted_by_date(@semesters) - @taught_in = Set.new(@semesters.map(&:season)) - @taught_in = if @taught_in.empty? - "Has not been offered since #{Semester.sorted_by_date.last.to_s}" + # To get a list of the seasons this course was taught in, + # put them in a Set to get rid of duplicates, then sort them chronologically + seasons_taught = sort_seasons(@semesters.pluck(:season).uniq) + @taught_in = if seasons_taught.empty? + "Has not been offered since #{Semester.sorted_by_date.last}" else - "Has been offered in #{(@taught_in.to_a).join(", ")}" + "Has been offered in #{seasons_taught.join(", ")}" end - if @semesters.first != Semester.sorted_by_date.first + # If this course is not being taught this semester, the current semester will not + # be in @semesters. However, users expect the default semester to be the current semester, + # so add the current semester as the first semester to ensure that will be the case. + unless @semesters.include?(Semester.sorted_by_date.first) @semesters = [Semester.sorted_by_date.first, *@semesters] end + # Get all the sections in the current semester and group them by their section type. @sections = @course.course_sections.where(semester: @semester).group_by { |s| s.section_type } end private def sort_seasons(seasons) + # Sort by Spring < Summer < Fall seasons.sort do |s1, s2| case when s1 == "Fall" @@ -39,4 +46,4 @@ def sort_seasons(seasons) end end end -end \ No newline at end of file +end diff --git a/schedules/app/models/course.rb b/schedules/app/models/course.rb index a443395..c2f919a 100644 --- a/schedules/app/models/course.rb +++ b/schedules/app/models/course.rb @@ -3,8 +3,8 @@ class Course < ApplicationRecord has_many :course_sections # Ensure all necessary are fields present. - validates :course_number, presence: true - validates :subject, presence: true + validates :course_number, format: { with: /^[0-9]{3}$/ } + validates :subject, format: { with: /^[A-Z]{2,4}$/ } def full_name "#{subject} #{course_number}" diff --git a/schedules/app/models/semester.rb b/schedules/app/models/semester.rb index c0d721c..46832a1 100644 --- a/schedules/app/models/semester.rb +++ b/schedules/app/models/semester.rb @@ -6,8 +6,8 @@ class Semester < ApplicationRecord has_many :closures # Ensure necessary fields are present. - validates :year, presence: true - validates :season, presence: true + validates :year, format: { with: /[0-9]{4}/, message: "must be a valid year" } + validates :season, inclusion: { in: %w(Spring Summer Fall), message: "must be a valid season" } def to_s "#{season} #{year}" diff --git a/schedules/config/routes.rb b/schedules/config/routes.rb index 36e06e9..f9d7fb6 100644 --- a/schedules/config/routes.rb +++ b/schedules/config/routes.rb @@ -1,9 +1,9 @@ # Registers all routes for the app. Rails.application.routes.draw do - get 'about', to: 'about#index', as: 'about' get '/', to: 'home#index', as: 'home' get 'search', to: 'search#index', as: 'search' + get 'about', to: 'about#index', as: 'about' resources :courses, only: [:show] resources :course_sections, only: [:index, :show] diff --git a/schedules/db/load_course_ratings.rb b/schedules/db/load_course_ratings.rb index b870b90..53bde8d 100644 --- a/schedules/db/load_course_ratings.rb +++ b/schedules/db/load_course_ratings.rb @@ -10,7 +10,6 @@ File.new('db/data/sems_loaded.txt', 'w') end -f = File.open('db/data/sems_loaded.txt', 'a') (Dir.entries("db/data").select { |f| f.end_with? ".json" }).each do |file| /(?[[:alpha:]]{1,2})(?[0-9]{2})/.match(file) do |m| if sems.include? m.to_s diff --git a/schedules/test/fixtures/semesters.yml b/schedules/test/fixtures/semesters.yml index 3fec11c..086b271 100644 --- a/schedules/test/fixtures/semesters.yml +++ b/schedules/test/fixtures/semesters.yml @@ -7,3 +7,19 @@ fall2018: spring2018: season: Spring year: 2018 + +summer2018: + season: Summer + year: 2018 + +fall2019: + season: Fall + year: 2019 + +spring2019: + season: Spring + year: 2019 + +summer2019: + season: Summer + year: 2019 diff --git a/schedules/test/models/semester_test.rb b/schedules/test/models/semester_test.rb index 9c06415..836f7bb 100644 --- a/schedules/test/models/semester_test.rb +++ b/schedules/test/models/semester_test.rb @@ -7,7 +7,17 @@ class SemesterTest < ActiveSupport::TestCase end end - test 'create successful' do - Semester.create!(season: 'Test', year: 'Test') + test 'create fails with invalid data' do + assert_raise do + Semester.create!(season: 'asdf', year: '2000') + end + + assert_raise do + Semester.create!(season: 'Spring', year: 'asdf') + end + end + + test 'create succeeds with valid data' do + Semester.create!(season: 'Spring', year: '2019') end end From c65aefc9095c45af64d14fd60e18b1dd9e647eb3 Mon Sep 17 00:00:00 2001 From: Zac Wood Date: Sun, 9 Feb 2020 21:29:36 -0500 Subject: [PATCH 2/8] Lots of comments and refactoring --- schedules/app/controllers/about_controller.rb | 5 + .../controllers/api/schedules_controller.rb | 2 + .../app/controllers/application_controller.rb | 2 +- .../app/controllers/concerns/by_semester.rb | 17 ++ .../app/controllers/courses_controller.rb | 34 +-- schedules/app/controllers/home_controller.rb | 4 + .../app/controllers/instructors_controller.rb | 17 +- .../app/controllers/schedules_controller.rb | 3 +- .../app/controllers/search_controller.rb | 13 +- schedules/app/helpers/application_helper.rb | 15 ++ schedules/app/models/course.rb | 4 +- schedules/app/models/instructor.rb | 20 +- schedules/app/views/courses/index.html.erb | 2 - schedules/app/views/instructors/show.html.erb | 6 +- schedules/app/views/schedules/show.html.erb | 193 ------------------ schedules/app/views/shared/_section.html.erb | 4 +- schedules/config/routes.rb | 28 ++- schedules/test/models/instructor_test.rb | 2 +- 18 files changed, 113 insertions(+), 258 deletions(-) delete mode 100644 schedules/app/views/courses/index.html.erb delete mode 100644 schedules/app/views/schedules/show.html.erb diff --git a/schedules/app/controllers/about_controller.rb b/schedules/app/controllers/about_controller.rb index 65749b3..267386f 100644 --- a/schedules/app/controllers/about_controller.rb +++ b/schedules/app/controllers/about_controller.rb @@ -1,3 +1,8 @@ class AboutController < ApplicationController + + # Suprise -- this page is left in from Schedules v3. + # No way to get to it expect manually navigating to /about. + # Leaving it in in case we ever want to add one again. def index; end + end diff --git a/schedules/app/controllers/api/schedules_controller.rb b/schedules/app/controllers/api/schedules_controller.rb index de12a89..df93ee3 100644 --- a/schedules/app/controllers/api/schedules_controller.rb +++ b/schedules/app/controllers/api/schedules_controller.rb @@ -1,4 +1,6 @@ class API::SchedulesController < ApplicationController + include BySemester + # Render an iCal file containing the schedules of all the # course sections with the given CRNs. def index diff --git a/schedules/app/controllers/application_controller.rb b/schedules/app/controllers/application_controller.rb index 03a6797..9df53ed 100644 --- a/schedules/app/controllers/application_controller.rb +++ b/schedules/app/controllers/application_controller.rb @@ -1,4 +1,4 @@ # Configures the application. class ApplicationController < ActionController::Base - include BySemester + include ApplicationHelper end diff --git a/schedules/app/controllers/concerns/by_semester.rb b/schedules/app/controllers/concerns/by_semester.rb index 48951a1..3f58958 100644 --- a/schedules/app/controllers/concerns/by_semester.rb +++ b/schedules/app/controllers/concerns/by_semester.rb @@ -18,4 +18,21 @@ def set_semester Semester.sorted_by_date.first end end + + # Get a sorted list of semesters from a given list of sections + def semesters_from(sections) + # "pluck" each semester_id from every section in the list of sections + semester_ids = sections.pluck(:semester_id).uniq + semesters = Semester.where(id: semester_ids) + semesters = Semester.sorted_by_date(semesters) + + # If this course is not being taught this semester, the current semester will not + # be in @semesters. However, users expect the default semester to be the current semester, + # so add the current semester as the first semester to ensure that will be the case. + unless semesters.include?(Semester.sorted_by_date.first) + semesters = [Semester.sorted_by_date.first, *semesters] + end + + semesters + end end diff --git a/schedules/app/controllers/courses_controller.rb b/schedules/app/controllers/courses_controller.rb index ed1ce71..1314d15 100644 --- a/schedules/app/controllers/courses_controller.rb +++ b/schedules/app/controllers/courses_controller.rb @@ -1,12 +1,12 @@ class CoursesController < ApplicationController + include BySemester + def show # Load the course with the id passed in the URL. - @course = Course.find_by_id(params[:id]) + @course = Course.find(params[:id]) @rating = @course.rating - semester_ids = @course.course_sections.pluck(:semester_id).uniq - @semesters = Semester.where(id: semester_ids) - @semesters = Semester.sorted_by_date(@semesters) + @semesters = semesters_from(@course.course_sections) # To get a list of the seasons this course was taught in, # put them in a Set to get rid of duplicates, then sort them chronologically @@ -18,32 +18,8 @@ def show end - # If this course is not being taught this semester, the current semester will not - # be in @semesters. However, users expect the default semester to be the current semester, - # so add the current semester as the first semester to ensure that will be the case. - unless @semesters.include?(Semester.sorted_by_date.first) - @semesters = [Semester.sorted_by_date.first, *@semesters] - end - # Get all the sections in the current semester and group them by their section type. - @sections = @course.course_sections.where(semester: @semester).group_by { |s| s.section_type } + @sections = @course.course_sections.where(semester: @semester).group_by(&:section_type) end - private - - def sort_seasons(seasons) - # Sort by Spring < Summer < Fall - seasons.sort do |s1, s2| - case - when s1 == "Fall" - -1 - when s1 == "Summer" && s2 == "Fall" - 1 - when s1 == "Spring" - 1 - else - 0 - end - end - end end diff --git a/schedules/app/controllers/home_controller.rb b/schedules/app/controllers/home_controller.rb index f4e8890..226ff89 100644 --- a/schedules/app/controllers/home_controller.rb +++ b/schedules/app/controllers/home_controller.rb @@ -1,4 +1,8 @@ # HomeController renders the homepage. It currently requires no logic. class HomeController < ApplicationController + + # Nothing to do here. + # Rails will just render the view in app/views/home/index.html.erb def index; end + end diff --git a/schedules/app/controllers/instructors_controller.rb b/schedules/app/controllers/instructors_controller.rb index 61458bd..a950ce7 100644 --- a/schedules/app/controllers/instructors_controller.rb +++ b/schedules/app/controllers/instructors_controller.rb @@ -1,20 +1,15 @@ class InstructorsController < ApplicationController + include BySemester + def show - @instructor = Instructor.find_by_id(params[:id]) + @instructor = Instructor.find(params[:id]) # find the courses being taught this semester sections = CourseSection.where(instructor: @instructor) - semester_ids = Set.new(sections.pluck(:semester_id)) - - @semesters = Semester.where(id: semester_ids) - @semesters = Semester.sorted_by_date(@semesters) - - @sections = sections.where(semester: @semester).group_by { |s| s.section_type } + @semesters = semesters_from(sections) - if @semesters.first != Semester.sorted_by_date.first - @semesters = [Semester.sorted_by_date.first, *@semesters] - end + @sections = sections.where(semester: @semester).group_by(&:section_type) - @rating = { teaching: @instructor.rating, respect: @instructor.rating(6) } + @rating = @instructor.rating(:teaching) end end diff --git a/schedules/app/controllers/schedules_controller.rb b/schedules/app/controllers/schedules_controller.rb index 0c43d73..03b4bc6 100644 --- a/schedules/app/controllers/schedules_controller.rb +++ b/schedules/app/controllers/schedules_controller.rb @@ -1,3 +1,4 @@ class SchedulesController < ApplicationController def index; end -end \ No newline at end of file + def show; end +end diff --git a/schedules/app/controllers/search_controller.rb b/schedules/app/controllers/search_controller.rb index 786e320..44fd7d8 100644 --- a/schedules/app/controllers/search_controller.rb +++ b/schedules/app/controllers/search_controller.rb @@ -1,13 +1,16 @@ class SearchController < ApplicationController + + # Searching is hard. This code isn't very good, but it works for the most part. + # It trys to match the search query to different regular expressions + # and attempts to take you to the correct place based on which one matches. + # + # - CRNs get sent to the course page that contains the CRN + # - Queries of the form "Subject CourseNum", like "CS 112", get sent to that course + # - Otherwise search every course's title and description, along with instutor names and display whatever matches def index params[:query].strip! redirect_to(home_url) unless params[:query].length > 1 - if params[:query].casecmp('god').zero? - bell = Instructor.find_by_name('Jonathan Bell') - redirect_to(instructor_url(bell)) - end - @instructors = nil @courses = nil diff --git a/schedules/app/helpers/application_helper.rb b/schedules/app/helpers/application_helper.rb index de6be79..1e745f4 100644 --- a/schedules/app/helpers/application_helper.rb +++ b/schedules/app/helpers/application_helper.rb @@ -1,2 +1,17 @@ module ApplicationHelper + def sort_seasons(seasons) + # Sort by Spring < Summer < Fall + seasons.sort do |s1, s2| + case + when s1 == "Fall" + -1 + when s1 == "Summer" && s2 == "Fall" + 1 + when s1 == "Spring" + 1 + else + 0 + end + end + end end diff --git a/schedules/app/models/course.rb b/schedules/app/models/course.rb index c2f919a..eae7426 100644 --- a/schedules/app/models/course.rb +++ b/schedules/app/models/course.rb @@ -3,8 +3,8 @@ class Course < ApplicationRecord has_many :course_sections # Ensure all necessary are fields present. - validates :course_number, format: { with: /^[0-9]{3}$/ } - validates :subject, format: { with: /^[A-Z]{2,4}$/ } + validates :course_number, format: { with: /[0-9]{3}/ } + validates :subject, format: { with: /[A-Z]{2,4}/ } def full_name "#{subject} #{course_number}" diff --git a/schedules/app/models/instructor.rb b/schedules/app/models/instructor.rb index 7b8b264..fd9e172 100644 --- a/schedules/app/models/instructor.rb +++ b/schedules/app/models/instructor.rb @@ -7,17 +7,25 @@ class Instructor < ApplicationRecord end } - def self.from_name(base_query, name) - base_query.where("upper(instructors.name) LIKE ?", "%#{name.upcase}%") - end + # All the questions in a course evaluation + QUESTIONS = %i(teaching course expectations + organized understand feedback + respect accessible grading_expectations + graded_work assignments textbook + return_time syllabus stimulating active_involvement) + + def rating(question) + question_i = QUESTIONS.find_index(question) + raise ArgumentError, "Question must be one of: #{QUESTIONS}" if question_i.nil? + + sections = CourseSection.where(instructor_id: id) - def rating(question = 0, sections = CourseSection.where(instructor_id: id)) total = 0 resp = 0 sections.each do |s| next if s.rating_questions.empty? - resp += s.rating_questions[question]["resp"].to_i - total += s.rating_questions[question]["instr_mean"].to_f * s.rating_questions[0]["resp"].to_i + resp += s.rating_questions[question_i]["resp"].to_i + total += s.rating_questions[question_i]["instr_mean"].to_f * s.rating_questions[0]["resp"].to_i end [(total / resp).round(2), resp] unless resp.zero? diff --git a/schedules/app/views/courses/index.html.erb b/schedules/app/views/courses/index.html.erb deleted file mode 100644 index 1d072e8..0000000 --- a/schedules/app/views/courses/index.html.erb +++ /dev/null @@ -1,2 +0,0 @@ -

Courses#index

-

Find me in app/views/courses/index.html.erb

diff --git a/schedules/app/views/instructors/show.html.erb b/schedules/app/views/instructors/show.html.erb index 7060222..a5c7fb6 100644 --- a/schedules/app/views/instructors/show.html.erb +++ b/schedules/app/views/instructors/show.html.erb @@ -4,9 +4,9 @@

<%= @instructor.name %>

- <% unless @rating[:teaching].nil? %> - <%= render(partial: 'shared/stars', locals: { percent: @rating[:teaching][0]/5*100 }) %> - <%= @rating[:teaching][0] %> / <%= @rating[:teaching][1] %> responses + <% unless @rating.nil? %> + <%= render(partial: 'shared/stars', locals: { percent: @rating[0]/5*100 }) %> + <%= @rating[0] %> / <%= @rating[1] %> responses <% end %>
diff --git a/schedules/app/views/schedules/show.html.erb b/schedules/app/views/schedules/show.html.erb deleted file mode 100644 index aa5f084..0000000 --- a/schedules/app/views/schedules/show.html.erb +++ /dev/null @@ -1,193 +0,0 @@ -<%= render(partial: 'shared/navbar') %> - -
-

Your Schedule

- -
- -
-
- - -
diff --git a/schedules/app/views/shared/_section.html.erb b/schedules/app/views/shared/_section.html.erb index 7908a64..e3bc128 100644 --- a/schedules/app/views/shared/_section.html.erb +++ b/schedules/app/views/shared/_section.html.erb @@ -15,8 +15,8 @@ local_assigns.has_key? lets us check if a variable with a given name is defined. So if that variable is defined, don't show the stars. %> - <% unless section.instructor.rating.nil? || local_assigns.has_key?(:on_instructor_page)%> - <%= render partial: 'shared/stars', locals: { percent: (section.instructor.rating[0] / 5 * 100).to_i }%> + <% unless section.instructor.rating(:teaching).nil? || local_assigns.has_key?(:on_instructor_page)%> + <%= render partial: 'shared/stars', locals: { percent: (section.instructor.rating(:teaching)[0] / 5 * 100).to_i }%> <% end %> <% end %>
diff --git a/schedules/config/routes.rb b/schedules/config/routes.rb index f9d7fb6..706391c 100644 --- a/schedules/config/routes.rb +++ b/schedules/config/routes.rb @@ -1,18 +1,42 @@ # Registers all routes for the app. +# +# This stuff is wacky magic. For an in depth guide +# to what the heck is going on, check out +# "Rails Routing from the Outside In": https://guides.rubyonrails.org/routing.html Rails.application.routes.draw do + # GET schedules.gmu.edu routes to HomeController#index get '/', to: 'home#index', as: 'home' + + # GET schedules.gmu.edu/search routes to SearchController#index get 'search', to: 'search#index', as: 'search' + + # GET schedules.gmu.edu/search routes to AboutController#index get 'about', to: 'about#index', as: 'about' + # GET schedules.gmu.edu/courses/:id routes to CoursesController#show resources :courses, only: [:show] + + # GET schedules.gmu.edu/courses_sections/:id routes to CourseSectionsController#show + # - this is for viewing the ratings for that section + # + # GET schedules.gmu.edu/course_sections routes to CourseSectionsController#index + # - this expects a list of crns as a query paramater + # - used to render the schedule for the /schedule page resources :course_sections, only: [:index, :show] + + # GET schedules.gmu.edu/instructors/:id routes to InstructorsController#show resources :instructors, only: [:show] - get 'schedule', to: 'schedules#show', as: 'schedule' - scope :api, module: 'api' do # Register /api routes + # GET schedules.gmu.edu/schedule routes to SchedulesController#show + get 'schedule', to: 'schedules#index', as: 'schedule' + + # Register /api routes + scope :api, module: 'api' do + # Renders the iCal for calendar subscriptions resources :schedules, only: [:index], as: 'api_schedules' end + # Set the home route as the root. Not sure if necessary but why not. root to: 'home#index' end diff --git a/schedules/test/models/instructor_test.rb b/schedules/test/models/instructor_test.rb index e5e3452..d6948d6 100644 --- a/schedules/test/models/instructor_test.rb +++ b/schedules/test/models/instructor_test.rb @@ -2,6 +2,6 @@ class InstructorTest < ActiveSupport::TestCase test "Instructor#named filters correctly" do - assert_equal 2, Instructor.from_name(Instructor.select("*"), "luke").count + assert_equal 2, Instructor.named("luke").count end end From d388169f818c0424fbace24c1156b34aaac063a4 Mon Sep 17 00:00:00 2001 From: Zac Wood Date: Sun, 9 Feb 2020 21:44:17 -0500 Subject: [PATCH 3/8] test for security checks --- schedules/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/schedules/README.md b/schedules/README.md index 674408b..8a30ab4 100644 --- a/schedules/README.md +++ b/schedules/README.md @@ -1,3 +1,4 @@ TODO: Add information about different folders for now, ping @zacwood on slack if you have questions! + From 07c7676d09d5a972dfe31d3070b28192b7c20a98 Mon Sep 17 00:00:00 2001 From: Zac Wood Date: Sun, 9 Feb 2020 21:54:55 -0500 Subject: [PATCH 4/8] try github action for testing --- schedules/.github/workflows/rails.yaml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 schedules/.github/workflows/rails.yaml diff --git a/schedules/.github/workflows/rails.yaml b/schedules/.github/workflows/rails.yaml new file mode 100644 index 0000000..01e8d17 --- /dev/null +++ b/schedules/.github/workflows/rails.yaml @@ -0,0 +1,21 @@ +name: Rails Test + +on: [push] + +jobs: + build: + runs-on: ubuntu-latest + steps: + - name: Clone Schedules + uses: actions/checkout@v2 + + - name: Set up Ruby 2.6 + uses: actions/setup-ruby@v1 + with: + ruby-version: 2.6.x + + - name: Build and test + run: | + gem install bundler + bundle install + bundle exec rails help From 8c849ee14965e9fccf0d9b410dfb3e46a6a45890 Mon Sep 17 00:00:00 2001 From: Zac Wood Date: Sun, 9 Feb 2020 21:56:08 -0500 Subject: [PATCH 5/8] move github dir --- {schedules/.github => .github}/workflows/rails.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {schedules/.github => .github}/workflows/rails.yaml (100%) diff --git a/schedules/.github/workflows/rails.yaml b/.github/workflows/rails.yaml similarity index 100% rename from schedules/.github/workflows/rails.yaml rename to .github/workflows/rails.yaml From 1e7e58f3afca37a6f3a552937743922d7e2f2cc1 Mon Sep 17 00:00:00 2001 From: Zac Wood Date: Sun, 9 Feb 2020 21:57:32 -0500 Subject: [PATCH 6/8] change to correct directory --- .github/workflows/rails.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/rails.yaml b/.github/workflows/rails.yaml index 01e8d17..81bda68 100644 --- a/.github/workflows/rails.yaml +++ b/.github/workflows/rails.yaml @@ -16,6 +16,7 @@ jobs: - name: Build and test run: | + cd schedules gem install bundler bundle install bundle exec rails help From 4b17b45e8fd30218543e953ec460bb3b34f34a97 Mon Sep 17 00:00:00 2001 From: Zac Wood Date: Sun, 9 Feb 2020 22:02:28 -0500 Subject: [PATCH 7/8] install sql dev libraries --- .github/workflows/rails.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/rails.yaml b/.github/workflows/rails.yaml index 81bda68..764622d 100644 --- a/.github/workflows/rails.yaml +++ b/.github/workflows/rails.yaml @@ -18,5 +18,6 @@ jobs: run: | cd schedules gem install bundler + sudo apt-get install -y mysql-client libmysqlclient-dev bundle install bundle exec rails help From 0bb27bb6339ab1616f6c42ff5018819839a578f1 Mon Sep 17 00:00:00 2001 From: Zac Wood Date: Thu, 20 Feb 2020 21:47:35 -0500 Subject: [PATCH 8/8] lots of updates and clean up --- .../app/assets/stylesheets/application.scss | 10 ++ .../controllers/api/schedules_controller.rb | 4 +- .../controllers/course_sections_controller.rb | 32 ----- .../app/controllers/schedules_controller.rb | 36 +++++- .../app/controllers/search_controller.rb | 6 +- schedules/app/helpers/schedules_helper.rb | 27 ---- .../src/controllers/schedule_controller.js | 2 +- schedules/app/models/course.rb | 2 +- schedules/app/models/course_section.rb | 8 +- schedules/app/models/instructor.rb | 2 +- schedules/app/models/semester.rb | 4 +- .../app/views/course_sections/index.html.erb | 7 -- schedules/app/views/courses/show.html.erb | 4 +- schedules/app/views/instructors/show.html.erb | 4 +- .../app/views/layouts/application.html.erb | 118 +++++++++--------- .../app/views/schedules/sections.html.erb | 7 ++ schedules/app/views/search/index.html.erb | 17 +-- schedules/app/views/shared/_course.html.erb | 58 +++------ .../app/views/shared/_instructor.html.erb | 15 +-- schedules/app/views/shared/_section.html.erb | 2 +- schedules/config/routes.rb | 13 +- 21 files changed, 152 insertions(+), 226 deletions(-) delete mode 100644 schedules/app/views/course_sections/index.html.erb create mode 100644 schedules/app/views/schedules/sections.html.erb diff --git a/schedules/app/assets/stylesheets/application.scss b/schedules/app/assets/stylesheets/application.scss index e76b631..ddf371a 100644 --- a/schedules/app/assets/stylesheets/application.scss +++ b/schedules/app/assets/stylesheets/application.scss @@ -159,6 +159,16 @@ nav { text-overflow: ellipsis; } + .prereqs { + display: block; + margin-top: 1em; + line-height: 1.7em; + font-family: 'Roboto', sans-serif; + color: $gray; + font-size: 0.9em; + max-height: 200px; + } + .see-more { display: block; text-align: center; diff --git a/schedules/app/controllers/api/schedules_controller.rb b/schedules/app/controllers/api/schedules_controller.rb index df93ee3..961c0ce 100644 --- a/schedules/app/controllers/api/schedules_controller.rb +++ b/schedules/app/controllers/api/schedules_controller.rb @@ -5,7 +5,7 @@ class API::SchedulesController < ApplicationController # course sections with the given CRNs. def index crns = params["crns"].split ',' - @schedule = Schedule.new(crns, @semester.season) - render plain: @schedule.to_ical # render a plaintext iCal file + schedule = Schedule.new(crns, @semester.season) + render plain: schedule.to_ical # render a plaintext iCal file end end diff --git a/schedules/app/controllers/course_sections_controller.rb b/schedules/app/controllers/course_sections_controller.rb index 98b95b2..6e01c3e 100644 --- a/schedules/app/controllers/course_sections_controller.rb +++ b/schedules/app/controllers/course_sections_controller.rb @@ -1,36 +1,4 @@ class CourseSectionsController < ApplicationController - # index renders the view that is asyncronously fetched for the /schedule page - def index - crns = params[:crns].split(',') - @sections = crns.map { |crn| CourseSection.latest_by_crn(crn) } - @days = { - "M" => [], "T" => [], "W" => [], - "R" => [], "F" => [], "Online" => [] - } - - @sections.each do |s| - days = s.days.gsub(/[^a-zA-Z]/, "") # get rid of weird   character - @days["Online"] << s if days.empty? - days.split('').each do |day| - @days[day] << s unless s.start_time == "TBA" - end - end - - @days_map = { - "M" => "Monday", "T" => "Tuesday", "W" => "Wednesday", - "R" => "Thursday", "F" => "Friday", "Online" => "Online" - } - - @days.each do |day, sections| - sections.sort! do |a,b| - Time.new(a.start_time) <=> Time.new(b.start_time) - end - end - - # Don't render the normal HTML head, just render what's in the view file - render(layout: false) - end - def show @section = CourseSection.find(params[:id]) end diff --git a/schedules/app/controllers/schedules_controller.rb b/schedules/app/controllers/schedules_controller.rb index 03b4bc6..8b84561 100644 --- a/schedules/app/controllers/schedules_controller.rb +++ b/schedules/app/controllers/schedules_controller.rb @@ -1,4 +1,36 @@ class SchedulesController < ApplicationController - def index; end - def show; end + # index renders the /schedule page, which then asynchronously + # requests #sections to get the list of sections currently in the cart. + def index; end + + def sections + crns = params[:crns].split(',') + @sections = crns.map { |crn| CourseSection.latest_by_crn(crn) } + @days = { + "M" => [], "T" => [], "W" => [], + "R" => [], "F" => [], "Online" => [] + } + + @sections.each do |s| + days = s.days.gsub(/[^a-zA-Z]/, "") # get rid of weird   character + @days["Online"] << s if days.empty? + days.split('').each do |day| + @days[day] << s unless s.start_time == "TBA" + end + end + + @days_map = { + "M" => "Monday", "T" => "Tuesday", "W" => "Wednesday", + "R" => "Thursday", "F" => "Friday", "Online" => "Online" + } + + @days.each do |day, sections| + sections.sort! do |a,b| + Time.new(a.start_time) <=> Time.new(b.start_time) + end + end + + # Don't render the normal HTML head, just render what's in the view file + render(layout: false) + end end diff --git a/schedules/app/controllers/search_controller.rb b/schedules/app/controllers/search_controller.rb index 44fd7d8..b4df0f1 100644 --- a/schedules/app/controllers/search_controller.rb +++ b/schedules/app/controllers/search_controller.rb @@ -28,10 +28,6 @@ def index @courses = Course.where("(courses.title LIKE ?) OR (courses.description LIKE ?)", query, query).uniq @instructors = Instructor.named(params[:query]) end - - @courses.map! do |c| - c.serializable_hash.merge(url: course_url(c)) - end end /[0-9]{5}/.match(params[:query]) do |m| @@ -39,7 +35,7 @@ def index end if @courses&.count == 1 && @instructors&.count&.zero? - redirect_to(course_url(@courses.first["id"])) + redirect_to(course_url(@courses.first)) elsif @courses&.count&.zero? && @instructors&.count == 1 redirect_to(instructor_url(@instructors.first)) end diff --git a/schedules/app/helpers/schedules_helper.rb b/schedules/app/helpers/schedules_helper.rb index adb10ee..86e05a4 100644 --- a/schedules/app/helpers/schedules_helper.rb +++ b/schedules/app/helpers/schedules_helper.rb @@ -1,29 +1,2 @@ module SchedulesHelper - DAYS = { - "M": Date.new(2019, 1, 14), - "T": Date.new(2019, 1, 15), - "W": Date.new(2019, 1, 16), - "R": Date.new(2019, 1, 17), - "F": Date.new(2019, 1, 18), - "S": Date.new(2019, 1, 19), - "U": Date.new(2019, 1, 20) - }.freeze - - def generate_fullcalender_events(sections) - sections.map do |s| - s.days.split('').map do |day| - formatted_date = DAYS[day.to_sym].to_s.tr('-', '') - time = Time.parse(s.start_time).strftime("%H%M%S") - endtime = Time.parse(s.end_time).strftime("%H%M%S") - - { - title: s.name, - start: "#{formatted_date}T#{time}", - end: "#{formatted_date}T#{endtime}", - crn: s.crn, - active: true - } - end - end.flatten - end end diff --git a/schedules/app/javascript/src/controllers/schedule_controller.js b/schedules/app/javascript/src/controllers/schedule_controller.js index 12d49fe..be71e27 100644 --- a/schedules/app/javascript/src/controllers/schedule_controller.js +++ b/schedules/app/javascript/src/controllers/schedule_controller.js @@ -15,7 +15,7 @@ export default class extends Controller { this.loaderTarget.classList.remove('hidden') this.scheduleTarget.innerHTML = '' - fetch(`/course_sections?crns=${getCart().join(',')}`) + fetch(`/schedule/sections?crns=${getCart().join(',')}`) .then(resp => resp.text()) .then(text => { this.scheduleTarget.innerHTML = text diff --git a/schedules/app/models/course.rb b/schedules/app/models/course.rb index eae7426..c1e7415 100644 --- a/schedules/app/models/course.rb +++ b/schedules/app/models/course.rb @@ -19,6 +19,6 @@ def rating(question = 1, sections = course_sections) total += s.rating_questions[question]["instr_mean"].to_f * s.rating_questions[0]["resp"].to_i end - [(total / resp).round(2), resp] unless resp.zero? + Rating.new((total / resp).round(2), resp) unless resp.zero? end end diff --git a/schedules/app/models/course_section.rb b/schedules/app/models/course_section.rb index b3ae1ed..69a5a4f 100644 --- a/schedules/app/models/course_section.rb +++ b/schedules/app/models/course_section.rb @@ -13,10 +13,10 @@ class CourseSection < ApplicationRecord validates :course_id, presence: true validates :semester_id, presence: true - serialize :rating_questions, Array - scope :in_semester, ->(semester) { where(semester: semester) } + serialize :rating_questions, Array + def teaching_rating if rating_questions.empty? nil @@ -45,12 +45,14 @@ def self.with_instructor(name: "") .select('course_sections.*, instructors.name as instructor_name') end + # Sections are uniquely identified by their: CRN , Course, and Semester, + # but they have other required attributes too, so we can't use find_or_create_by. def self.find_or_update_by!(s) base_attrs = { crn: s[:crn], course: s[:course], semester: s[:semester] } sections = CourseSection.where(base_attrs) if sections.empty? section = CourseSection.new(base_attrs) - else + else # multiple sections with same attributes, destroy them sections = sections.to_a section = sections.shift sections.each do |bad_section| diff --git a/schedules/app/models/instructor.rb b/schedules/app/models/instructor.rb index fd9e172..ea15f68 100644 --- a/schedules/app/models/instructor.rb +++ b/schedules/app/models/instructor.rb @@ -28,6 +28,6 @@ def rating(question) total += s.rating_questions[question_i]["instr_mean"].to_f * s.rating_questions[0]["resp"].to_i end - [(total / resp).round(2), resp] unless resp.zero? + Rating.new((total / resp).round(2), resp) unless resp.zero? end end diff --git a/schedules/app/models/semester.rb b/schedules/app/models/semester.rb index 46832a1..02b7c72 100644 --- a/schedules/app/models/semester.rb +++ b/schedules/app/models/semester.rb @@ -15,8 +15,8 @@ def to_s # Sorts semesters in descending temporal order. # i.e. Fall 2020, Summer 2020, Spring 2020, Fall 2019, ... - def self.sorted_by_date(sems = Semester.all) - sems.sort do |s1, s2| + def self.sorted_by_date(semesters = Semester.all) + semesters.sort do |s1, s2| if s2.year != s1.year s2.year <=> s1.year else diff --git a/schedules/app/views/course_sections/index.html.erb b/schedules/app/views/course_sections/index.html.erb deleted file mode 100644 index a1f49fb..0000000 --- a/schedules/app/views/course_sections/index.html.erb +++ /dev/null @@ -1,7 +0,0 @@ -<% @days.each do |day, sections| %> - <% next if sections.empty? %> -

<%= @days_map[day] %>

-
    - <%= render(partial: 'shared/section', collection: sections) %> -
-<% end %> \ No newline at end of file diff --git a/schedules/app/views/courses/show.html.erb b/schedules/app/views/courses/show.html.erb index 8ea42b3..da67964 100644 --- a/schedules/app/views/courses/show.html.erb +++ b/schedules/app/views/courses/show.html.erb @@ -5,8 +5,8 @@

<%= @course.full_name %>

<%= @course.title %>

<% unless @rating.nil? %> - <%= render partial: 'shared/stars', locals: { percent: @course.rating[0]/5*100 } %> - Average course rating: <%= @course.rating[0] %> / <%= @course.rating[1] %> responses + <%= render partial: 'shared/stars', locals: { percent: @course.rating.avg/5*100 } %> + Average course rating: <%= @course.rating.avg %> / <%= @course.rating.responses %> responses <% end %>
diff --git a/schedules/app/views/instructors/show.html.erb b/schedules/app/views/instructors/show.html.erb index a5c7fb6..7c10c13 100644 --- a/schedules/app/views/instructors/show.html.erb +++ b/schedules/app/views/instructors/show.html.erb @@ -5,8 +5,8 @@

<%= @instructor.name %>

<% unless @rating.nil? %> - <%= render(partial: 'shared/stars', locals: { percent: @rating[0]/5*100 }) %> - <%= @rating[0] %> / <%= @rating[1] %> responses + <%= render(partial: 'shared/stars', locals: { percent: @rating.avg/5*100 }) %> + <%= @rating.avg %> / <%= @rating.responses %> responses <% end %>
diff --git a/schedules/app/views/layouts/application.html.erb b/schedules/app/views/layouts/application.html.erb index 476bd62..0f3aea6 100644 --- a/schedules/app/views/layouts/application.html.erb +++ b/schedules/app/views/layouts/application.html.erb @@ -1,62 +1,62 @@ - - Schedules - <%= csrf_meta_tags %> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - <%= favicon_link_tag %> - - - - <%= javascript_pack_tag('application') %> - <%= stylesheet_link_tag('application') %> - - - - <%= render 'shared/page' do %> - <%= yield %> - <% end %> - - - - + + Schedules + <%= csrf_meta_tags %> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + <%= favicon_link_tag %> + + + + <%= javascript_pack_tag('application') %> + <%= stylesheet_link_tag('application') %> + + + + <%= render 'shared/page' do %> + <%= yield %> + <% end %> + + + + diff --git a/schedules/app/views/schedules/sections.html.erb b/schedules/app/views/schedules/sections.html.erb new file mode 100644 index 0000000..0860efd --- /dev/null +++ b/schedules/app/views/schedules/sections.html.erb @@ -0,0 +1,7 @@ +<% @days.each do |day, sections| %> + <% next if sections.empty? %> +

<%= @days_map[day] %>

+
    + <%= render(partial: 'shared/section', collection: sections) %> +
+<% end %> diff --git a/schedules/app/views/search/index.html.erb b/schedules/app/views/search/index.html.erb index bd328ca..7e5fbe2 100644 --- a/schedules/app/views/search/index.html.erb +++ b/schedules/app/views/search/index.html.erb @@ -1,21 +1,8 @@ <%= render(partial: 'shared/navbar') %>
- <% (@instructors || []).each do |inst| %> -
-
<%= inst.name %>
-
- <% end %> - - <% (@courses || []).each do |course| %> -
-

<%= course['subject'] %> <%= course['course_number'] %>

-
<%= course['title'] %>. <%= course['credits'] %> <%= 'credit'.pluralize(course['credits'].to_i) %>.
- - <%= course['description'] %> - View Sections -
- <% end %> + <%= render partial: 'shared/instructor', collection: @instructors %> + <%= render partial: 'shared/course', collection: @courses %> <% if @courses&.empty? && @instructors&.empty? %> No results found. Please try again with another search term. diff --git a/schedules/app/views/shared/_course.html.erb b/schedules/app/views/shared/_course.html.erb index 67ae8d7..9238dc2 100644 --- a/schedules/app/views/shared/_course.html.erb +++ b/schedules/app/views/shared/_course.html.erb @@ -1,45 +1,15 @@ -
-
-
-
- <%= link_to course do %> -

<%= "#{course.subject} #{course.course_number}" %>

- <% end %> -
-
-
-
<%= course.title %>
-
-
-
- -
- <%= course.credits %> credits -
-     -
-
-
-
- -

<%= course.description %>

- - <% unless course.prereqs.nil? || course.prereqs.empty? %> - <% first, rest = course.prereqs.split(':') %> - <% prereqs, note = rest.split('.') %> -

<%= first %>: <%= prereqs %> <%= note %>

- <% end %> +
+

<%= link_to("#{course.subject} #{course.course_number}", course)%>

- <% if expanded %> -
-

Minimize

- -
- <% else %> -
-

Expand

- -
- <% end %> -
-
+
<%= course.title %>. <%= course.credits %> <%= 'credit'.pluralize(course.credits.to_i) %>.
+ + <%= course.description %> + + <% unless course.prereqs.nil? || course.prereqs.empty? %> + <% first, rest = course.prereqs.split(':') %> + <% prereqs, note = rest.split('.') %> + <%= first %>: <%= prereqs %> <%= note %> + <% end %> + + <%= link_to("View Sections", course, class: "see-more") %> + diff --git a/schedules/app/views/shared/_instructor.html.erb b/schedules/app/views/shared/_instructor.html.erb index 15d44b6..c3cfa5d 100644 --- a/schedules/app/views/shared/_instructor.html.erb +++ b/schedules/app/views/shared/_instructor.html.erb @@ -1,12 +1,3 @@ -
-
-
-
-
- -
- <%= link_to instructor.name, instructor_path(instructor) %> -
-
-
-
+
+
<%= link_to(instructor.name, instructor) %>
+
diff --git a/schedules/app/views/shared/_section.html.erb b/schedules/app/views/shared/_section.html.erb index e3bc128..aace018 100644 --- a/schedules/app/views/shared/_section.html.erb +++ b/schedules/app/views/shared/_section.html.erb @@ -16,7 +16,7 @@ So if that variable is defined, don't show the stars. %> <% unless section.instructor.rating(:teaching).nil? || local_assigns.has_key?(:on_instructor_page)%> - <%= render partial: 'shared/stars', locals: { percent: (section.instructor.rating(:teaching)[0] / 5 * 100).to_i }%> + <%= render partial: 'shared/stars', locals: { percent: (section.instructor.rating(:teaching).avg/ 5 * 100).to_i }%> <% end %> <% end %>
diff --git a/schedules/config/routes.rb b/schedules/config/routes.rb index 706391c..348f1bd 100644 --- a/schedules/config/routes.rb +++ b/schedules/config/routes.rb @@ -14,23 +14,20 @@ # GET schedules.gmu.edu/search routes to AboutController#index get 'about', to: 'about#index', as: 'about' + # GET schedules.gmu.edu/schedule routes to SchedulesController#show + get 'schedule', to: 'schedules#index', as: 'schedule' + get 'schedule/sections', to: 'schedules#sections' + # GET schedules.gmu.edu/courses/:id routes to CoursesController#show resources :courses, only: [:show] # GET schedules.gmu.edu/courses_sections/:id routes to CourseSectionsController#show # - this is for viewing the ratings for that section - # - # GET schedules.gmu.edu/course_sections routes to CourseSectionsController#index - # - this expects a list of crns as a query paramater - # - used to render the schedule for the /schedule page - resources :course_sections, only: [:index, :show] + resources :course_sections, only: [:show] # GET schedules.gmu.edu/instructors/:id routes to InstructorsController#show resources :instructors, only: [:show] - # GET schedules.gmu.edu/schedule routes to SchedulesController#show - get 'schedule', to: 'schedules#index', as: 'schedule' - # Register /api routes scope :api, module: 'api' do # Renders the iCal for calendar subscriptions