From 7104da413547e68247e53951cd85127017387e05 Mon Sep 17 00:00:00 2001
From: loiswells97 <loiswells97@users.noreply.github.com>
Date: Mon, 10 Jan 2022 15:55:58 +0000
Subject: [PATCH 01/10] Create draft PR for #5


From 6c7e85b242e7f56f32b0d71893add4e2ead8c4a8 Mon Sep 17 00:00:00 2001
From: Lois Wells <lois.wells@raspberrypi.org>
Date: Mon, 10 Jan 2022 16:19:16 +0000
Subject: [PATCH 02/10] Updated migration to prevent project identifier being
 null

---
 db/migrate/20211220155302_create_projects.rb | 2 +-
 db/schema.rb                                 | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/db/migrate/20211220155302_create_projects.rb b/db/migrate/20211220155302_create_projects.rb
index bb3613ed6..48eb5e85a 100644
--- a/db/migrate/20211220155302_create_projects.rb
+++ b/db/migrate/20211220155302_create_projects.rb
@@ -3,7 +3,7 @@ def change
     create_table :projects, id: :uuid do |t|
       t.uuid :user_id
       t.string :name
-      t.string :identifier
+      t.string :identifier, null: false
       t.string :project_type, null: false, default: 'python'
 
       t.timestamps
diff --git a/db/schema.rb b/db/schema.rb
index 3fd86102c..0a448cba9 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -29,7 +29,7 @@
   create_table "projects", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
     t.uuid "user_id"
     t.string "name"
-    t.string "identifier"
+    t.string "identifier", null: false
     t.string "project_type", default: "python", null: false
     t.datetime "created_at", precision: 6, null: false
     t.datetime "updated_at", precision: 6, null: false

From 53896b30e274d67182ed55dc4128e6814a608635 Mon Sep 17 00:00:00 2001
From: Lois Wells <lois.wells@raspberrypi.org>
Date: Mon, 10 Jan 2022 17:04:50 +0000
Subject: [PATCH 03/10] Added step before validation to check identifier is
 unique and not null

---
 app/models/project.rb | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index 038537e7b..8a4a19cef 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1,3 +1,14 @@
 class Project < ApplicationRecord
+  before_validation :check_unique_not_null, on: :create
   has_many :components, dependent: :destroy
-end
+
+  require 'phrase_identifier'
+
+  private
+    def check_unique_not_null
+      self.identifier ||= PhraseIdentifier.generate
+      while !Project.find_by(identifier: self.identifier).nil?
+        self.identifier = PhraseIdentifier.generate
+      end
+    end
+end
\ No newline at end of file

From e4e20a9df14f2803d418180bf4656b2d791a5ed2 Mon Sep 17 00:00:00 2001
From: Lois Wells <lois.wells@raspberrypi.org>
Date: Mon, 10 Jan 2022 17:48:16 +0000
Subject: [PATCH 04/10] Using FactoryBot and Faker to test the project
 identifier is unique

---
 Gemfile                     |  2 ++
 Gemfile.lock                | 11 +++++++++++
 spec/factories/project.rb   | 10 ++++++++++
 spec/models/project_spec.rb |  5 +++++
 4 files changed, 28 insertions(+)
 create mode 100644 spec/factories/project.rb

diff --git a/Gemfile b/Gemfile
index f01ca559f..9e00b3024 100644
--- a/Gemfile
+++ b/Gemfile
@@ -16,6 +16,8 @@ gem 'turbo-rails'
 
 group :development, :test do
   gem 'dotenv-rails'
+  gem 'factory_bot_rails'
+  gem 'faker'
   gem 'rspec-rails'
   gem 'rubocop'
 end
diff --git a/Gemfile.lock b/Gemfile.lock
index 6624e9608..015db9226 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -72,6 +72,13 @@ GEM
       dotenv (= 2.7.6)
       railties (>= 3.2)
     erubi (1.10.0)
+    factory_bot (6.2.0)
+      activesupport (>= 5.0.0)
+    factory_bot_rails (6.2.0)
+      factory_bot (~> 6.2.0)
+      railties (>= 5.0.0)
+    faker (2.19.0)
+      i18n (>= 1.6, < 2)
     globalid (1.0.0)
       activesupport (>= 5.0)
     i18n (1.8.11)
@@ -96,6 +103,8 @@ GEM
     nokogiri (1.12.5)
       mini_portile2 (~> 2.6.1)
       racc (~> 1.4)
+    nokogiri (1.12.5-x86_64-linux)
+      racc (~> 1.4)
     parallel (1.21.0)
     parser (3.0.3.2)
       ast (~> 2.4.1)
