Merge pull request #4671 from okriuchykhin/ok_SCI_7513

Add versioning implementation to protocols data layer [SCI-7513]

app/models/protocol.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 class Protocol < ApplicationRecord
+  include ArchivableModel
   include SearchableModel
   include RenamingUtil
   include SearchableByNameModel
@@ -8,6 +9,7 @@ class Protocol < ApplicationRecord
   include PermissionCheckableModel
   include TinyMceImages
 
+  before_validation :assign_version_number, on: :update, if: -> { protocol_type_changed? && in_repository_published? }
   after_update :update_user_assignments, if: -> { saved_change_to_protocol_type? && in_repository? }
   after_destroy :decrement_linked_children
   after_save :update_linked_children
@@ -16,9 +18,8 @@ class Protocol < ApplicationRecord
   enum protocol_type: {
     unlinked: 0,
     linked: 1,
-    in_repository_private: 2,
+    in_repository_published: 2,
-    in_repository_public: 3,
+    in_repository_draft: 3
-    in_repository_archived: 4
   }
 
   auto_strip_attributes :name, :description, nullify: false
@@ -27,63 +28,54 @@ class Protocol < ApplicationRecord
   validates :description, length: { maximum: Constants::RICH_TEXT_MAX_LENGTH }
   validates :team, presence: true
   validates :protocol_type, presence: true
+  validate :prevent_update, on: :update, if: :in_repository_published?
+  # Only one draft can exist for each protocol
+  validates :previous_version_id, uniqueness: true, if: -> { in_repository_draft? && previous_version_id.present? }
 
-  with_options if: :in_module? do |protocol|
+  with_options if: :in_module? do
-    protocol.validates :my_module, presence: true
+    validates :my_module, presence: true
-    protocol.validates :published_on, absence: true
+    validates :archived_by, absence: true
-    protocol.validates :archived_by, absence: true
+    validates :archived_on, absence: true
-    protocol.validates :archived_on, absence: true
   end
-  with_options if: :linked? do |protocol|
+  with_options if: :linked? do
-    protocol.validates :added_by, presence: true
+    validate :parent_type_constrain
-    protocol.validates :parent, presence: true
+    validates :added_by, presence: true
-    protocol.validates :parent_updated_at, presence: true
+    validates :parent, presence: true
+    validates :parent_updated_at, presence: true
   end
-
+  with_options if: :in_repository? do
-  with_options if: :in_repository? do |protocol|
+    validates :name, presence: true
-    protocol.validates :name, presence: true
+    validate :versions_same_name_constrain
-    protocol.validates :added_by, presence: true
+    validates :added_by, presence: true
-    protocol.validates :my_module, absence: true
+    validates :my_module, absence: true
-    protocol.validates :parent, absence: true
+    validates :parent, absence: true
-    protocol.validates :parent_updated_at, absence: true
+    validates :parent_updated_at, absence: true
   end
