diff --git a/app/models/project.rb b/app/models/project.rb index ace0132e..165cf513 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -5,6 +5,8 @@ class Project < ApplicationRecord before_validation :check_unique_not_null, on: :create validates :identifier, presence: true, uniqueness: { scope: :locale } + validate :identifier_cannot_be_taken_by_another_user + validates :locale, presence: true, unless: :user_id 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 +17,11 @@ class Project < ApplicationRecord def check_unique_not_null self.identifier ||= PhraseIdentifier.generate - self.identifier = PhraseIdentifier.generate until Project.find_by(identifier: self.identifier, locale:).nil? + end + + def identifier_cannot_be_taken_by_another_user + return if Project.where(identifier: self.identifier).where.not(user_id:).empty? + + errors.add(:identifier, "can't be taken by another user") end end diff --git a/lib/concepts/project/operations/create_remix.rb b/lib/concepts/project/operations/create_remix.rb index 38a9cdf2..78a103dd 100644 --- a/lib/concepts/project/operations/create_remix.rb +++ b/lib/concepts/project/operations/create_remix.rb @@ -32,10 +32,7 @@ def remix_project(response, params, user_id, original_project) end def create_remix(original_project, params, user_id) - remix = original_project.dup.tap do |proj| - proj.user_id = user_id - proj.remixed_from_id = original_project.id - end + remix = format_project(original_project, user_id) original_project.images.each do |image| remix.images.attach(image.blob) @@ -47,6 +44,15 @@ def create_remix(original_project, params, user_id) remix end + + def format_project(original_project, user_id) + original_project.dup.tap do |proj| + proj.user_id = user_id + proj.remixed_from_id = original_project.id + proj.identifier = PhraseIdentifier.generate + proj.locale = nil + end + end end end end diff --git a/lib/phrase_identifier.rb b/lib/phrase_identifier.rb index f0aeb96d..5c90f5a8 100644 --- a/lib/phrase_identifier.rb +++ b/lib/phrase_identifier.rb @@ -2,6 +2,11 @@ class PhraseIdentifier def self.generate - Word.order(Arel.sql('RANDOM()')).take(3).pluck(:word).join('-') + phrase = Word.order(Arel.sql('RANDOM()')).take(3).pluck(:word).join('-') until unique?(phrase) + phrase + end + + def self.unique?(phrase) + !phrase.nil? && Project.find_by(identifier: phrase).nil? end end diff --git a/spec/concepts/project/create_remix_spec.rb b/spec/concepts/project/create_remix_spec.rb index 3c1307e1..058fe481 100644 --- a/spec/concepts/project/create_remix_spec.rb +++ b/spec/concepts/project/create_remix_spec.rb @@ -45,6 +45,11 @@ expect(remixed_project.identifier).not_to eq(original_project.identifier) end + it 'sets new project locale to nil' do + remixed_project = create_remix[:project] + expect(remixed_project.locale).to be_nil + end + it 'assigns user_id to new project' do remixed_project = create_remix[:project] expect(remixed_project.user_id).to eq(user_id) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 96f11a90..0572db25 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -14,6 +14,49 @@ end end + describe 'validations' do + let(:project) { create(:project) } + let(:identifier) { project.identifier } + + it 'is invalid if no user or locale' do + invalid_project = build(:project, locale: nil, user_id: nil) + expect(invalid_project).to be_invalid + end + + it 'is valid if user but no locale' do + valid_project = build(:project, locale: nil) + expect(valid_project).to be_valid + end + + context 'with same identifier and same user as existing project' do + let(:user_id) { project.user_id } + + it 'is invalid if identifier in use by same user in the same locale' do + new_project = build(:project, identifier:, user_id:, locale: project.locale) + expect(new_project).to be_invalid + end + + it 'is valid if identifier only in use by the user in the another locale' do + new_project = build(:project, identifier:, user_id:, locale: 'another_locale') + expect(new_project).to be_valid + end + end + + context 'with same identifier but different user as existing project' do + let(:user_id) { 'another_user' } + + it 'is invalid if identifier in use by another user in same locale' do + new_project = build(:project, identifier:, user_id:, locale: project.locale) + expect(new_project).to be_invalid + end + + it 'is invalid if identifier in use in another locale by another user' do + new_project = build(:project, identifier:, user_id:, locale: 'another_locale') + expect(new_project).to be_invalid + end + end + end + describe 'check_unique_not_null' do let(:saved_project) { create(:project) } @@ -21,15 +64,5 @@ 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 - - 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 85a06554..0e6b0704 100644 --- a/spec/project_importer_spec.rb +++ b/spec/project_importer_spec.rb @@ -21,10 +21,10 @@ end 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) } + let(:project) { Project.find_by(identifier: importer.identifier, user_id: nil, locale: importer.locale) } before do - create(:project, identifier: importer.identifier) + create(:project, identifier: importer.identifier, user_id: nil) end it 'saves the project to the database' do