@@ -197,6 +206,8 @@ PLATFORMS
 DEPENDENCIES
   bootsnap
   dotenv-rails
+  factory_bot_rails
+  faker
   importmap-rails
   jbuilder
   pg (~> 1.1)
diff --git a/spec/factories/project.rb b/spec/factories/project.rb
new file mode 100644
index 000000000..2ed4f59e1
--- /dev/null
+++ b/spec/factories/project.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+    factory :project do
+        user_id { rand( 10 ** 10 ) }
+        name { Faker::Book.title }
+        identifier { Faker::Verb.base(number: 3).join('-') }
+        project_type { %w[python, html].sample }
+    end
+end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 5df403ad8..34adf50ae 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -4,4 +4,9 @@
   describe 'associations' do
     it { is_expected.to have_many(:components) }
   end
+
+  context 'not null' do
+    subject { Project.new }
+    it { should validate_uniqueness_of(:identifier) }
+  end
 end

From 29ab9922b3df704d42ae90c32c02f35987d26cf6 Mon Sep 17 00:00:00 2001
From: Lois Wells <lois.wells@raspberrypi.org>
Date: Tue, 11 Jan 2022 09:45:54 +0000
Subject: [PATCH 05/10] Modifiying the validations and testing

---
 app/models/project.rb       |  1 +
 spec/factories/project.rb   |  2 +-
 spec/models/project_spec.rb | 11 +++++++++--
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index 8a4a19cef..48c1faad7 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1,5 +1,6 @@
 class Project < ApplicationRecord
   before_validation :check_unique_not_null, on: :create
+  validates :identifier, presence: true, uniqueness: true
   has_many :components, dependent: :destroy
 
   require 'phrase_identifier'
diff --git a/spec/factories/project.rb b/spec/factories/project.rb
index 2ed4f59e1..211b2e49d 100644
--- a/spec/factories/project.rb
+++ b/spec/factories/project.rb
@@ -4,7 +4,7 @@
     factory :project do
         user_id { rand( 10 ** 10 ) }
         name { Faker::Book.title }
-        identifier { Faker::Verb.base(number: 3).join('-') }
+        identifier {Faker::Verb.base+'-'+Faker::Verb.base+'-'+Faker::Verb.base}
         project_type { %w[python, html].sample }
     end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 34adf50ae..586197e7e 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'rails_helper'
 
 RSpec.describe Project, type: :model do
@@ -5,8 +7,13 @@
     it { is_expected.to have_many(:components) }
   end
 
-  context 'not null' do
+  describe 'identifier not nil' do
     subject { Project.new }
-    it { should validate_uniqueness_of(:identifier) }
+    it { is_expected.not_to allow_value(nil).for(:identifier)}
+  end
+
+  describe 'identifier unique' do
+    before {FactoryBot.build(:project)}
+    it {should validate_uniqueness_of(:identifier)}
   end
 end

From 34042f666ab619d242b5dbe247cc0694c7b93335 Mon Sep 17 00:00:00 2001
From: Lois Wells <lois.wells@raspberrypi.org>
Date: Tue, 11 Jan 2022 09:46:25 +0000
Subject: [PATCH 06/10] Trying to fix testing

---
 spec/models/project_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 586197e7e..b76347573 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -12,7 +12,7 @@
     it { is_expected.not_to allow_value(nil).for(:identifier)}
   end
 
-  describe 'identifier unique' do
+  context 'identifier unique' do
     before {FactoryBot.build(:project)}
     it {should validate_uniqueness_of(:identifier)}
   end

From 3b9f4c4af8140c84b0a3c878ff366ae96a3e0300 Mon Sep 17 00:00:00 2001
From: Lois Wells <lois.wells@raspberrypi.org>
Date: Tue, 11 Jan 2022 12:23:27 +0000
Subject: [PATCH 07/10] Fixed test for the before validation hook

---
 spec/models/project_spec.rb | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index b76347573..9768facb6 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -8,12 +8,16 @@
   end
 
   describe 'identifier not nil' do
-    subject { Project.new }
+    subject { FactoryBot.build(:project) }
     it { is_expected.not_to allow_value(nil).for(:identifier)}
   end
 
-  context 'identifier unique' do
-    before {FactoryBot.build(:project)}
-    it {should validate_uniqueness_of(:identifier)}
+  describe 'identifier unique' do
+    it do
+      project1=FactoryBot.build(:project)
+      project1.save
+      project2=FactoryBot.build(:project, identifier: project1.identifier)
+      expect {project2.valid?}.to change {project2.identifier}
+    end
   end
 end