-  with_options if: :in_repository_public? do |protocol|
+  with_options if: -> { in_repository? && active? && !previous_version } do |protocol|
-    # Public protocol must have unique name inside its team
+    # Active protocol must have unique name inside its team
     protocol
       .validates_uniqueness_of :name, case_sensitive: false,
                                scope: :team,
                                conditions: lambda {
-                                 where(
+                                 active.where(
-                                   protocol_type:
+                                   protocol_type: [
-                                     Protocol
+                                     Protocol.protocol_types[:in_repository_published],
-                                      .protocol_types[:in_repository_public]
+                                     Protocol.protocol_types[:in_repository_draft]
-                                 )
+                                   ]
-                               }
-    protocol.validates :published_on, presence: true
-  end
-  with_options if: :in_repository_private? do |protocol|
-    # Private protocol must have unique name inside its team & user scope
-    protocol
-      .validates_uniqueness_of :name, case_sensitive: false,
-                               scope: %i(team added_by),
-                               conditions: lambda {
-                                 where(
-                                   protocol_type:
-                                     Protocol
-                                       .protocol_types[:in_repository_private]
                                  )
                                }
   end
-  with_options if: :in_repository_archived? do |protocol|
+  with_options if: -> { in_repository? && archived? && !previous_version } do |protocol|
-    # Archived protocol must have unique name inside its team & user scope
+    # Archived protocol must have unique name inside its team
     protocol
       .validates_uniqueness_of :name, case_sensitive: false,
-                               scope: %i(team added_by),
+                               scope: :team,
                                conditions: lambda {
-                                 where(
+                                 archived.where(
-                                   protocol_type:
+                                   protocol_type: [
-                                    Protocol
+                                     Protocol.protocol_types[:in_repository_published],
-                                      .protocol_types[:in_repository_archived]
+                                     Protocol.protocol_types[:in_repository_draft]
+                                   ]
                                  )
                                }
     protocol.validates :archived_by, presence: true
@@ -91,7 +83,6 @@ class Protocol < ApplicationRecord
   end
 
   belongs_to :added_by,
-             foreign_key: 'added_by_id',
              class_name: 'User',
              inverse_of: :added_protocols,
              optional: true
@@ -99,21 +90,27 @@ class Protocol < ApplicationRecord
              inverse_of: :protocols,
              optional: true
   belongs_to :team, inverse_of: :protocols
+  belongs_to :previous_version,
+             class_name: 'Protocol',
+             inverse_of: :next_version,
+             optional: true
   belongs_to :parent,
-             foreign_key: 'parent_id',
              class_name: 'Protocol',
              optional: true
   belongs_to :archived_by,
-             foreign_key: 'archived_by_id',
              class_name: 'User',
              inverse_of: :archived_protocols, optional: true
   belongs_to :restored_by,
-             foreign_key: 'restored_by_id',
              class_name: 'User',
              inverse_of: :restored_protocols, optional: true
   has_many :linked_children,
            class_name: 'Protocol',
            foreign_key: 'parent_id'
+  has_one  :next_version,
+           class_name: 'Protocol',
+           foreign_key: 'previous_version_id',
+           inverse_of: :previous_version,
+           dependent: :destroy
   has_many :protocol_protocol_keywords,
            inverse_of: :protocol,
            dependent: :destroy
@@ -288,11 +285,11 @@ class Protocol < ApplicationRecord
   end
 
   def in_repository_active?
-    in_repository_private? || in_repository_public?
+    in_repository && active?
   end
 
   def in_repository?
-    in_repository_active? || in_repository_archived?
+    in_repository_published? || in_repository_draft?
   end
 
   def in_module?
@@ -717,4 +714,32 @@ class Protocol < ApplicationRecord
   def decrement_linked_children
     parent.decrement!(:nr_of_linked_children) if parent.present?
   end
+
+  def assign_version_number
+    return if previous_version.blank?
+
+    self.version_number = previous_version.version_number + 1
+  end
+
+  def prevent_update
+    errors.add(:base, I18n.t('activerecord.errors.models.protocol.unchangable'))
+  end
+
+  def parent_type_constrain
+    unless parent.in_repository_published?
+      errors.add(:base, I18n.t('activerecord.errors.models.protocol.wrong_parent_type'))
+    end
+  end
+
+  def versions_same_name_constrain
+    if previous_version.present? && !previous_version.name.eql?(name)
+      errors.add(:base, I18n.t('activerecord.errors.models.protocol.wrong_version_name'))
+    end
+  end
+
+  def version_number_constrain
+    if previous_version.present? && version_number != previous_version.version_number + 1
+      errors.add(:base, I18n.t('activerecord.errors.models.protocol.wrong_version_number'))
+    end
+  end
 end

config/locales/en.yml

@@ -143,6 +143,10 @@ en:
             shared_object_id:
               is_globally_shared: "Inventory is already globally shared"
         protocol:
+          unchangable: "Published protocols can not be changed!"
+          wrong_parent_type: "Protocol can only be linked to published protocol!"
+          wrong_version_name: "Protocol versions should have same name!"
+          wrong_version_number: "Protocol version number should be sequential!"
           attributes:
             step_order:
               invalid: "Invalid step order."

db/migrate/20160421100400_add_published_on_to_protocols.rb

@@ -2,7 +2,7 @@ class AddPublishedOnToProtocols < ActiveRecord::Migration[4.2]
   def up
     add_column :protocols, :published_on, :datetime
 
-    Protocol.where(protocol_type: :in_repository_public).find_each do |p|
+    Protocol.where(protocol_type: :in_repository_published).find_each do |p|
       p.update_column(:published_on, p.created_at)
     end
   end

db/migrate/20221125133611_add_protocol_versioning.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+class AddProtocolVersioning < ActiveRecord::Migration[6.1]
+  def up
+    change_table :protocols, bulk: true do |t|
+      t.boolean :archived, default: false, null: false, index: true
+      t.integer :version_number, default: 1
+    end
+    add_reference :protocols, :previous_version, index: true, foreign_key: { to_table: :protocols }
+    execute(
+      'UPDATE "protocols" SET "archived" = TRUE WHERE "protocols"."protocol_type" = 4;'
+    )
+    execute(
+      'UPDATE "protocols" SET "protocol_type" = 2 WHERE "protocols"."protocol_type" IN (3, 4);'
+    )
+  end
+
+  def down
+    execute(
+      'UPDATE "protocols" SET "protocol_type" = 4 WHERE "protocols"."protocol_type" = 2 AND '\
+      '"protocols"."archived" = TRUE;'
+    )
+    remove_reference :protocols, :previous_version, index: true, foreign_key: { to_table: :protocols }
+    change_table :protocols, bulk: true do |t|
+      t.remove :version_number
+      t.remove :archived
+    end
+  end
+end

db/structure.sql

@@ -50,8 +50,6 @@ CREATE FUNCTION public.trim_html_tags(input text, OUT output text) RETURNS text
 
 SET default_tablespace = '';
 
-SET default_table_access_method = heap;
-
 --
 -- Name: active_storage_attachments; Type: TABLE; Schema: public; Owner: -
 --
@@ -710,7 +708,6 @@ CREATE TABLE public.label_templates (
     type character varying,
     width_mm double precision,
     height_mm double precision,
-    height_mm double precision,
     unit integer DEFAULT 0,
     density integer DEFAULT 12
 );
@@ -1374,7 +1371,10 @@ CREATE TABLE public.protocols (
     created_at timestamp without time zone NOT NULL,
     updated_at timestamp without time zone NOT NULL,
     published_on timestamp without time zone,
-    nr_of_linked_children integer DEFAULT 0
+    nr_of_linked_children integer DEFAULT 0,
+    archived boolean DEFAULT false NOT NULL,
+    version_number integer DEFAULT 1,
+    previous_version_id bigint
 );
 
 
@@ -5561,6 +5561,13 @@ CREATE INDEX index_protocol_protocol_keywords_on_protocol_keyword_id ON public.p
 CREATE INDEX index_protocols_on_added_by_id ON public.protocols USING btree (added_by_id);
 
 
+--
+-- Name: index_protocols_on_archived; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX index_protocols_on_archived ON public.protocols USING btree (archived);
+
+
 --
 -- Name: index_protocols_on_archived_by_id; Type: INDEX; Schema: public; Owner: -
 --
@@ -5603,6 +5610,13 @@ CREATE INDEX index_protocols_on_name ON public.protocols USING gin (public.trim_
 CREATE INDEX index_protocols_on_parent_id ON public.protocols USING btree (parent_id);
 
 
+--
+-- Name: index_protocols_on_previous_version_id; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX index_protocols_on_previous_version_id ON public.protocols USING btree (previous_version_id);
+
+
 --
 -- Name: index_protocols_on_protocol_type; Type: INDEX; Schema: public; Owner: -
 --
@@ -7867,6 +7881,14 @@ ALTER TABLE ONLY public.repository_list_items
     ADD CONSTRAINT fk_rails_ace46bca57 FOREIGN KEY (repository_column_id) REFERENCES public.repository_columns(id);
 
 
+--
+-- Name: protocols fk_rails_ae930efae7; Type: FK CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY public.protocols
+    ADD CONSTRAINT fk_rails_ae930efae7 FOREIGN KEY (previous_version_id) REFERENCES public.protocols(id);
+
+
 --
 -- Name: my_module_statuses fk_rails_b024d15104; Type: FK CONSTRAINT; Schema: public; Owner: -
 --
@@ -8568,6 +8590,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20220803122405'),
 ('20220818094636'),
 ('20220914124900'),
-('20221007113010');
+('20221007113010'),
+('20221125133611');