diff --git a/app/controllers/github_webhooks_controller.rb b/app/controllers/github_webhooks_controller.rb index bd643c531..e072b1d2e 100644 --- a/app/controllers/github_webhooks_controller.rb +++ b/app/controllers/github_webhooks_controller.rb @@ -16,6 +16,6 @@ def webhook_secret(_payload) def edited_code?(payload) commits = payload[:commits] modified_paths = commits.map { |commit| commit[:added] | commit[:modified] | commit[:removed] }.flatten - modified_paths.count { |path| path.start_with?('en/code') }.positive? + modified_paths.count { |path| path.split('/')[1] == 'code' }.positive? end end diff --git a/app/jobs/upload_job.rb b/app/jobs/upload_job.rb index f45c4dd69..ef7ca0543 100644 --- a/app/jobs/upload_job.rb +++ b/app/jobs/upload_job.rb @@ -35,27 +35,32 @@ class UploadJob < ApplicationJob GRAPHQL def perform(payload) - repository = payload[:repository][:name] - owner = payload[:repository][:owner][:name] - projects_data = load_projects_data(repository, owner) - - projects_data.data.repository.object.entries.each do |project_dir| - project = format_project(project_dir, repository, owner) - project_importer = ProjectImporter.new(**project) - project_importer.import! + modified_locales(payload).each do |locale| + projects_data = load_projects_data(locale, repository(payload), owner(payload)) + projects_data.data.repository.object.entries.each do |project_dir| + project = format_project(project_dir, locale, repository(payload), owner(payload)) + project_importer = ProjectImporter.new(**project) + project_importer.import! + end end end private - def load_projects_data(repository, owner) + def modified_locales(payload) + commits = payload[:commits] + modified_paths = commits.map { |commit| commit[:added] | commit[:modified] | commit[:removed] }.flatten + modified_paths.map { |path| path.split('/')[0] }.uniq + end + + def load_projects_data(locale, repository, owner) GithubApi::Client.query( ProjectContentQuery, - variables: { repository:, owner:, expression: "#{ENV.fetch('GITHUB_WEBHOOK_REF')}:en/code" } + variables: { repository:, owner:, expression: "#{ENV.fetch('GITHUB_WEBHOOK_REF')}:#{locale}/code" } ) end - def format_project(project_dir, repository, owner) + def format_project(project_dir, locale, repository, owner) components = [] images = [] project_dir.object.entries.each do |file| @@ -64,10 +69,10 @@ def format_project(project_dir, repository, owner) elsif file.object.text components << component(file) else - images << image(file, project_dir, repository, owner) + images << image(file, project_dir, locale, repository, owner) end end - { **@proj_config, components:, images: } + { **@proj_config, locale:, components:, images: } end def component(file) @@ -78,10 +83,18 @@ def component(file) { name:, extension:, content:, default: } end - def image(file, project_dir, repository, owner) + def image(file, project_dir, locale, repository, owner) filename = file.name directory = project_dir.name - url = "https://github.com/#{owner}/#{repository}/raw/#{ENV.fetch('GITHUB_WEBHOOK_REF')}/en/code/#{directory}/#{filename}" + url = "https://github.com/#{owner}/#{repository}/raw/#{ENV.fetch('GITHUB_WEBHOOK_REF')}/#{locale}/code/#{directory}/#{filename}" { filename:, io: URI.parse(url).open } end + + def repository(payload) + payload[:repository][:name] + end + + def owner(payload) + payload[:repository][:owner][:name] + end end diff --git a/app/models/project.rb b/app/models/project.rb index d7ab81ebd..ace0132ed 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -4,7 +4,7 @@ class Project < ApplicationRecord before_validation :check_unique_not_null, on: :create - validates :identifier, presence: true, uniqueness: true + validates :identifier, presence: true, uniqueness: { scope: :locale } belongs_to :parent, class_name: 'Project', foreign_key: 'remixed_from_id', optional: true, inverse_of: :remixes has_many :components, -> { order(default: :desc, name: :asc) }, dependent: :destroy, inverse_of: :project has_many :remixes, class_name: 'Project', foreign_key: 'remixed_from_id', dependent: :nullify, inverse_of: :parent @@ -15,6 +15,6 @@ class Project < ApplicationRecord def check_unique_not_null self.identifier ||= PhraseIdentifier.generate - self.identifier = PhraseIdentifier.generate until Project.find_by(identifier: self.identifier).nil? + self.identifier = PhraseIdentifier.generate until Project.find_by(identifier: self.identifier, locale:).nil? end end diff --git a/db/migrate/20230303112129_remove_identifier_uniqueness.rb b/db/migrate/20230303112129_remove_identifier_uniqueness.rb new file mode 100644 index 000000000..9be6626b8 --- /dev/null +++ b/db/migrate/20230303112129_remove_identifier_uniqueness.rb @@ -0,0 +1,11 @@ +class RemoveIdentifierUniqueness < ActiveRecord::Migration[7.0] + def up + remove_index :projects, :identifier, unique: true + add_index :projects, :identifier + end + + def down + remove_index :projects, :identifier + add_index :projects, :identifier, unique: true + end +end diff --git a/db/migrate/20230303113051_make_identifier_and_locale_unique.rb b/db/migrate/20230303113051_make_identifier_and_locale_unique.rb new file mode 100644 index 000000000..87a2c0c22 --- /dev/null +++ b/db/migrate/20230303113051_make_identifier_and_locale_unique.rb @@ -0,0 +1,5 @@ +class MakeIdentifierAndLocaleUnique < ActiveRecord::Migration[7.0] + def change + add_index :projects, [:identifier, :project_locale], unique: true + end +end diff --git a/db/migrate/20230303123149_rename_project_locale_column.rb b/db/migrate/20230303123149_rename_project_locale_column.rb new file mode 100644 index 000000000..aee4e862f --- /dev/null +++ b/db/migrate/20230303123149_rename_project_locale_column.rb @@ -0,0 +1,5 @@ +class RenameProjectLocaleColumn < ActiveRecord::Migration[7.0] + def change + rename_column :projects, :project_locale, :locale + end +end diff --git a/db/schema.rb b/db/schema.rb index 4a20dd90b..fe23a335f 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[7.0].define(version: 2023_03_02_145905) do +ActiveRecord::Schema[7.0].define(version: 2023_03_03_123149) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -121,8 +121,9 @@ t.datetime "created_at", null: false t.datetime "updated_at", null: false t.uuid "remixed_from_id" - t.string "project_locale" - t.index ["identifier"], name: "index_projects_on_identifier", unique: true + t.string "locale" + t.index ["identifier", "locale"], name: "index_projects_on_identifier_and_locale", unique: true + t.index ["identifier"], name: "index_projects_on_identifier" t.index ["remixed_from_id"], name: "index_projects_on_remixed_from_id" end diff --git a/lib/project_importer.rb b/lib/project_importer.rb index 9912e5593..306571f84 100644 --- a/lib/project_importer.rb +++ b/lib/project_importer.rb @@ -1,14 +1,15 @@ # frozen_string_literal: true class ProjectImporter - attr_reader :name, :identifier, :images, :components, :type - - def initialize(name:, identifier:, type:, components:, images: []) - @name = name - @identifier = identifier - @components = components - @images = images - @type = type + attr_reader :name, :identifier, :images, :components, :type, :locale + + def initialize(**kwargs) + @name = kwargs[:name] + @identifier = kwargs[:identifier] + @components = kwargs[:components] + @images = kwargs[:images] + @type = kwargs[:type] + @locale = kwargs[:locale] end def import! @@ -26,7 +27,7 @@ def import! private def project - @project ||= Project.find_or_initialize_by(identifier:) + @project ||= Project.find_or_initialize_by(identifier:, locale:) end def setup_project diff --git a/lib/tasks/projects.rake b/lib/tasks/projects.rake index ffc039bba..040e8347f 100644 --- a/lib/tasks/projects.rake +++ b/lib/tasks/projects.rake @@ -26,7 +26,8 @@ namespace :projects do end project_importer = ProjectImporter.new(name: proj_config['NAME'], identifier: proj_config['IDENTIFIER'], - type: proj_config['TYPE'] ||= 'python', components:, images:) + type: proj_config['TYPE'] ||= 'python', + locale: proj_config['LOCALE'] ||= 'en', components:, images:) project_importer.import! end end diff --git a/spec/factories/project.rb b/spec/factories/project.rb index 889671c13..e1da44760 100644 --- a/spec/factories/project.rb +++ b/spec/factories/project.rb @@ -6,6 +6,7 @@ name { Faker::Book.title } identifier { "#{Faker::Verb.base}-#{Faker::Verb.base}-#{Faker::Verb.base}" } project_type { 'python' } + locale { %w[en es-LA fr-FR].sample } trait :with_components do transient do diff --git a/spec/jobs/upload_job_spec.rb b/spec/jobs/upload_job_spec.rb index 1a8956438..468df4e24 100644 --- a/spec/jobs/upload_job_spec.rb +++ b/spec/jobs/upload_job_spec.rb @@ -14,10 +14,10 @@ GraphQL::Client::Response.new(raw_response, data: UploadJob::ProjectContentQuery.new(raw_response['data'], GraphQL::Client::Errors.new)) end let(:payload) do - { repository: { name: 'my-amazing-repo', owner: { name: 'me' } } } + { repository: { name: 'my-amazing-repo', owner: { name: 'me' } }, commits: [{ added: ['ja-JP/code/dont-collide-starter/main.py'], modified: [], removed: [] }] } end let(:variables) do - { repository: 'my-amazing-repo', owner: 'me', expression: "#{ENV.fetch('GITHUB_WEBHOOK_REF')}:en/code" } + { repository: 'my-amazing-repo', owner: 'me', expression: "#{ENV.fetch('GITHUB_WEBHOOK_REF')}:ja-JP/code" } end let(:raw_response) do @@ -74,6 +74,7 @@ name: "Don't Collide!", identifier: 'dont-collide-starter', type: 'python', + locale: 'ja-JP', components: [ { name: 'main', @@ -93,7 +94,7 @@ before do allow(GithubApi::Client).to receive(:query).and_return(graphql_response) - stub_request(:get, 'https://github.com/me/my-amazing-repo/raw/branches/whatever/en/code/dont-collide-starter/astronaut1.png').to_return(status: 200, body: '', headers: {}) + stub_request(:get, 'https://github.com/me/my-amazing-repo/raw/branches/whatever/ja-JP/code/dont-collide-starter/astronaut1.png').to_return(status: 200, body: '', headers: {}) allow(ProjectImporter).to receive(:new).and_call_original end @@ -114,7 +115,7 @@ it 'requests the image from the correct URL' do described_class.perform_now(payload) - expect(WebMock).to have_requested(:get, 'https://github.com/me/my-amazing-repo/raw/branches/whatever/en/code/dont-collide-starter/astronaut1.png').once + expect(WebMock).to have_requested(:get, 'https://github.com/me/my-amazing-repo/raw/branches/whatever/ja-JP/code/dont-collide-starter/astronaut1.png').once end it 'saves the project to the database' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f8baab59e..96f11a905 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -14,21 +14,22 @@ end end - describe 'identifier not nil' do - it 'generates an identifier if not present' do - proj = build(:project, identifier: nil) - expect { proj.valid? } - .to change { proj.identifier.nil? } - .from(true) - .to(false) + describe 'check_unique_not_null' do + let(:saved_project) { create(:project) } + + it 'generates an identifier if nil' do + unsaved_project = build(:project, identifier: nil) + expect { unsaved_project.valid? }.to change { unsaved_project.identifier.nil? }.from(true).to(false) + end + + it 'generates identifier if non-unique within locale' do + invalid_project = build(:project, identifier: saved_project.identifier, locale: saved_project.locale) + expect { invalid_project.valid? }.to change(invalid_project, :identifier) end - end - describe 'identifier unique' do - it do - project1 = create(:project) - project2 = build(:project, identifier: project1.identifier) - expect { project2.valid? }.to change(project2, :identifier) + it 'does not change identifier if duplicated in different locale' do + valid_project = build(:project, identifier: saved_project.identifier, locale: 'ja-JP') + expect { valid_project.valid? }.not_to change(valid_project, :identifier) end end end diff --git a/spec/project_importer_spec.rb b/spec/project_importer_spec.rb index b6b8572e0..85a06554f 100644 --- a/spec/project_importer_spec.rb +++ b/spec/project_importer_spec.rb @@ -9,6 +9,7 @@ name: 'My amazing project', identifier: 'my-amazing-project', type: 'python', + locale: 'ja-JP', components: [ { name: 'main', extension: 'py', content: 'print(\'hello\')', default: true }, { name: 'amazing', extension: 'py', content: 'print(\'this is amazing\')' } @@ -19,8 +20,12 @@ ) end - context 'when the project does not already exist in the database' do - let(:project) { Project.find_by(identifier: importer.identifier) } + context 'when the project with correct locale does not already exist in the database' do + let(:project) { Project.find_by(identifier: importer.identifier, locale: importer.locale) } + + before do + create(:project, identifier: importer.identifier) + end it 'saves the project to the database' do expect { importer.import! }.to change(Project, :count).by(1) @@ -55,7 +60,8 @@ :with_components, :with_attached_image, component_count: 2, - identifier: 'my-amazing-project' + identifier: 'my-amazing-project', + locale: 'ja-JP' ) end