From bd6149598b97489083908fc41a358e33a345963a Mon Sep 17 00:00:00 2001
From: Lois Wells <lois.wells@raspberrypi.org>
Date: Tue, 11 Jan 2022 14:16:52 +0000
Subject: [PATCH 08/10] Tidying up

---
 app/models/project.rb       | 4 ++--
 spec/models/project_spec.rb | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index 48c1faad7..2ae323033 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1,10 +1,10 @@
+require 'phrase_identifier'
+
 class Project < ApplicationRecord
   before_validation :check_unique_not_null, on: :create
   validates :identifier, presence: true, uniqueness: true
   has_many :components, dependent: :destroy
 
-  require 'phrase_identifier'
-
   private
     def check_unique_not_null
       self.identifier ||= PhraseIdentifier.generate
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 9768facb6..470e48c94 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -14,8 +14,7 @@
 
   describe 'identifier unique' do
     it do
-      project1=FactoryBot.build(:project)
-      project1.save
+      project1=FactoryBot.create(:project)
       project2=FactoryBot.build(:project, identifier: project1.identifier)
       expect {project2.valid?}.to change {project2.identifier}
     end

From 542ab975e6944bdcd405a87ca0c65f1476df3c9d Mon Sep 17 00:00:00 2001
From: Lois Wells <lois.wells@raspberrypi.org>
Date: Tue, 11 Jan 2022 14:17:18 +0000
Subject: [PATCH 09/10] Moving migration change into separate migration

---
 db/migrate/20211220155302_create_projects.rb                 | 2 +-
 .../20220111141057_make_project_identifier_non_null.rb       | 5 +++++
 db/schema.rb                                                 | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)
 create mode 100644 db/migrate/20220111141057_make_project_identifier_non_null.rb

diff --git a/db/migrate/20211220155302_create_projects.rb b/db/migrate/20211220155302_create_projects.rb
index 48eb5e85a..bb3613ed6 100644
--- a/db/migrate/20211220155302_create_projects.rb
+++ b/db/migrate/20211220155302_create_projects.rb
@@ -3,7 +3,7 @@ def change
     create_table :projects, id: :uuid do |t|
       t.uuid :user_id
       t.string :name
-      t.string :identifier, null: false
+      t.string :identifier
       t.string :project_type, null: false, default: 'python'
 
       t.timestamps
diff --git a/db/migrate/20220111141057_make_project_identifier_non_null.rb b/db/migrate/20220111141057_make_project_identifier_non_null.rb
new file mode 100644
index 000000000..e0360f7df
--- /dev/null
+++ b/db/migrate/20220111141057_make_project_identifier_non_null.rb
@@ -0,0 +1,5 @@
+class MakeProjectIdentifierNonNull < ActiveRecord::Migration[7.0]
+  def change
+    change_column_null(:projects, :identifier, false)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 0a448cba9..43e38b3f7 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_12_21_135519) do
+ActiveRecord::Schema.define(version: 2022_01_11_141057) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"

From effaf9c2a281b9d06b3540c62602b8e45421ce69 Mon Sep 17 00:00:00 2001
From: Lois Wells <lois.wells@raspberrypi.org>
Date: Tue, 11 Jan 2022 14:19:51 +0000
Subject: [PATCH 10/10] Tidying up style

---
 spec/factories/project.rb   | 2 +-
 spec/models/project_spec.rb | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/spec/factories/project.rb b/spec/factories/project.rb
index 211b2e49d..053055160 100644
--- a/spec/factories/project.rb
+++ b/spec/factories/project.rb
@@ -4,7 +4,7 @@
     factory :project do
         user_id { rand( 10 ** 10 ) }
         name { Faker::Book.title }
-        identifier {Faker::Verb.base+'-'+Faker::Verb.base+'-'+Faker::Verb.base}
+        identifier { Faker::Verb.base+'-'+Faker::Verb.base+'-'+Faker::Verb.base }
         project_type { %w[python, html].sample }
     end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 470e48c94..72785e11e 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -9,14 +9,14 @@
 
   describe 'identifier not nil' do
     subject { FactoryBot.build(:project) }
-    it { is_expected.not_to allow_value(nil).for(:identifier)}
+    it { is_expected.not_to allow_value(nil).for(:identifier) }
   end
 
   describe 'identifier unique' do
     it do
-      project1=FactoryBot.create(:project)
-      project2=FactoryBot.build(:project, identifier: project1.identifier)
-      expect {project2.valid?}.to change {project2.identifier}
+      project1 = FactoryBot.create(:project)
+      project2 = FactoryBot.build(:project, identifier: project1.identifier)
+      expect { project2.valid? }.to change { project2.identifier }
     end
   end
 end