From 440290c08fe665968c27fcae9b7ed7fde230f573 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Fri, 4 Dec 2015 15:59:07 -0700 Subject: [PATCH] remove active_polymorph fixes CNVS-25580 Change-Id: Id1a001e8b85f87d1c0e9678500ed13d435872b55 Reviewed-on: https://gerrit.instructure.com/68292 Tested-by: Jenkins Reviewed-by: Rob Orton Product-Review: Cody Cutrer QA-Review: Cody Cutrer --- Gemfile.d/app.rb | 1 - app/models/quizzes/quiz.rb | 4 - app/models/quizzes/quiz_submission.rb | 4 - config/initializers/active_record.rb | 86 ++++++++++ ...224305_fix_deprecated_polymorphic_names.rb | 20 +++ gems/active_polymorph/Gemfile | 10 -- gems/active_polymorph/Rakefile | 2 - .../active_polymorph/active_polymorph.gemspec | 26 --- gems/active_polymorph/lib/active_polymorph.rb | 145 ----------------- .../spec/active_polymorph_spec.rb | 152 ------------------ gems/active_polymorph/test.sh | 15 -- spec/lib/user_merge_spec.rb | 1 - spec/models/quizzes/quiz_spec.rb | 33 ---- spec/models/quizzes/quiz_submission_spec.rb | 45 ------ 14 files changed, 106 insertions(+), 438 deletions(-) create mode 100644 db/migrate/20151204224305_fix_deprecated_polymorphic_names.rb delete mode 100644 gems/active_polymorph/Gemfile delete mode 100644 gems/active_polymorph/Rakefile delete mode 100644 gems/active_polymorph/active_polymorph.gemspec delete mode 100644 gems/active_polymorph/lib/active_polymorph.rb delete mode 100644 gems/active_polymorph/spec/active_polymorph_spec.rb delete mode 100755 gems/active_polymorph/test.sh diff --git a/Gemfile.d/app.rb b/Gemfile.d/app.rb index e1eb5456fb2..7c1a8c22f07 100644 --- a/Gemfile.d/app.rb +++ b/Gemfile.d/app.rb @@ -101,7 +101,6 @@ gem 'sentry-raven', '0.13.2', require: false gem 'canvas_statsd', '1.0.3' gem 'diplomat', '0.14.0', require: false -gem 'active_polymorph', path: 'gems/active_polymorph' gem 'activesupport-suspend_callbacks', path: 'gems/activesupport-suspend_callbacks' gem 'acts_as_list', path: 'gems/acts_as_list' gem 'adheres_to_policy', path: 'gems/adheres_to_policy' diff --git a/app/models/quizzes/quiz.rb b/app/models/quizzes/quiz.rb index 33a217d30ab..123768fe051 100644 --- a/app/models/quizzes/quiz.rb +++ b/app/models/quizzes/quiz.rb @@ -54,10 +54,6 @@ class Quizzes::Quiz < ActiveRecord::Base belongs_to :assignment belongs_to :assignment_group - def self.polymorphic_names - [self.base_class.name, "Quiz"] - end - EXPORTABLE_ATTRIBUTES = [ :id, :title, :description, :quiz_data, :points_possible, :context_id, :context_type, :assignment_id, :workflow_state, :shuffle_answers, diff --git a/app/models/quizzes/quiz_submission.rb b/app/models/quizzes/quiz_submission.rb index d88edca0ef2..3606de5f48f 100644 --- a/app/models/quizzes/quiz_submission.rb +++ b/app/models/quizzes/quiz_submission.rb @@ -21,10 +21,6 @@ require 'sanitize' class Quizzes::QuizSubmission < ActiveRecord::Base self.table_name = 'quiz_submissions' - def self.polymorphic_names - [self.name, "QuizSubmission"] - end - include Workflow attr_accessible :quiz, :user, :temporary_user_code, :submission_data, :score_before_regrade, :has_seen_results diff --git a/config/initializers/active_record.rb b/config/initializers/active_record.rb index f2d25f265dd..af35a4b3bd7 100644 --- a/config/initializers/active_record.rb +++ b/config/initializers/active_record.rb @@ -640,6 +640,92 @@ if CANVAS_RAILS3 super end end + + # copy/paste from Rails 3, with addition of applying the scope_chain (copy/paste from Rails 4) + def add_constraints(scope) + tables = construct_tables + + chain.each_with_index do |reflection, i| + table, foreign_table = tables.shift, tables.first + + if reflection.source_macro == :has_and_belongs_to_many + join_table = tables.shift + + scope = scope.joins(join( + join_table, + table[reflection.association_primary_key]. + eq(join_table[reflection.association_foreign_key]) + )) + + table, foreign_table = join_table, tables.first + end + + if reflection.source_macro == :belongs_to + if reflection.options[:polymorphic] + key = reflection.association_primary_key(klass) + else + key = reflection.association_primary_key + end + + foreign_key = reflection.foreign_key + else + key = reflection.foreign_key + foreign_key = reflection.active_record_primary_key + end + + conditions = self.conditions[i] + + if reflection == chain.last + scope = scope.where(table[key].eq(owner[foreign_key])) + + if reflection.type + scope = scope.where(table[reflection.type].eq(owner.class.base_class.name)) + end + + conditions.each do |condition| + if options[:through] && condition.is_a?(Hash) + condition = disambiguate_condition(table, condition) + end + + scope = scope.where(interpolate(condition)) + end + else + constraint = table[key].eq(foreign_table[foreign_key]) + + if reflection.type + type = chain[i + 1].klass.base_class.name + constraint = constraint.and(table[reflection.type].eq(type)) + end + + scope = scope.joins(join(foreign_table, constraint)) + + unless conditions.empty? + scope = scope.where(sanitize(conditions, table)) + end + end + + # rails 4 part + is_first_chain = i == 0 + klass = is_first_chain ? self.klass : reflection.klass + + self.reflection.scope_chain[i].each do |scope_chain_item| + item = klass.unscoped.instance_exec(&scope_chain_item) + + if scope_chain_item == self.reflection.scope + scope = scope.merge(item.except(:where, :includes)) + end + + if is_first_chain + scope = scope.includes(*item.includes_values) + end + + scope.where_values += item.where_values + scope.order_values |= item.order_values + end + end + + scope + end end ActiveRecord::Associations::AssociationScope.prepend(RelationForAssociationScope) diff --git a/db/migrate/20151204224305_fix_deprecated_polymorphic_names.rb b/db/migrate/20151204224305_fix_deprecated_polymorphic_names.rb new file mode 100644 index 00000000000..4b57fd8a589 --- /dev/null +++ b/db/migrate/20151204224305_fix_deprecated_polymorphic_names.rb @@ -0,0 +1,20 @@ +class FixDeprecatedPolymorphicNames < ActiveRecord::Migration + tag :predeploy + disable_ddl_transaction! + + def up + reflections = [Quizzes::Quiz, Quizzes::QuizSubmission].map do |klass| + klass.reflections.values.select { |r| r.macro == :has_many && r.options[:as] } + end + reflections.flatten! + reflections.group_by(&:klass).each do |(klass, klass_reflections)| + klass.find_ids_in_ranges(batch_size: 10000) do |min_id, max_id| + klass_reflections.each do |reflection| + klass.where(id: min_id..max_id, + reflection.type => reflection.active_record.name.sub("Quizzes::", "")). + update_all(reflection.type => reflection.active_record.name) + end + end + end + end +end diff --git a/gems/active_polymorph/Gemfile b/gems/active_polymorph/Gemfile deleted file mode 100644 index 410fe47149d..00000000000 --- a/gems/active_polymorph/Gemfile +++ /dev/null @@ -1,10 +0,0 @@ -source 'https://rubygems.org' -gemspec - -require File.expand_path("../../../config/canvas_rails4", __FILE__) - -if CANVAS_RAILS3 - gem 'activerecord', '~> 3.2.0' -else - gem 'activerecord', '~> 4.0.0' -end diff --git a/gems/active_polymorph/Rakefile b/gems/active_polymorph/Rakefile deleted file mode 100644 index 809eb5616ad..00000000000 --- a/gems/active_polymorph/Rakefile +++ /dev/null @@ -1,2 +0,0 @@ -require "bundler/gem_tasks" - diff --git a/gems/active_polymorph/active_polymorph.gemspec b/gems/active_polymorph/active_polymorph.gemspec deleted file mode 100644 index 09f984f4ddf..00000000000 --- a/gems/active_polymorph/active_polymorph.gemspec +++ /dev/null @@ -1,26 +0,0 @@ -# coding: utf-8 -lib = File.expand_path('../lib', __FILE__) -$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) - -Gem::Specification.new do |spec| - spec.name = "active_polymorph" - spec.version = "0.0.1" - spec.authors = ["Anthus Williams"] - spec.email = ["aj@instructure.com"] - spec.summary = %q{Support for polymorphic names in ActiveRecord has_many associations} - spec.homepage = "https://github.com/instructure/canvas-lms" - spec.license = "AGPL" - - spec.files = Dir.glob("{lib,spec}/**/*") + %w(Rakefile test.sh) - spec.executables = spec.files.grep(%r{^bin/}) { |f| File.basename(f) } - spec.test_files = spec.files.grep(%r{^(test|spec|features)/}) - spec.require_paths = ["lib"] - - spec.add_dependency "activerecord", ">= 3.2", "< 4.1" - - spec.add_development_dependency "bundler", "~> 1.5" - spec.add_development_dependency "rake" - spec.add_development_dependency "rspec", "2.99.0" - spec.add_development_dependency "sqlite3" -end - diff --git a/gems/active_polymorph/lib/active_polymorph.rb b/gems/active_polymorph/lib/active_polymorph.rb deleted file mode 100644 index bb10f51c0a4..00000000000 --- a/gems/active_polymorph/lib/active_polymorph.rb +++ /dev/null @@ -1,145 +0,0 @@ -require 'active_record' - -class ActiveRecord::Base - def self.polymorphic_names - [self.base_class.name] - end -end - -module ActiveRecord - class Relation - # this method prevents ActiveRecord from attempting to constantize the polymorphic_names array as part of the build_associations process - def scope_for_create_with_polymorphic_names - scope = scope_for_create_without_polymorphic_names - if default_scoped? - reflect_on_polymorphic_associations.each do |assoc| - # since all class names in are returned in select queries (via self.polymorphic_names) it is ok to just take the first element of the array (rather than trying to constantize the array itself) - # still, in future it would be better to devise some way of declaring a canonical form for the create scope - # (e.g. select queries should retrieve all instances of PreRefactoredClassName and PostRefactoredClassName, but insert queries should always prefer the latter) - scope[assoc.foreign_type] = scope[assoc.foreign_type].first if scope[assoc.foreign_type].is_a?(Array) - end - end - scope - end - alias_method_chain :scope_for_create, :polymorphic_names - - def reflect_on_polymorphic_associations - reflect_on_all_associations.select{ |assoc| assoc.options[:polymorphic] } - end - end - - class Associations::AssociationScope - def add_constraints(scope) - tables = construct_tables - - chain.each_with_index do |reflection, i| - table, foreign_table = tables.shift, tables.first - - if reflection.source_macro == :has_and_belongs_to_many - join_table = tables.shift - - scope = scope.joins(join( - join_table, - table[reflection.association_primary_key]. - eq(join_table[reflection.association_foreign_key]) - )) - - table, foreign_table = join_table, tables.first - end - - if reflection.source_macro == :belongs_to - if reflection.options[:polymorphic] - key = reflection.association_primary_key(klass) - else - key = reflection.association_primary_key - end - - foreign_key = reflection.foreign_key - else - key = reflection.foreign_key - foreign_key = reflection.active_record_primary_key - end - - conditions = CANVAS_RAILS3 ? self.conditions[i] : [] - - if reflection == chain.last - bind_val = CANVAS_RAILS3 ? owner[foreign_key] : - bind(scope, table.table_name, key.to_s, owner[foreign_key]) - scope = scope.where(table[key].eq(bind_val)) - - if reflection.type - types = owner.class.polymorphic_names - types = types.first if types.length == 1 - scope = scope.where(table.name => {table[reflection.type].name => types}) - end - - conditions.each do |condition| - if options[:through] && condition.is_a?(Hash) - condition = disambiguate_condition(table, condition) - end - - scope = scope.where(interpolate(condition)) - end - - else - constraint = table[key].eq(foreign_table[foreign_key]) - - if reflection.type - types = chain[i + 1].klass.polymorphic_names - types = types.first if types.length == 1 - constraint = constraint.and(table.name => {table[reflection.type].name => types}) - end - - scope = scope.joins(join(foreign_table, constraint)) - - unless conditions.empty? - scope = scope.where(sanitize(conditions, table)) - end - end - - if !CANVAS_RAILS3 - is_first_chain = i == 0 - klass = is_first_chain ? self.klass : reflection.klass - - # Exclude the scope of the association itself, because that - # was already merged in the #scope method. - self.reflection.scope_chain[i].each do |scope_chain_item| - item = eval_scope(klass, scope_chain_item) - - if scope_chain_item == self.reflection.scope - scope.merge! item.except(:where, :includes) - end - - if is_first_chain - scope.includes! item.includes_values - end - - scope.where_values += item.where_values - scope.order_values |= item.order_values - end - elsif self.reflection.respond_to?(:scope_chain) - # rails 3 supporting rails 4 style scope on associations - is_first_chain = i == 0 - klass = is_first_chain ? self.klass : reflection.klass - - self.reflection.scope_chain[i].each do |scope_chain_item| - item = klass.unscoped.instance_exec(&scope_chain_item) - - if scope_chain_item == self.reflection.scope - scope = scope.merge(item.except(:where, :includes)) - end - - if is_first_chain - scope = scope.includes(*item.includes_values) - end - - scope.where_values += item.where_values - scope.order_values |= item.order_values - end - end - end - - scope - end - end -end diff --git a/gems/active_polymorph/spec/active_polymorph_spec.rb b/gems/active_polymorph/spec/active_polymorph_spec.rb deleted file mode 100644 index 901ebd7c0e9..00000000000 --- a/gems/active_polymorph/spec/active_polymorph_spec.rb +++ /dev/null @@ -1,152 +0,0 @@ -# -# Copyright (C) 2014 Instructure, Inc. -# -# This file is part of Canvas. -# -# Canvas is free software: you can redistribute it and/or modify it under -# the terms of the GNU Affero General Public License as published by the Free -# Software Foundation, version 3 of the License. -# -# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY -# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR -# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more -# details. -# -# You should have received a copy of the GNU Affero General Public License along -# with this program. If not, see . -# - -require "active_polymorph" - -describe ActiveRecord do - - describe ActiveRecord::Base do - it "should respond to polymorphic_names" do - ActiveRecord::Base.respond_to?(:polymorphic_names).should be_true - end - - it "should by default return its own class" do - class SomeClass < ActiveRecord::Base; end - SomeClass.polymorphic_names.should == ["SomeClass"] - end - - it "should return defined polymorphic names" do - class SomeClass < ActiveRecord::Base - def self.polymorphic_names - [self.base_class.name, "SomeOtherClass"] - end - end - SomeClass.polymorphic_names.sort.should == ["SomeClass", "SomeOtherClass"] - end - end - - describe ActiveRecord::Associations do - before(:all) do - ActiveRecord::Base.establish_connection( - :adapter => 'sqlite3', - :database => ':memory:' - ) - - ActiveRecord::Schema.define do - create_table :things, :force => true do |t| - t.integer :context_id - t.string :context_type - end - - create_table :normal_contexts, :force => true do |t| - t.string :something_meaningful - end - - end - - class Thing < ActiveRecord::Base - belongs_to :context, :polymorphic => true - end - - class NormalContext < ActiveRecord::Base - has_many :things, :as => :context - - def self.polymorphic_names - [self.base_class.name, "Namespaced::ParaNormalContext"] - end - end - - module Namespaced - class ParaNormalContext < ::ActiveRecord::Base - self.table_name = "normal_contexts" - has_many :things, :as => :context - - def self.polymorphic_names - [self.base_class.name, "NormalContext"] - end - end - end - end - - describe "HasManyAssociation" do - before(:each) do - @normal_context = NormalContext.create!(something_meaningful: "This is a holy scripture for a small tribe in Paraguay") - @namespaced_context = Namespaced::ParaNormalContext.find(@normal_context.id) - @thing_a = @normal_context.things.create! - @thing_b = @namespaced_context.things.create! - end - - it "should find records bearing polymorphic names" do - @normal_context.reload.things.should == [@thing_a,@thing_b] - @namespaced_context.reload.things.should == [@thing_a,@thing_b] - end - - it "should not find records not bearing polymorphic names" do - ActiveRecord::Base.connection.execute("UPDATE things SET context_type = 'NotReallyAThing' WHERE id = #{@thing_a.id}") - @normal_context.reload.things.should == [@thing_b] - @namespaced_context.reload.things.should == [@thing_b] - end - - - it "should use polymorphic names in count queries" do - @normal_context.reload.things.count.should == 2 - ActiveRecord::Base.connection.execute("UPDATE things SET context_type = 'NotReallyAThing' WHERE id = #{@thing_a.id}") - @normal_context.reload.things.count.should == 1 - end - - it "should prefer custom finder_sql if provided" do - NormalContext.class_eval do - has_many :things, :as => :context, :finder_sql => - proc{ "SELECT * FROM things WHERE context_id = #{id} AND context_type = '#{self.class.base_class.name}'" } - end - - @namespaced_context.reload.things.should == [@thing_a, @thing_b] - @normal_context.reload.things.should == [@thing_a] - end - - it "should prefer custom counter_sql if provided" do - NormalContext.class_eval do - has_many :things, :as => :context, :counter_sql => - proc{ "SELECT COUNT(*) FROM things WHERE context_id = #{id} AND context_type = '#{self.class.base_class.name}'" } - end - - @namespaced_context.reload.things.count.should == 2 - @normal_context.reload.things.count.should == 1 - end - - it "should count/find correctly with additional where/find scopes" do - @normal_context.things.find(@thing_a.id).should == @thing_a - @normal_context.things.where(context_type: "Namespaced::ParaNormalContext").first.should == @thing_b - scope = @normal_context.things.where(context_type: "Namespaced::ParaNormalContext") - scope.count.should == 1 - scope.should == [@thing_b] - end - - it "should work with pluck" do - @normal_context.things.pluck(:id).should == [@thing_a.id, @thing_b.id] - end - - it "should work with build_associations" do - thing_c = @normal_context.things.build - thing_c.save! - @normal_context.reload.things.should == [@thing_a, @thing_b, thing_c] - end - - end - end -end diff --git a/gems/active_polymorph/test.sh b/gems/active_polymorph/test.sh deleted file mode 100755 index 5fc361d81a5..00000000000 --- a/gems/active_polymorph/test.sh +++ /dev/null @@ -1,15 +0,0 @@ -#!/bin/bash -result=0 - -echo "################ active_polymorph ################" -bundle check || bundle install -bundle exec rspec spec -let result=$result+$? - -if [ $result -eq 0 ]; then - echo "SUCCESS" -else - echo "FAILURE" -fi - -exit $result diff --git a/spec/lib/user_merge_spec.rb b/spec/lib/user_merge_spec.rb index a27438a69ca..b7cd92bb383 100644 --- a/spec/lib/user_merge_spec.rb +++ b/spec/lib/user_merge_spec.rb @@ -518,7 +518,6 @@ describe UserMerge do qs1 = @quiz_submission quiz_with_graded_submission([], user: user2) qs2 = @quiz_submission - Version.where(:versionable_type => "Quizzes::QuizSubmission", :versionable_id => qs2).update_all(:versionable_type => "QuizSubmission") expect(qs1.versions).to be_present qs1.versions.each{ |v| expect(v.model.user_id).to eql(user2.id) } diff --git a/spec/models/quizzes/quiz_spec.rb b/spec/models/quizzes/quiz_spec.rb index 9bee7c031ab..19d31c01434 100644 --- a/spec/models/quizzes/quiz_spec.rb +++ b/spec/models/quizzes/quiz_spec.rb @@ -1701,39 +1701,6 @@ describe Quizzes::Quiz do end end - context 'with versioning' do - let_once(:quiz) { @course.quizzes.create! title: 'Test Quiz' } - describe "#versions" do - it "finds the versions of both namespaced and non-namespaced quizzes" do - quiz.title = "Renamed Test Quiz" - quiz.save - expect(quiz.versions.count).to eq 2 - - Version.where(versionable_id: quiz, versionable_type: 'Quizzes::Quiz').update_all("versionable_type='Quiz'") - - expect(Quizzes::Quiz.find(quiz).versions.count).to eq 2 - end - end - end - - describe '#context_module_tags' do - it "finds both namespaced and non-namespaced content tags" do - quiz = @course.quizzes.create! title: 'Test Quiz' - mod = @course.context_modules.create! name: 'Test Module' - tag1 = mod.add_item id: quiz.id, type: 'quiz' - tag2 = mod.add_item id: quiz.id, type: 'quiz' - tag3 = mod.add_item id: quiz.id, type: 'quiz' - ContentTag.where(id: tag2).update_all(content_type: 'Quiz') - tag3.destroy - expect(quiz.context_module_tags.pluck(:id).sort).to eql [tag1.id, tag2.id].sort - end - - it "should act like an association" do - quiz = @course.quizzes.create! title: 'Test Quiz' - expect { quiz.context_module_tags.loaded? }.not_to raise_error - end - end - describe 'differentiated assignments' do context 'visible_to_user?' do before :once do diff --git a/spec/models/quizzes/quiz_submission_spec.rb b/spec/models/quizzes/quiz_submission_spec.rb index f43a0834fa2..4ede44045cd 100644 --- a/spec/models/quizzes/quiz_submission_spec.rb +++ b/spec/models/quizzes/quiz_submission_spec.rb @@ -1490,51 +1490,6 @@ describe Quizzes::QuizSubmission do end end - context "with versioning" do - before(:each) do - student_in_course - assignment_quiz([]) - qd = multiple_choice_question_data - @quiz.quiz_data = [qd] - @quiz.points_possible = qd[:points_possible] - @quiz.save! - end - - describe "#versions" do - it "finds the versions with both namespaced and non-namespaced quizzes" do - qs = @quiz.generate_submission(@student) - qs.submission_data = { "question_1" => "2405" } - Quizzes::SubmissionGrader.new(qs).grade_submission - - qs = @quiz.generate_submission(@student) - qs.submission_data = { "question_1" => "8544" } - Quizzes::SubmissionGrader.new(qs).grade_submission - - expect(qs.versions.count).to eq 2 - Version.update_all("versionable_type='QuizSubmission'","versionable_id=#{qs.id} AND versionable_type='Quizzes::QuizSubmission'") - expect(Quizzes::QuizSubmission.find(qs).versions.count).to eq 2 - end - end - end - - context "with attachments" do - before(:each) do - course_with_student_logged_in :active_all => true - course_quiz !!:active - @qs = @quiz.generate_submission @user - create_attachment_for_file_upload_submission!(@qs) - end - - describe "#attachments" do - it "finds the attachment with both namespaced and non-namespaced quizzes" do - expect(Quizzes::QuizSubmission.find(@qs).attachments.count).to eq 1 - - Attachment.update_all("context_type='QuizSubmission'","context_id=#{@qs.id} AND context_type='Quizzes::QuizSubmission'") - expect(Quizzes::QuizSubmission.find(@qs).attachments.count).to eq 1 - end - end - end - describe '#points_possible_at_submission_time' do it 'should work' do quiz_with_graded_submission([