From 28ded8aecefa5beea392310181d23d94839c3c05 Mon Sep 17 00:00:00 2001 From: Dante Soares Date: Mon, 1 Feb 2021 10:46:14 -0600 Subject: [PATCH 1/9] Added ActiveStorage --- Gemfile | 11 ++- Gemfile.lock | 17 +++- .../api/v1/attachments_controller.rb | 50 ------------ .../api/v1/attachment_representer.rb | 32 -------- app/representers/api/v1/image_representer.rb | 17 ++++ config/environments/production.rb | 2 +- config/initializers/active_storage.rb | 12 +++ config/routes.rb | 3 +- config/secrets.yml | 1 + config/storage.yml | 41 +++------- ...te_active_storage_tables.active_storage.rb | 36 +++++++++ db/schema.rb | 32 +++++++- lib/has_attachments.rb | 21 +++-- ...nter_spec.rb => image_representer_spec.rb} | 2 +- .../api/v1/attachments_controller_spec.rb | 77 ------------------- 15 files changed, 148 insertions(+), 206 deletions(-) delete mode 100644 app/controllers/api/v1/attachments_controller.rb delete mode 100644 app/representers/api/v1/attachment_representer.rb create mode 100644 app/representers/api/v1/image_representer.rb create mode 100644 config/initializers/active_storage.rb create mode 100644 db/migrate/20210201164624_create_active_storage_tables.active_storage.rb rename spec/representers/api/v1/{attachment_representer_spec.rb => image_representer_spec.rb} (72%) delete mode 100644 spec/requests/api/v1/attachments_controller_spec.rb diff --git a/Gemfile b/Gemfile index 98da6db1..775ddeb8 100644 --- a/Gemfile +++ b/Gemfile @@ -49,6 +49,12 @@ gem 'rinku' # Sanitizes user content gem 'sanitize' +# ActiveStorage variants +gem 'image_processing' + +# ActiveStorage S3 support +gem 'aws-sdk-s3' + # Utilities for OpenStax websites gem 'openstax_utilities' @@ -76,14 +82,11 @@ gem 'fine_print' # Keyword search gem 'keyword_search' -# File uploads +# File uploads (old) gem 'remotipart' gem 'carrierwave' gem 'mimemagic' -# Image editing -gem 'mini_magick' - # Read Excel xlsx spreadsheet files gem 'roo' diff --git a/Gemfile.lock b/Gemfile.lock index 68d545e6..132c9c62 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -87,6 +87,13 @@ GEM aws-partitions (~> 1, >= 1.239.0) aws-sigv4 (~> 1.1) jmespath (~> 1.0) + aws-sdk-kms (1.41.0) + aws-sdk-core (~> 3, >= 3.109.0) + aws-sigv4 (~> 1.1) + aws-sdk-s3 (1.87.0) + aws-sdk-core (~> 3, >= 3.109.0) + aws-sdk-kms (~> 1) + aws-sigv4 (~> 1.1) aws-sdk-secretsmanager (1.43.0) aws-sdk-core (~> 3, >= 3.109.0) aws-sigv4 (~> 1.1) @@ -201,6 +208,9 @@ GEM multi_xml (>= 0.5.2) i18n (1.8.7) concurrent-ruby (~> 1.0) + image_processing (1.12.1) + mini_magick (>= 4.9.5, < 5) + ruby-vips (>= 2.0.17, < 3) ipaddress (0.8.3) jaro_winkler (1.5.2) jmespath (1.4.0) @@ -245,7 +255,7 @@ GEM mime-types-data (~> 3.2015) mime-types-data (3.2019.0331) mimemagic (0.3.5) - mini_magick (4.9.4) + mini_magick (4.11.0) mini_mime (1.0.2) mini_portile2 (2.5.0) mini_racer (0.2.6) @@ -427,6 +437,8 @@ GEM rubocop (>= 0.70.0) ruby-graphviz (1.2.4) ruby-progressbar (1.10.1) + ruby-vips (2.0.17) + ffi (~> 1.9) ruby_dep (1.5.0) rubyzip (1.3.0) sanitize (5.2.1) @@ -507,6 +519,7 @@ DEPENDENCIES addressable apipie-rails autoprefixer-rails + aws-sdk-s3 aws-sdk-secretsmanager aws-sdk-ssm bootsnap (~> 1.4.0) @@ -528,6 +541,7 @@ DEPENDENCIES fine_print fog-aws httparty + image_processing jquery-rails jquery-ui-rails keyword_search @@ -536,7 +550,6 @@ DEPENDENCIES lograge maruku mimemagic - mini_magick mini_racer nifty-generators oj diff --git a/app/controllers/api/v1/attachments_controller.rb b/app/controllers/api/v1/attachments_controller.rb deleted file mode 100644 index a90f68c5..00000000 --- a/app/controllers/api/v1/attachments_controller.rb +++ /dev/null @@ -1,50 +0,0 @@ -module Api::V1 - class AttachmentsController < OpenStax::Api::V1::ApiController - - include ::Exercises::Finders - - before_action :find_exercise_or_create_draft, only: [:create] - before_action :find_exercise, only: [:destroy] - - ########## - # create # - ########## - - api :POST, '/exercises/:exercise_id/attachments', 'Save an image onto an exercise' - description <<-EOS - Saves a file asset as an attachment on an Exercise. - - Unlike other API calls, this is accomplished via a multi-part form upload - with a file part, not as a traditional POST of JSON data - - Requires a single form parameter named "file" - - #{json_schema(Api::V1::AttachmentRepresenter, include: :readable)} - EOS - def create - attachment = AttachFile.call( - attachable: @exercise, file: params[:file].tempfile - ).outputs[:attachment] - respond_with attachment, represent_with: Api::V1::AttachmentRepresenter, location: nil - end - - - ########### - # destroy # - ########### - - api :DELETE, '/exercises/:exercise_id/attachments', 'Deletes the specified Attachment' - description <<-EOS - Deletes an attachment belonging to an exercise - - Requires a single form parameter named "filename" - - #{json_schema(Api::V1::AttachmentRepresenter, include: :readable)} - EOS - def destroy - attachment = @exercise.attachments.find_by! asset: params[:filename] - standard_destroy(attachment) - end - - end -end diff --git a/app/representers/api/v1/attachment_representer.rb b/app/representers/api/v1/attachment_representer.rb deleted file mode 100644 index 6b9f9733..00000000 --- a/app/representers/api/v1/attachment_representer.rb +++ /dev/null @@ -1,32 +0,0 @@ -module Api::V1 - class AttachmentRepresenter < Roar::Decorator - - include Roar::JSON - - property :id, - type: String, - writeable: false, - readable: true - - property :asset, - type: String, - readable: true, - writeable: false, - getter: ->(*) do - host = AssetUploader.asset_host - filename = read_attribute(:asset) - - { - filename: filename, - url: "#{host}/attachments/#{filename}", - large: { url: "#{host}/attachments/large_#{filename}" }, - medium: { url: "#{host}/attachments/medium_#{filename}" }, - small: { url: "#{host}/attachments/small_#{filename}" } - } - end, - schema_info: { - required: true - } - - end -end diff --git a/app/representers/api/v1/image_representer.rb b/app/representers/api/v1/image_representer.rb new file mode 100644 index 00000000..f9c2baec --- /dev/null +++ b/app/representers/api/v1/image_representer.rb @@ -0,0 +1,17 @@ +module Api::V1 + class ImageRepresenter < Roar::Decorator + include Roar::JSON + + property :id, + type: String, + writeable: false, + readable: true + + property :created_at, + type: String, + readable: true, + writeable: false, + getter: ->(*) { DateTimeUtilities.to_api_s(created_at) }, + schema_info: { required: true } + end +end diff --git a/config/environments/production.rb b/config/environments/production.rb index edb864e0..f35f79b5 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -38,7 +38,7 @@ # config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for NGINX # Store uploaded files on the local file system (see config/storage.yml for options). - config.active_storage.service = :local + config.active_storage.service = :amazon # Force all access to the app over SSL, use Strict-Transport-Security, and use secure cookies. # config.force_ssl = true diff --git a/config/initializers/active_storage.rb b/config/initializers/active_storage.rb new file mode 100644 index 00000000..869b4f46 --- /dev/null +++ b/config/initializers/active_storage.rb @@ -0,0 +1,12 @@ +Rails.application.config.after_initialize do + ActiveStorage::Blob.class_exec do + # To prevent problems with case-insensitive filesystems, especially in combination + # with databases which treat indices as case-sensitive, all blob keys generated are going + # to only contain the base-36 character alphabet and will therefore be lowercase. To maintain + # the same or higher amount of entropy as in the base-58 encoding used by `has_secure_token` + # the number of bytes used is increased to 28 from the standard 24 + def self.generate_unique_secure_token(length: ActiveStorage::Blob::MINIMUM_TOKEN_LENGTH) + "#{Rails.application.secrets.environment_name}/#{SecureRandom.base36(length)}" + end + end +end diff --git a/config/routes.rb b/config/routes.rb index cc68727c..1b983896 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -22,8 +22,6 @@ resources :exercises do match :search, action: :index, via: [:get, :post], on: :collection - resource :attachments, only: [:create, :destroy] - publishable # Not in V1 #has_logic @@ -51,6 +49,7 @@ mount OpenStax::Accounts::Engine => :accounts mount FinePrint::Engine => :fine_print + mount ActiveStorage::Engine, at: '/rails/active_storage' mount OpenStax::Utilities::Engine => :status use_doorkeeper do diff --git a/config/secrets.yml b/config/secrets.yml index abe6799f..8b8724e3 100644 --- a/config/secrets.yml +++ b/config/secrets.yml @@ -51,6 +51,7 @@ test: aws: s3: region: us-east-1 + exports_bucket_name: not-a-real-bucket uploads_bucket_name: not-a-real-bucket access_key_id: NOTAREALKEY secret_access_key: NOTAREALSECRET diff --git a/config/storage.yml b/config/storage.yml index d32f76e8..2540c4be 100644 --- a/config/storage.yml +++ b/config/storage.yml @@ -1,34 +1,17 @@ -test: - service: Disk - root: <%= Rails.root.join("tmp/storage") %> - local: service: Disk root: <%= Rails.root.join("storage") %> -# Use rails credentials:edit to set the AWS secrets (as aws:access_key_id|secret_access_key) -# amazon: -# service: S3 -# access_key_id: <%= Rails.application.credentials.dig(:aws, :access_key_id) %> -# secret_access_key: <%= Rails.application.credentials.dig(:aws, :secret_access_key) %> -# region: us-east-1 -# bucket: your_own_bucket - -# Remember not to checkin your GCS keyfile to a repository -# google: -# service: GCS -# project: your_project -# credentials: <%= Rails.root.join("path/to/gcs.keyfile") %> -# bucket: your_own_bucket - -# Use rails credentials:edit to set the Azure Storage secret (as azure_storage:storage_access_key) -# microsoft: -# service: AzureStorage -# storage_account_name: your_account_name -# storage_access_key: <%= Rails.application.credentials.dig(:azure_storage, :storage_access_key) %> -# container: your_container_name +test: + service: Disk + root: <%= Rails.root.join("tmp/storage") %> -# mirror: -# service: Mirror -# primary: local -# mirrors: [ amazon, google, microsoft ] +amazon: + service: S3 + access_key_id: <%= ENV['AWS_S3_ACCESS_KEY_ID'] %> + secret_access_key: <%= ENV['AWS_S3_SECRET_ACCESS_KEY'] %> + bucket: <%= ENV['AWS_S3_UPLOADS_BUCKET_NAME'] %> + region: <%= ENV['AWS_S3_REGION'] %> + http_open_timeout: <%= ENV.fetch('AWS_S3_OPEN_TIMEOUT', 60).to_i %> + http_read_timeout: <%= ENV.fetch('AWS_S3_READ_TIMEOUT', 60).to_i %> + retry_limit: <%= ENV.fetch('AWS_S3_RETRY_LIMIT', 3).to_i %> diff --git a/db/migrate/20210201164624_create_active_storage_tables.active_storage.rb b/db/migrate/20210201164624_create_active_storage_tables.active_storage.rb new file mode 100644 index 00000000..87798267 --- /dev/null +++ b/db/migrate/20210201164624_create_active_storage_tables.active_storage.rb @@ -0,0 +1,36 @@ +# This migration comes from active_storage (originally 20170806125915) +class CreateActiveStorageTables < ActiveRecord::Migration[5.2] + def change + create_table :active_storage_blobs do |t| + t.string :key, null: false + t.string :filename, null: false + t.string :content_type + t.text :metadata + t.string :service_name, null: false + t.bigint :byte_size, null: false + t.string :checksum, null: false + t.datetime :created_at, null: false + + t.index [ :key ], unique: true + end + + create_table :active_storage_attachments do |t| + t.string :name, null: false + t.references :record, null: false, polymorphic: true, index: false + t.references :blob, null: false + + t.datetime :created_at, null: false + + t.index [ :record_type, :record_id, :name, :blob_id ], name: "index_active_storage_attachments_uniqueness", unique: true + t.foreign_key :active_storage_blobs, column: :blob_id + end + + create_table :active_storage_variant_records do |t| + t.belongs_to :blob, null: false, index: false + t.string :variation_digest, null: false + + t.index %i[ blob_id variation_digest ], name: "index_active_storage_variant_records_uniqueness", unique: true + t.foreign_key :active_storage_blobs, column: :blob_id + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 695970fe..40b39961 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,13 +10,41 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_08_11_214625) do +ActiveRecord::Schema.define(version: 2021_02_01_164624) do # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pgcrypto" enable_extension "plpgsql" + create_table "active_storage_attachments", force: :cascade do |t| + t.string "name", null: false + t.string "record_type", null: false + t.bigint "record_id", null: false + t.bigint "blob_id", null: false + t.datetime "created_at", null: false + t.index ["blob_id"], name: "index_active_storage_attachments_on_blob_id" + t.index ["record_type", "record_id", "name", "blob_id"], name: "index_active_storage_attachments_uniqueness", unique: true + end + + create_table "active_storage_blobs", force: :cascade do |t| + t.string "key", null: false + t.string "filename", null: false + t.string "content_type" + t.text "metadata" + t.string "service_name", null: false + t.bigint "byte_size", null: false + t.string "checksum", null: false + t.datetime "created_at", null: false + t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true + end + + create_table "active_storage_variant_records", force: :cascade do |t| + t.bigint "blob_id", null: false + t.string "variation_digest", null: false + t.index ["blob_id", "variation_digest"], name: "index_active_storage_variant_records_uniqueness", unique: true + end + create_table "administrators", id: :serial, force: :cascade do |t| t.integer "user_id", null: false t.datetime "created_at", null: false @@ -532,6 +560,8 @@ t.index ["voter_id", "voter_type", "vote_scope"], name: "index_votes_on_voter_id_and_voter_type_and_vote_scope" end + add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id" + add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id" add_foreign_key "exercise_tags", "exercises", on_update: :cascade, on_delete: :cascade add_foreign_key "exercise_tags", "tags", on_update: :cascade, on_delete: :cascade add_foreign_key "list_publication_groups", "lists" diff --git a/lib/has_attachments.rb b/lib/has_attachments.rb index 5ef5f3e8..5d06e2a0 100644 --- a/lib/has_attachments.rb +++ b/lib/has_attachments.rb @@ -2,8 +2,16 @@ module HasAttachments module ActiveRecord module Base def has_attachments - class_exec do - has_many :attachments, as: :parent, dependent: :destroy, inverse_of: :parent + has_many :attachments, as: :parent, dependent: :destroy, inverse_of: :parent + + Rails.application.config.after_initialize do + class_exec do + has_many_attached :images do |image| + image.variant :large, resize_to_limit: '720x1080' + image.variant :medium, resize_to_limit: '360x540' + image.variant :small, resize_to_limit: '180x270' + end + end end end end @@ -12,12 +20,11 @@ def has_attachments module Roar module Decorator def has_attachments(options={}) - collection :attachments, + collection :images, { - class: Attachment, - extend: Api::V1::AttachmentRepresenter, - writeable: false, - readable: true + extend: Api::V1::ImageRepresenter, + readable: true, + writeable: false }.merge(options) end end diff --git a/spec/representers/api/v1/attachment_representer_spec.rb b/spec/representers/api/v1/image_representer_spec.rb similarity index 72% rename from spec/representers/api/v1/attachment_representer_spec.rb rename to spec/representers/api/v1/image_representer_spec.rb index 3efd034c..0766ad14 100644 --- a/spec/representers/api/v1/attachment_representer_spec.rb +++ b/spec/representers/api/v1/image_representer_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' module Api::V1 - RSpec.describe AttachmentRepresenter do + RSpec.describe ImageRepresenter do pending "add some examples to (or delete) #{__FILE__}" end end diff --git a/spec/requests/api/v1/attachments_controller_spec.rb b/spec/requests/api/v1/attachments_controller_spec.rb deleted file mode 100644 index 7cb3afce..00000000 --- a/spec/requests/api/v1/attachments_controller_spec.rb +++ /dev/null @@ -1,77 +0,0 @@ -require "rails_helper" - -RSpec.describe Api::V1::AttachmentsController, type: :request, api: true, version: :v1 do - - let(:application) { FactoryBot.create :doorkeeper_application } - let(:user) { FactoryBot.create :user, :agreed_to_terms } - let(:admin) { FactoryBot.create :user, :administrator, :agreed_to_terms } - - let(:user_token) do - FactoryBot.create :doorkeeper_access_token, - application: application, - resource_owner_id: user.id - end - let(:admin_token) do - FactoryBot.create :doorkeeper_access_token, - application: application, - resource_owner_id: admin.id - end - let(:application_token) do - FactoryBot.create :doorkeeper_access_token, - application: application, - resource_owner_id: nil - end - - let(:exercise) do - FactoryBot.build(:exercise).tap do |exercise| - exercise.publication.authors << FactoryBot.build( - :author, user: user, publication: exercise.publication - ) - exercise.publication.copyright_holders << FactoryBot.build( - :copyright_holder, user: user, publication: exercise.publication - ) - exercise.save! - end - end - - let(:exercise_id) { "#{exercise.number}@draft" } - - context "POST /api/exercises/:exercise_id/attachments" do - - let(:image) do - Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/rails.png", 'image/jpeg') - end - - it 'attaches a file to an exercise' do - expect do - api_post api_exercise_attachments_url(exercise_id), user_token, params: { file: image } - end.to change{ exercise.attachments.count }.by(1) - expect(response).to have_http_status(:created) - end - - it 'creates a draft if needed' do - exercise.publication.publish.save! - expect do - api_post api_exercise_attachments_url(exercise_id), user_token, params: { file: image } - end.to change{ exercise.publication_group.reload.latest_version }.from(1).to(2) - expect(response).to have_http_status(:created) - end - - end - - context "DELETE /api/exercises/:exercise_id/attachments" do - it 'removes the attachment when called' do - attachment = AttachFile.call( - attachable: exercise, file: 'spec/fixtures/os_exercises_logo.png' - ).outputs[:attachment] - - expect do - api_delete api_exercise_attachments_url(exercise_id), user_token, - params: { filename: attachment.read_attribute(:asset) }.to_json - end.to change(Attachment, :count).by(-1) - expect(response).to have_http_status(:ok) - expect(exercise.attachments.find_by(id: attachment.id)).to be_nil - end - end - -end From 8ddbe5c44c8236bf9daf8fd1c8e602a08e0af173 Mon Sep 17 00:00:00 2001 From: Dante Soares Date: Mon, 1 Feb 2021 12:21:15 -0600 Subject: [PATCH 2/9] Proxy ActiveStorage files by default --- config/initializers/active_storage.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/initializers/active_storage.rb b/config/initializers/active_storage.rb index 869b4f46..5a6ee3ff 100644 --- a/config/initializers/active_storage.rb +++ b/config/initializers/active_storage.rb @@ -1,3 +1,5 @@ +Rails.application.config.active_storage.resolve_model_to_route = :rails_storage_proxy + Rails.application.config.after_initialize do ActiveStorage::Blob.class_exec do # To prevent problems with case-insensitive filesystems, especially in combination From f07d09ee74be9166668bbfe402142cd7ea7f9bd3 Mon Sep 17 00:00:00 2001 From: Dante Soares Date: Mon, 1 Feb 2021 13:10:39 -0600 Subject: [PATCH 3/9] Use the force flag when resetting the branch after creating the data to migrate --- .github/workflows/migrations.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/migrations.yml b/.github/workflows/migrations.yml index 569bb0be..0bec99d1 100644 --- a/.github/workflows/migrations.yml +++ b/.github/workflows/migrations.yml @@ -65,7 +65,7 @@ jobs: bundle install --jobs 4 --retry 3 bundle exec rake db:create db:schema:load db:seed --trace bundle exec rails runner '10.times { FactoryBot.create :exercise }' - git checkout - + git checkout --force - # Retrieve gem cache for PR merge commit - uses: actions/cache@v2 From 2b2a8db522f24edde2ea2e69f910f0f82a32b998 Mon Sep 17 00:00:00 2001 From: Dante Soares Date: Mon, 1 Feb 2021 14:54:40 -0600 Subject: [PATCH 4/9] Don't attempt to use S3 while precompiling assets --- config/environments/production.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/config/environments/production.rb b/config/environments/production.rb index f35f79b5..45d035ed 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -37,8 +37,10 @@ # config.action_dispatch.x_sendfile_header = 'X-Sendfile' # for Apache # config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for NGINX - # Store uploaded files on the local file system (see config/storage.yml for options). - config.active_storage.service = :amazon + # Store uploaded files on the local file system when precompiling assets and on S3 afterwards + config.active_storage.service = ActiveModel::Type::Boolean.new.cast( + ENV.fetch('DISABLE_S3', false) + ) ? :local : :amazon # Force all access to the app over SSL, use Strict-Transport-Security, and use secure cookies. # config.force_ssl = true From 968db39bee7a277e27ec351c73be8597d4207a69 Mon Sep 17 00:00:00 2001 From: Dante Soares Date: Mon, 1 Feb 2021 17:14:42 -0600 Subject: [PATCH 5/9] Allow writes to the images collection --- lib/has_attachments.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/has_attachments.rb b/lib/has_attachments.rb index 5d06e2a0..cdb4ecf0 100644 --- a/lib/has_attachments.rb +++ b/lib/has_attachments.rb @@ -24,7 +24,7 @@ def has_attachments(options={}) { extend: Api::V1::ImageRepresenter, readable: true, - writeable: false + writeable: true }.merge(options) end end From adb7d3d9b52bde84d00f4708eb8c38c0597e5597 Mon Sep 17 00:00:00 2001 From: Nathan Stitt Date: Tue, 2 Feb 2021 11:51:19 -0600 Subject: [PATCH 6/9] add constraint to filter out active storage --- config/routes.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 1b983896..125c0b1e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -91,5 +91,10 @@ end end - get :'*path', controller: :webview, action: :index + # Catch-all frontend route, excluding active_storage + # https://github.com/rails/rails/issues/31228 + get :'*path', controller: :webview, action: :index, constraints: ->(req) do + req.path.exclude? 'rails/active_storage' + end + end From e424036d20c3c8391d6c1a1b75ad09ad6f7e3233 Mon Sep 17 00:00:00 2001 From: Dante Soares Date: Tue, 2 Feb 2021 13:28:56 -0600 Subject: [PATCH 7/9] Added a migration to convert type tags to assignment-type tags --- ..._replace_type_tags_with_assignment_type.rb | 67 +++++++++++++++++++ db/schema.rb | 2 +- 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20210202184008_replace_type_tags_with_assignment_type.rb diff --git a/db/migrate/20210202184008_replace_type_tags_with_assignment_type.rb b/db/migrate/20210202184008_replace_type_tags_with_assignment_type.rb new file mode 100644 index 00000000..5c2f864d --- /dev/null +++ b/db/migrate/20210202184008_replace_type_tags_with_assignment_type.rb @@ -0,0 +1,67 @@ +class ReplaceTypeTagsWithAssignmentType < ActiveRecord::Migration[6.1] + def up + new_hw_tag = Tag.find_or_create_by name: 'assignment-type:homework' + old_hw_tag_id = Tag.find_by(name: 'type:practice').id + + exercise_ids = ExerciseTag.where(tag_id: old_hw_tag_id).distinct.pluck(:exercise_id) + Exercise.where(id: exercise_ids).preload(:exercise_tags).find_each do |exercise| + if exercise.is_published? + exercise = exercise.new_version + exercise.save! + end + exercise.exercise_tags = exercise.exercise_tags.reject do |et| + old_hw_tag_id == et.tag_id + end + exercise.exercise_tags << ExerciseTag.new(exercise: exercise, tag: new_hw_tag) + exercise.publication.publish.save! + end + + vocab_term_ids = VocabTermTag.where(tag_id: old_hw_tag_id).distinct.pluck(:vocab_term_id) + VocabTerm.where(id: vocab_term_ids).preload(:vocab_term_tags).find_each do |vocab_term| + if vocab_term.is_published? + vocab_term = vocab_term.new_version + vocab_term.save! + end + vocab_term.vocab_term_tags = vocab_term.vocab_term_tags.reject do |vtt| + old_hw_tag_id == vtt.tag_id + end + vocab_term.vocab_term_tags << VocabTermTag.new(vocab_term: vocab_term, tag: new_hw_tag) + vocab_term.publication.publish.save! + end + + new_rd_tag = Tag.find_or_create_by name: 'assignment-type:reading' + old_rd_tag_ids = Tag.where( + name: [ 'type:conceptual', 'type:recall', 'type:conceptual-or-recall' ] + ).pluck(:id) + + exercise_ids = ExerciseTag.where(tag_id: old_rd_tag_ids).distinct.pluck(:exercise_id) + Exercise.where(id: exercise_ids).preload(:exercise_tags).find_each do |exercise| + if exercise.is_published? + exercise = exercise.new_version + exercise.save! + end + exercise.exercise_tags = exercise.exercise_tags.reject do |et| + old_rd_tag_ids.include? et.tag_id + end + exercise.exercise_tags << ExerciseTag.new(exercise: exercise, tag: new_rd_tag) + exercise.publication.publish.save! + end + + vocab_term_ids = VocabTermTag.where(tag_id: old_rd_tag_ids).distinct.pluck(:vocab_term_id) + VocabTerm.where(id: vocab_term_ids).preload(:vocab_term_tags).find_each do |vocab_term| + if vocab_term.is_published? + vocab_term = vocab_term.new_version + vocab_term.save! + end + vocab_term.vocab_term_tags = vocab_term.vocab_term_tags.reject do |vtt| + old_rd_tag_ids.include? vtt.tag_id + end + vocab_term.vocab_term_tags << VocabTermTag.new(vocab_term: vocab_term, tag: new_rd_tag) + vocab_term.publication.publish.save! + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/schema.rb b/db/schema.rb index 40b39961..15a9f5fc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_02_01_164624) do +ActiveRecord::Schema.define(version: 2021_02_02_184008) do # These are extensions that must be enabled in order to support this database enable_extension "citext" From aa4402194b9e792e93478075da43b5061daee4cc Mon Sep 17 00:00:00 2001 From: Dante Soares Date: Tue, 2 Feb 2021 13:33:00 -0600 Subject: [PATCH 8/9] Also preload publication objects --- ...20210202184008_replace_type_tags_with_assignment_type.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/db/migrate/20210202184008_replace_type_tags_with_assignment_type.rb b/db/migrate/20210202184008_replace_type_tags_with_assignment_type.rb index 5c2f864d..37bb7dd5 100644 --- a/db/migrate/20210202184008_replace_type_tags_with_assignment_type.rb +++ b/db/migrate/20210202184008_replace_type_tags_with_assignment_type.rb @@ -4,7 +4,7 @@ def up old_hw_tag_id = Tag.find_by(name: 'type:practice').id exercise_ids = ExerciseTag.where(tag_id: old_hw_tag_id).distinct.pluck(:exercise_id) - Exercise.where(id: exercise_ids).preload(:exercise_tags).find_each do |exercise| + Exercise.where(id: exercise_ids).preload(:exercise_tags, :publication).find_each do |exercise| if exercise.is_published? exercise = exercise.new_version exercise.save! @@ -17,7 +17,9 @@ def up end vocab_term_ids = VocabTermTag.where(tag_id: old_hw_tag_id).distinct.pluck(:vocab_term_id) - VocabTerm.where(id: vocab_term_ids).preload(:vocab_term_tags).find_each do |vocab_term| + VocabTerm.where(id: vocab_term_ids).preload( + :vocab_term_tags, :publication + ).find_each do |vocab_term| if vocab_term.is_published? vocab_term = vocab_term.new_version vocab_term.save! From 9cddc411984bd87ec0d149e3e4116f4687a8d25d Mon Sep 17 00:00:00 2001 From: Dante Soares Date: Tue, 2 Feb 2021 14:00:33 -0600 Subject: [PATCH 9/9] Don't blow up when type:practice does not exist --- ...202184008_replace_type_tags_with_assignment_type.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/db/migrate/20210202184008_replace_type_tags_with_assignment_type.rb b/db/migrate/20210202184008_replace_type_tags_with_assignment_type.rb index 37bb7dd5..75186aba 100644 --- a/db/migrate/20210202184008_replace_type_tags_with_assignment_type.rb +++ b/db/migrate/20210202184008_replace_type_tags_with_assignment_type.rb @@ -1,22 +1,22 @@ class ReplaceTypeTagsWithAssignmentType < ActiveRecord::Migration[6.1] def up new_hw_tag = Tag.find_or_create_by name: 'assignment-type:homework' - old_hw_tag_id = Tag.find_by(name: 'type:practice').id + old_hw_tag_ids = Tag.where(name: 'type:practice').pluck(:id) - exercise_ids = ExerciseTag.where(tag_id: old_hw_tag_id).distinct.pluck(:exercise_id) + exercise_ids = ExerciseTag.where(tag_id: old_hw_tag_ids).distinct.pluck(:exercise_id) Exercise.where(id: exercise_ids).preload(:exercise_tags, :publication).find_each do |exercise| if exercise.is_published? exercise = exercise.new_version exercise.save! end exercise.exercise_tags = exercise.exercise_tags.reject do |et| - old_hw_tag_id == et.tag_id + old_hw_tag_ids.include? et.tag_id end exercise.exercise_tags << ExerciseTag.new(exercise: exercise, tag: new_hw_tag) exercise.publication.publish.save! end - vocab_term_ids = VocabTermTag.where(tag_id: old_hw_tag_id).distinct.pluck(:vocab_term_id) + vocab_term_ids = VocabTermTag.where(tag_id: old_hw_tag_ids).distinct.pluck(:vocab_term_id) VocabTerm.where(id: vocab_term_ids).preload( :vocab_term_tags, :publication ).find_each do |vocab_term| @@ -25,7 +25,7 @@ def up vocab_term.save! end vocab_term.vocab_term_tags = vocab_term.vocab_term_tags.reject do |vtt| - old_hw_tag_id == vtt.tag_id + old_hw_tag_ids.include? vtt.tag_id end vocab_term.vocab_term_tags << VocabTermTag.new(vocab_term: vocab_term, tag: new_hw_tag) vocab_term.publication.publish.save!