used ignored_columns instead of DROPPED_COLUMNS
closes CORE-1245 recent internal changes in rails (released in 5.1.6) make it more difficult for our module prepend to work, because column information is no longer referenced lazily when checking on attribute defaults (see changes to model_schema.rb). this means that we would need to tie in at a lower level so that _default_attributes excludes the dropped column, but in looking at that, Rails 5 added support for ignoring columns, which ties in at the right level and should solve the same use case we have. see http://api.rubyonrails.org/classes/ActiveRecord/ModelSchema/ClassMethods.html#method-i-ignored_columns-3D Change-Id: I1f76e9d0894551f90232a36f5d48f27f8989c3a1 Reviewed-on: https://gerrit.instructure.com/145723 Tested-by: Jenkins Reviewed-by: Cody Cutrer <cody@instructure.com> QA-Review: Tucker McKnight <tmcknight@instructure.com> Product-Review: Simon Williams <simon@instructure.com>
This commit is contained in:
Simon Williams
parent
29dbbf588d
commit
b4fde1f11b
|
|
@ -16,165 +16,11 @@
|
|||
# with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
#
|
||||
|
||||
ActiveRecord::Base::DROPPED_COLUMNS = {
|
||||
'abstract_courses' => %w(sis_name sis_course_code).freeze,
|
||||
'accounts' => %w(type
|
||||
sis_name
|
||||
account_code
|
||||
authentication_type
|
||||
ldap_host
|
||||
ldap_domain
|
||||
require_authorization_code
|
||||
).freeze,
|
||||
'account_authorization_configs' => %w(auth_uid
|
||||
login_handle_name
|
||||
change_password_url
|
||||
unknown_user_url).freeze,
|
||||
'account_notification_roles' => %w(role_type).freeze,
|
||||
'account_users' => %w(membership_type).freeze,
|
||||
'access_tokens' => %w(token).freeze,
|
||||
'appointment_groups' => %w{context_id context_type sub_context_id sub_context_type}.freeze,
|
||||
'assessment_question_bank_users' => %w{deleted_at permissions workflow_state}.freeze,
|
||||
'assessment_requests' => %w{comments}.freeze,
|
||||
'asset_user_accesses' => %w(asset_access_stat_id
|
||||
interaction_seconds
|
||||
progress
|
||||
count
|
||||
summarized_at
|
||||
).freeze,
|
||||
'assignments' => %w(sequence_position
|
||||
minimum_required_blog_posts
|
||||
minimum_required_blog_comments
|
||||
reminders_created_for_due_at
|
||||
publishing_reminder_sent
|
||||
previously_published
|
||||
before_quiz_submission_types
|
||||
grading_scheme_id
|
||||
location
|
||||
needs_grading_count
|
||||
).freeze,
|
||||
'attachments' => %w(enrollment_id
|
||||
cached_s3_url
|
||||
s3_url_cached_at
|
||||
scribd_account_id
|
||||
scribd_user
|
||||
scribd_mime_type_id
|
||||
submitted_to_scribd_at
|
||||
scribd_doc
|
||||
scribd_attempts
|
||||
cached_scribd_thumbnail
|
||||
last_inline_view
|
||||
local_filename).freeze,
|
||||
'calendar_events' => %w(calendar_event_repeat_id for_repeat_on).freeze,
|
||||
'canvadocs' => %w(preferred_plugin_course_id).freeze,
|
||||
'communication_channels' => %w(access_token_id internal_path).freeze,
|
||||
'content_exports' => %w(course_id).freeze,
|
||||
'content_migrations' => %w{error_count error_data}.freeze,
|
||||
'content_tags' => %w(sequence_position context_module_association_id).freeze,
|
||||
'context_external_tools' => %w(integration_type).freeze,
|
||||
'context_modules' => %w(downstream_modules start_at end_at).freeze,
|
||||
'conversation_messages' => %w(context_message_id).freeze,
|
||||
'course_sections' => %w(sis_cross_listed_section_id
|
||||
sis_cross_listed_section_sis_batch_id
|
||||
sticky_xlist
|
||||
sis_name
|
||||
students_can_participate_before_start_at
|
||||
section_organization_name
|
||||
long_section_code
|
||||
account_id
|
||||
section_code).freeze,
|
||||
'courses' => %w(section
|
||||
hidden_tabs
|
||||
sis_name
|
||||
sis_course_code
|
||||
hashtag
|
||||
allow_student_assignment_edits
|
||||
publish_grades_immediately
|
||||
old_account_id
|
||||
show_all_discussion_entries
|
||||
).freeze,
|
||||
'delayed_notifications' => %w(asset_context_type asset_context_id).freeze,
|
||||
'developer_keys' => %w(tool_id).freeze,
|
||||
'discussion_topics' => %w(authorization_list_id).freeze,
|
||||
'enrollment_terms' => %w(sis_data
|
||||
sis_name
|
||||
ignore_term_date_restrictions).freeze,
|
||||
'enrollments' => %w(invitation_email
|
||||
can_participate_before_start_at
|
||||
computed_current_score
|
||||
computed_final_score
|
||||
limit_priveleges_to_course_sections
|
||||
role_name
|
||||
sis_source_id).freeze,
|
||||
'enrollment_states' => %w{state_invalidated_at state_recalculated_at access_invalidated_at access_recalculated_at}.freeze,
|
||||
'eportfolio_entries' => %w(attachment_id artifact_type url).freeze,
|
||||
'eportfolios' => %w{context_id context_type}.freeze,
|
||||
'external_feeds' => %w(body_match feed_type feed_purpose).freeze,
|
||||
'external_feed_entries' => %w(start_at end_at).freeze,
|
||||
'failed_jobs' => %w(original_id).freeze,
|
||||
'feature_flags' => %w(locking_account_id).freeze,
|
||||
'gradebook_uploads' => %w(context_type context_id).freeze,
|
||||
'grading_periods' => %w(course_id account_id).freeze,
|
||||
'groups' => %w(sis_name type groupable_id groupable_type hashtag
|
||||
show_public_context_messages default_wiki_editing_roles).freeze,
|
||||
'learning_outcome_results' => %w{comments}.freeze,
|
||||
'learning_outcome_question_results' => %w{context_code context_id context_type}.freeze,
|
||||
'lti_resource_placements' => %w(resource_handler_id).freeze,
|
||||
'master_courses_master_migrations' => %w{import_results}.freeze,
|
||||
'messages' => %w(cc bcc notification_category asset_context_code asset_context_type asset_context_id).freeze,
|
||||
'moderated_grading_provisional_grades' => %w(position).freeze,
|
||||
'notification_policies' => %w(user_id broadcast).freeze,
|
||||
'page_views' => %w(contributed).freeze,
|
||||
'pseudonyms' => %w(sis_update_data
|
||||
deleted_unique_id
|
||||
sis_source_id
|
||||
crypted_webdav_access_code
|
||||
type
|
||||
login_path_to_ignore).freeze,
|
||||
'quizzes' => %w(root_quiz_id).freeze,
|
||||
'role_overrides' => %w(context_code enrollment_type).freeze,
|
||||
'rubric_assessments' => %w{comments}.freeze,
|
||||
'rubric_associations' => %w{description}.freeze,
|
||||
'sis_batches' => %w(batch_id errored_attempts).freeze,
|
||||
'stream_items' => %w(context_code item_asset_string).freeze,
|
||||
'stream_item_instances' => %w(context_code).freeze,
|
||||
'submissions' => %w(changed_since_publish late accepted_at).freeze,
|
||||
'submission_comments' => %w{recipient_id}.freeze,
|
||||
'users' => %w(type
|
||||
creation_unique_id
|
||||
creation_sis_batch_id
|
||||
creation_email
|
||||
sis_name
|
||||
bio
|
||||
merge_to
|
||||
unread_inbox_items_count
|
||||
visibility
|
||||
).freeze,
|
||||
'web_conference_participants' => %w{workflow_state}.freeze,
|
||||
'wiki_pages' => %w(hide_from_students delayed_post_at recent_editors wiki_page_comments_count).freeze
|
||||
}.freeze
|
||||
|
||||
module DroppedColumns
|
||||
def columns_hash
|
||||
@columns_hash_with_dropped ||= super.reject { |k, c|
|
||||
(ActiveRecord::Base::DROPPED_COLUMNS[self.table_name] || []).include?(k)
|
||||
}
|
||||
end
|
||||
|
||||
def reset_column_information
|
||||
@columns_hash_with_dropped = nil
|
||||
super
|
||||
end
|
||||
|
||||
def define_attribute(name, *args)
|
||||
super unless (ActiveRecord::Base::DROPPED_COLUMNS[self.table_name] || []).include?(name)
|
||||
end
|
||||
|
||||
def instantiate(attributes, *args)
|
||||
(ActiveRecord::Base::DROPPED_COLUMNS[self.table_name] || []).each do |attr|
|
||||
attributes.delete(attr)
|
||||
end unless self < Tableless
|
||||
super
|
||||
end
|
||||
end
|
||||
ActiveRecord::Base.singleton_class.prepend(DroppedColumns)
|
||||
# Use `ignored_columns` instead:
|
||||
# class User
|
||||
# self.ignored_columns = %w(name)
|
||||
#
|
||||
# ...
|
||||
# end
|
||||
#
|
||||
# http://api.rubyonrails.org/classes/ActiveRecord/ModelSchema/ClassMethods.html#method-i-ignored_columns-3D
|
||||
|
|
|
|||
|
|
@ -179,44 +179,6 @@ describe ActiveRecord::Base do
|
|||
end
|
||||
end
|
||||
|
||||
describe "#remove_dropped_columns" do
|
||||
before do
|
||||
@orig_dropped = ActiveRecord::Base::DROPPED_COLUMNS
|
||||
end
|
||||
|
||||
after do
|
||||
ActiveRecord::Base.send(:remove_const, :DROPPED_COLUMNS)
|
||||
ActiveRecord::Base::DROPPED_COLUMNS = @orig_dropped
|
||||
User.reset_column_information
|
||||
end
|
||||
|
||||
it "should mask columns marked as dropped from column info methods" do
|
||||
expect(User.columns.any? { |c| c.name == 'name' }).to be_truthy
|
||||
expect(User.column_names).to be_include('name')
|
||||
u = User.create!(:name => 'my name')
|
||||
# if we ever actually drop the name column, this spec will fail on the line
|
||||
# above, so it's all good
|
||||
ActiveRecord::Base.send(:remove_const, :DROPPED_COLUMNS)
|
||||
ActiveRecord::Base::DROPPED_COLUMNS = { 'users' => %w(name) }
|
||||
User.reset_column_information
|
||||
expect(User.columns.any? { |c| c.name == 'name' }).to be_falsey
|
||||
expect(User.column_names).not_to be_include('name')
|
||||
|
||||
# load from the db should hide the attribute
|
||||
u = User.find(u.id)
|
||||
expect(u.attributes.keys.include?('name')).to be_falsey
|
||||
end
|
||||
|
||||
it "should only drop columns from the specific table specified" do
|
||||
ActiveRecord::Base.send(:remove_const, :DROPPED_COLUMNS)
|
||||
ActiveRecord::Base::DROPPED_COLUMNS = { 'users' => %w(name) }
|
||||
User.reset_column_information
|
||||
Group.reset_column_information
|
||||
expect(User.columns.any? { |c| c.name == 'name' }).to be_falsey
|
||||
expect(Group.columns.any? { |c| c.name == 'name' }).to be_truthy
|
||||
end
|
||||
end
|
||||
|
||||
context "rank helpers" do
|
||||
it "should generate appropriate rank sql" do
|
||||
expect(ActiveRecord::Base.rank_sql(['a', ['b', 'c'], ['d']], 'foo')).
|
||||
|
|
|
|||
Loading…
Reference in New Issue