From 5eb89a44795608792a800a620433f202225b04ef Mon Sep 17 00:00:00 2001 From: David Liu Date: Wed, 27 May 2015 12:19:45 -0400 Subject: [PATCH 1/3] Fix error related to lazy loading and STI. Also fixes the error handling in the assignment update action. --- app/controllers/assignments_controller.rb | 27 +++++++++---------- app/models/submission_rule.rb | 10 ++++++- .../_assignments_dropdown_menu.html.erb | 2 ++ config/locales/en.yml | 2 +- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb index c159071212..fd6ef0725d 100644 --- a/app/controllers/assignments_controller.rb +++ b/app/controllers/assignments_controller.rb @@ -1,11 +1,6 @@ -# We need to force loading of all submission rules so that methods -# like .const_defined? work correctly (rails abuse of autoload was -# causing issues) -Dir.glob('app/models/*_submission_rule.rb').each do |rule| - require File.expand_path(rule) -end - class AssignmentsController < ApplicationController + require_dependency 'submission_rule' + before_filter :authorize_only_for_admin, except: [:deletegroup, :delete_rejected, @@ -238,10 +233,9 @@ def update @assignment = process_assignment_form(@assignment) end rescue SubmissionRule::InvalidRuleType => e - @assignment.errors.add(:base, I18n.t('assignment.error', - message: e.message)) - render :edit, id: @assignment.id - return + @assignment.errors.add(:base, t('assignment.error', message: e.message)) + flash[:error] = t('assignment.error', message: e.message) + render :edit, id: @assignment.id and return end if @assignment.save @@ -639,11 +633,14 @@ def process_assignment_form(assignment) # First, figure out what kind of rule has been requested rule_attributes = params[:assignment][:submission_rule_attributes] rule_name = rule_attributes[:type] - potential_rule = if SubmissionRule.const_defined?(rule_name) - SubmissionRule.const_get(rule_name) - end - unless potential_rule && potential_rule.ancestors.include?(SubmissionRule) + # Force the classes to be loaded so that the following check will + # succeed on these inputs. + [NoLateSubmissionRule, PenaltyPeriodSubmissionRule, + PenaltyDecayPeriodSubmissionRule, GracePeriodSubmissionRule] + if SubmissionRule.const_defined?(rule_name) + potential_rule = SubmissionRule.const_get(rule_name) + else raise SubmissionRule::InvalidRuleType, rule_name end diff --git a/app/models/submission_rule.rb b/app/models/submission_rule.rb index e9c78e2df7..c3a6db7220 100644 --- a/app/models/submission_rule.rb +++ b/app/models/submission_rule.rb @@ -2,17 +2,25 @@ class SubmissionRule < ActiveRecord::Base class InvalidRuleType < Exception def initialize(rule_name) - super I18n.t('assignment.not_valid_submission_rule', rule_name) + super I18n.t('assignment.not_valid_submission_rule', type: rule_name) end end belongs_to :assignment has_many :periods, dependent: :destroy, order: 'id' accepts_nested_attributes_for :periods, allow_destroy: true + attr_accessible :type # validates_associated :assignment # validates_presence_of :assignment + def self.descendants + [ NoLateSubmissionRule, + PenaltyPeriodSubmissionRule, + PenaltyDecayPeriodSubmissionRule, + GracePeriodSubmissionRule ] + end + def can_collect_now? return @can_collect_now if !@can_collect_now.nil? @can_collect_now = Time.zone.now >= get_collection_time diff --git a/app/views/shared/_assignments_dropdown_menu.html.erb b/app/views/shared/_assignments_dropdown_menu.html.erb index e16fe28c4d..a0c7fb1f22 100644 --- a/app/views/shared/_assignments_dropdown_menu.html.erb +++ b/app/views/shared/_assignments_dropdown_menu.html.erb @@ -87,6 +87,8 @@ # menu for assignments should go to browse, and not repo_browser if target_action == 'repo_browser' target_action = 'browse' + elsif target_action == 'update' + target_action = 'edit' end %> <% # Don't show hidden assignments in the dropdown menu for students # either. Append '(hidden)' for TAs and Admins. diff --git a/config/locales/en.yml b/config/locales/en.yml index 0c52ea53a3..36f38bb162 100755 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -147,7 +147,7 @@ en: short_identifier: "Short Identifier*" update_success: "Successfully updated the assignment" create_success: "Successfully created the assignment" - error: "Could not assign SubmissionRule: #%{message}" + error: "Could not assign SubmissionRule: %{message}" not_valid_submission_rule: "%{type} is not a valid SubmissionRule" rules: "Assignment Rules" edit: "Edit Assignment:" From 9618347a5fea27e6f6e67f57c9402ac9f73a0d0a Mon Sep 17 00:00:00 2001 From: David Liu Date: Thu, 28 May 2015 10:15:54 -0400 Subject: [PATCH 2/3] submission-rule-STI: change default to NoLateSubmissionRule --- ...0527172828_change_submission_rule_column_default.rb | 9 +++++++++ db/schema.rb | 10 +++++----- 2 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20150527172828_change_submission_rule_column_default.rb diff --git a/db/migrate/20150527172828_change_submission_rule_column_default.rb b/db/migrate/20150527172828_change_submission_rule_column_default.rb new file mode 100644 index 0000000000..58b70812df --- /dev/null +++ b/db/migrate/20150527172828_change_submission_rule_column_default.rb @@ -0,0 +1,9 @@ +class ChangeSubmissionRuleColumnDefault < ActiveRecord::Migration + def up + change_column_default :submission_rules, :type, 'NoLateSubmissionRule' + end + + def down + change_column_default :submission_rules, :type, 'NullSubmissionRule' + end +end diff --git a/db/schema.rb b/db/schema.rb index 6cd950b231..a3dc9824e4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20150326163940) do +ActiveRecord::Schema.define(:version => 20150527172828) do create_table "annotation_categories", :force => true do |t| t.text "annotation_category_name" @@ -372,10 +372,10 @@ add_index "submission_files", ["submission_id"], :name => "index_submission_files_on_submission_id" create_table "submission_rules", :force => true do |t| - t.integer "assignment_id", :null => false - t.string "type", :default => "NullSubmissionRule" - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false + t.integer "assignment_id", :null => false + t.string "type", :default => "NoLateSubmissionRule" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end add_index "submission_rules", ["assignment_id"], :name => "index_submission_rules_on_assignment_id" From 7193c4c35adf64e34a9a0e2203e3e4ca8b485f2a Mon Sep 17 00:00:00 2001 From: David Liu Date: Thu, 28 May 2015 10:35:45 -0400 Subject: [PATCH 3/3] submission-rule-STI: hound fixes --- app/controllers/assignments_controller.rb | 3 ++- app/models/submission_rule.rb | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb index fd6ef0725d..cfc26f3067 100644 --- a/app/controllers/assignments_controller.rb +++ b/app/controllers/assignments_controller.rb @@ -235,7 +235,8 @@ def update rescue SubmissionRule::InvalidRuleType => e @assignment.errors.add(:base, t('assignment.error', message: e.message)) flash[:error] = t('assignment.error', message: e.message) - render :edit, id: @assignment.id and return + render :edit, id: @assignment.id + return end if @assignment.save diff --git a/app/models/submission_rule.rb b/app/models/submission_rule.rb index c3a6db7220..e7afe00dd6 100644 --- a/app/models/submission_rule.rb +++ b/app/models/submission_rule.rb @@ -15,10 +15,10 @@ def initialize(rule_name) # validates_presence_of :assignment def self.descendants - [ NoLateSubmissionRule, - PenaltyPeriodSubmissionRule, - PenaltyDecayPeriodSubmissionRule, - GracePeriodSubmissionRule ] + [NoLateSubmissionRule, + PenaltyPeriodSubmissionRule, + PenaltyDecayPeriodSubmissionRule, + GracePeriodSubmissionRule] end def can_collect_now?