Merge pull request #501 from wri/feature/two-levels-of-observation-qc…

…-new-model

Feature/two levels of observation qc new model

Gemfile

@@ -144,6 +144,7 @@ group :test do
   gem "cuprite"
   gem "database_cleaner"
   gem "email_spec"
+  gem "rspec-collection_matchers"
   gem "rspec-activejob"
   gem "rspec-rails"
   gem "rspec-request_snapshot", github: "tsubik/rspec-request_snapshot", branch: "fix/ignore-order"

Gemfile.lock

@@ -573,7 +573,7 @@ GEM
     responders (3.1.1)
       actionpack (>= 5.2)
       railties (>= 5.2)
-    rexml (3.3.4)
+    rexml (3.3.6)
       strscan
     rgeo (3.0.1)
     rgeo-activerecord (7.0.1)
@@ -588,6 +588,8 @@ GEM
     rspec-activejob (0.6.1)
       activejob (>= 4.2)
       rspec-mocks
+    rspec-collection_matchers (1.2.1)
+      rspec-expectations (>= 2.99.0.beta1)
     rspec-core (3.13.0)
       rspec-support (~> 3.13.0)
     rspec-expectations (3.13.0)
@@ -819,6 +821,7 @@ DEPENDENCIES
   rgeo
   rgeo-geojson
   rspec-activejob
+  rspec-collection_matchers
   rspec-rails
   rspec-request_snapshot!
   rspec_api_documentation!

app/admin/observation.rb

@@ -37,12 +37,6 @@ def scoped_collection
     end
   end
 
-  before_action do
-    if %w[POST PATCH PUT].include?(request.method) && action_name != "batch_action"
-      resource.user_type = :admin
-    end
-  end
-
   scope_to do
     Class.new do
       def self.observations
@@ -52,52 +46,18 @@ def self.observations
   end
 
   actions :all, except: [:new]
-  controller do
-    def permitted_params
-      if current_user.user_permission.user_role == "bo_manager"
-        params.permit(observation: [:validation_status, :admin_comment])
-      else
-        params.permit(
-          observation: [
-            :name, :lng, :pv, :lat, :lon, :subcategory_id, :severity_id, :country_id, :operator_id, :user_type,
-            :validation_status, :publication_date, :observation_report_id, :location_information, :evidence_type,
-            :evidence_on_report, :location_accuracy, :law_id, :fmu_id, :hidden, :admin_comment,
-            :monitor_comment, :actions_taken, :is_physical_place, :force_translations_from,
-            relevant_operator_ids: [], government_ids: [],
-            observation_document_ids: [],
-            translations_attributes: [:id, :locale, :details, :concern_opinion, :litigation_status, :_destroy]
-          ]
-        )
-      end
-    end
-  end
 
-  member_action :perform_qc, method: [:put, :get] do
-    @page_title = I18n.t("active_admin.shared.perform_qc")
-    if request.get?
-      if resource.validation_status == "QC in progress"
-        render "perform_qc"
-      else
-        redirect_to collection_path, alert: I18n.t("active_admin.observations_page.not_in_qc_in_progress")
-      end
-    elsif resource.update permitted_params[:observation]
-      redirect_to collection_path, notice: I18n.t("active_admin.observations_page.performed_qc")
-    else
-      render "perform_qc"
-    end
-  end
-
-  member_action :ready_for_publication, method: :put do
-    resource.validation_status = Observation.validation_statuses["Ready for publication"]
-    notice = resource.save ? I18n.t("active_admin.observations_page.moved_ready") : I18n.t("active_admin.observations_page.not_modified")
-    redirect_to collection_path, notice: notice
-  end
+  permit_params :name, :lng, :pv, :lat, :lon, :subcategory_id, :severity_id, :country_id, :operator_id, :user_type,
+    :validation_status, :publication_date, :observation_report_id, :location_information, :evidence_type,
+    :evidence_on_report, :location_accuracy, :law_id, :fmu_id, :hidden,
+    :actions_taken, :is_physical_place, :force_translations_from,
+    relevant_operator_ids: [], government_ids: [],
+    observation_document_ids: [],
+    translations_attributes: [:id, :locale, :details, :concern_opinion, :litigation_status, :_destroy]
 
   member_action :start_qc, method: [:put, :get] do
-    resource.user_type = :admin
-    resource.validation_status = Observation.validation_statuses["QC in progress"]
-    if resource.save
-      redirect_to perform_qc_admin_observation_path(resource), notice: I18n.t("active_admin.observations_page.moved_qc_in_progress")
+    if resource.update(user_type: :reviewer, validation_status: "QC2 in progress")
+      redirect_to new_admin_quality_control_path(quality_control: {reviewable_id: resource.id, reviewable_type: "Observation"}), notice: I18n.t("active_admin.observations_page.moved_qc_in_progress")
     else
       redirect_to collection_path, notice: I18n.t("active_admin.observations_page.not_modified")
     end
@@ -117,44 +77,12 @@ def permitted_params
     end
   end
 
-  action_item :ready_for_publication, only: :show do
-    if resource.validation_status == "QC in progress"
-      link_to I18n.t("active_admin.observations_page.ready_for_publication"), ready_for_publication_admin_observation_path(observation),
-        method: :put, data: {confirm: I18n.t("active_admin.observations_page.confirm_ready_publication")},
-        notice: I18n.t("active_admin.observations_page.approved")
-    end
-  end
-
-  action_item :needs_revision, only: :show do
-    if resource.validation_status == "QC in progress"
-      link_to I18n.t("active_admin.observations_page.needs_revision"), perform_qc_admin_observation_path(observation)
-    end
-  end
-
-  action_item :start_qc, only: :show, if: proc { resource.validation_status == "Ready for QC" } do
+  action_item :start_qc, only: :show, if: proc { resource.validation_status == "Ready for QC2" && resource.responsible_for_qc2.include?(current_user) } do
     link_to I18n.t("active_admin.shared.start_qc"), start_qc_admin_observation_path(observation), method: :put
   end
 
   # Bulk actions should be available only if this env flag is set to `true`
   if ENV.fetch("BULK_EDIT_OBSERVATIONS", "TRUE").upcase == "TRUE"
-    batch_action :move_to_qc_in_progress, confirm: I18n.t("active_admin.observations_page.bulk_confirm_qc") do |ids|
-      batch_action_collection.find(ids).each do |observation|
-        next unless observation.validation_status == "Ready for QC"
-
-        observation.update(validation_status: "QC in progress")
-      end
-      redirect_to collection_path, notice: I18n.t("active_admin.observations_page.qc_started")
-    end
-
-    batch_action :move_to_ready_for_publication, confirm: I18n.t("active_admin.observations_page.bulk_ready_for_publication") do |ids|
-      batch_action_collection.find(ids).each do |observation|
-        next unless observation.validation_status == "QC in progress"
-
-        observation.update(validation_status: "Ready for publication")
-      end
-      redirect_to collection_path, notice: I18n.t("active_admin.observations_page.ready_to_publish")
-    end
-
     batch_action :hide, confirm: I18n.t("active_admin.observations_page.bulk_hide") do |ids|
       batch_action_collection.find(ids).each do |observation|
         observation.update(hidden: true)
@@ -170,7 +98,7 @@ def permitted_params
     end
   end
 
-  sidebar I18n.t("active_admin.operator_page.documents"), only: [:show, :perform_qc] do
+  sidebar I18n.t("active_admin.operator_page.documents"), only: [:show] do
     attributes_table_for resource do
       ul do
         resource.observation_documents.collect do |od|
@@ -319,7 +247,6 @@ def permitted_params
     column I18n.t("document_types.Report") do |observation|
       observation.observation_report&.title
     end
-    column :admin_comment
     column :monitor_comment
     column I18n.t("activerecord.models.user") do |observation|
       observation.user&.name
@@ -408,7 +335,6 @@ def permitted_params
       title = o.observation_report.title[0..100] + ((o.observation_report.title.length >= 100) ? "..." : "") if o.observation_report&.title
       link_to title, admin_observation_report_path(o.observation_report_id) if o.observation_report.present?
     end
-    column :admin_comment, sortable: false
     column :monitor_comment, sortable: false
     column :user, sortable: false
     column :modified_user, sortable: false
@@ -419,9 +345,10 @@ def permitted_params
     column :updated_at
     column :deleted_at
     column(I18n.t("active_admin.shared.actions")) do |observation|
-      a I18n.t("active_admin.shared.start_qc"), href: start_qc_admin_observation_path(observation), "data-method": :put if observation.validation_status == "Ready for QC"
-      a I18n.t("active_admin.observations_page.needs_revision"), href: perform_qc_admin_observation_path(observation) if observation.validation_status == "QC in progress"
-      a I18n.t("active_admin.observations_page.ready_to_publish"), href: ready_for_publication_admin_observation_path(observation), "data-method": :put if observation.validation_status == "QC in progress"
+      if observation.responsible_for_qc2.include? current_user
+        a I18n.t("active_admin.shared.start_qc"), href: start_qc_admin_observation_path(observation), "data-method": :put if observation.validation_status == "Ready for QC2"
+        a I18n.t("active_admin.shared.start_qc"), href: new_admin_quality_control_path(quality_control: {reviewable_id: observation.id, reviewable_type: "Observation"}) if observation.validation_status == "QC2 in progress"
+      end
     end
     actions
 
@@ -467,7 +394,6 @@ def permitted_params
             ["is_physical_place", I18n.t("activerecord.attributes.observation.is_physical_place"), :checked],
             ["litigation_status", I18n.t("activerecord.attributes.observation/translation.litigation_status"), :checked],
             ["report", I18n.t("document_types.Report"), :checked],
-            ["admin_comment", I18n.t("activerecord.attributes.observation.admin_comment"), :checked],
             ["monitor_comment", I18n.t("activerecord.attributes.observation.monitor_comment"), :checked],
             ["user", I18n.t("activerecord.attributes.observation.user"), :checked],
             ["modified_user", Observation.human_attribute_name(:modified_user), :checked],
@@ -491,12 +417,7 @@ def permitted_params
     f.inputs I18n.t("shared.status") do
       f.input :is_active, input_html: {disabled: true}
       f.input :hidden, **visibility
-      if Observation::STATUS_TRANSITIONS[:admin].key?(f.object.validation_status)
-        valid_statuses = [Observation::STATUS_TRANSITIONS[:admin][f.object.validation_status], f.object.validation_status].flatten
-        f.input :validation_status, collection: valid_statuses
-      else
-        f.input :validation_status, {input_html: {disabled: true}}
-      end
+      f.input :validation_status, input_html: {disabled: true}
     end
     f.inputs I18n.t("active_admin.observations_page.details") do
       f.input :observation_type, input_html: {disabled: true}
@@ -583,8 +504,6 @@ def permitted_params
       f.input :lat, **visibility
       f.input :lng, **visibility
       f.input :actions_taken, **visibility
-      f.input :admin_comment
-      f.input :monitor_comment, **visibility
       f.input :observation_report, as: :select, **visibility
       f.input :evidence_type, as: :select, **visibility
       f.input :evidence_on_report, **visibility
@@ -596,7 +515,7 @@ def permitted_params
 
     f.inputs I18n.t("active_admin.shared.translated_fields") do
       f.input :locale, input_html: {disabled: true}
-      if Observation::PUBLISHED_STATES.include? object.validation_status
+      if f.object.published?
         f.input :force_translations_from, label: I18n.t("active_admin.shared.translate_from"),
           as: :select,
           collection: I18n.available_locales.sort,

app/admin/observer.rb

@@ -12,12 +12,12 @@
 
   controller do
     def scoped_collection
-      end_of_association_chain.includes(:responsible_admin, :countries)
+      end_of_association_chain.includes(:responsible_qc1, :responsible_qc2, :countries)
     end
   end
 
   permit_params :observer_type, :is_active, :logo, :name, :organization_type, :delete_logo,
-    :responsible_admin_id, country_ids: []
+    :responsible_qc1_id, :responsible_qc2_id, country_ids: []
 
   csv do
     column :is_active
@@ -50,7 +50,8 @@ def scoped_collection
       link_to o.logo&.identifier, o.logo&.url if o.logo&.url
     end
     column :name
-    column :responsible_admin
+    column :responsible_qc1
+    column :responsible_qc2
     column :created_at
     column :updated_at
     actions
@@ -83,7 +84,8 @@ def scoped_collection
       row :public_info
       row :observer_type
       row :organization_type
-      row :responsible_admin
+      row :responsible_qc1
+      row :responsible_qc2
       # TODO: Reactivate rubocop and fix this
       # rubocop:disable Rails/OutputSafety
       row :countries do |observer|
@@ -115,7 +117,6 @@ def scoped_collection
     f.inputs I18n.t("active_admin.shared.monitor_details") do
       f.input :name
       f.input :is_active
-      f.input :responsible_admin, as: :select, collection: User.joins(:user_permission).where(user_permissions: {user_role: :admin})
       f.input :countries, collection: Country.with_translations(I18n.locale).order("country_translations.name asc")
       f.input :observer_type, as: :select, collection: %w[Mandated SemiMandated External Government]
       f.input :organization_type, as: :select, collection: ["NGO", "Academic", "Research Institute", "Private Company", "Other"]
@@ -126,6 +127,12 @@ def scoped_collection
         f.input :logo, as: :file
       end
     end
+    unless f.object.new_record?
+      f.inputs "Quality Control" do
+        f.input :responsible_qc1, as: :select, collection: User.with_roles(:ngo_manager).filter_actives
+        f.input :responsible_qc2, as: :select, collection: User.with_roles([:admin, :ngo_manager]).filter_actives
+      end
+    end
     f.inputs I18n.t("activerecord.attributes.observer.public_info") do
       f.input :public_info, input_html: {disabled: true}
       f.input :address, input_html: {disabled: true}

app/admin/quality_control.rb

@@ -0,0 +1,53 @@
+ActiveAdmin.register QualityControl do
+  extend BackRedirectable
+
+  menu false
+
+  actions :new, :create
+
+  permit_params :reviewer_id, :reviewable_id, :reviewable_type, :passed, :comment
+
+  controller do
+    def new
+      super do
+        redirect_to reviewable_path, notice: I18n.t("active_admin.quality_control_page.reviewable_not_in_qc") and return unless resource.reviewable.qc_in_progress?
+      end
+    end
+
+    def create
+      super do |format|
+        redirect_to reviewable_path and return
+      end
+    end
+
+    helper_method :reviewable_path
+    def reviewable_path
+      admin_observation_path(resource.reviewable) if resource.reviewable.is_a?(Observation)
+    end
+  end
+
+  form do |f|
+    f.semantic_errors(*f.object.errors.attribute_names)
+
+    f.hidden_field :reviewer_id, value: current_user.id
+    f.hidden_field :reviewable_id, value: resource.reviewable_id
+    f.hidden_field :reviewable_type, value: resource.reviewable_type
+
+    f.inputs do
+      f.input :passed, as: :radio, collection: resource.reviewable.qc_available_decisions, label: I18n.t("operator_documents.qc_form.decision")
+      f.input :comment, as: :text
+    end
+
+    f.actions do
+      f.action :submit, label: I18n.t("active_admin.submit")
+
+      li class: "cancel" do
+        link_to I18n.t("active_admin.cancel"), reviewable_path
+      end
+    end
+
+    if resource.reviewable_type == "Observation"
+      render partial: "admin/observations/attributes_table", locals: {observation: resource.reviewable}
+    end
+  end
+end

app/admin/user.rb

@@ -7,6 +7,7 @@
   permit_params :email, :password, :password_confirmation, :country_id,
     :name, :first_name, :last_name, :is_active, :organization_account,
     :observer_id, :operator_id, :holding_id, :locale,
+    qc1_observer_ids: [], qc2_observer_ids: [],
     managed_observer_ids: [],
     responsible_for_country_ids: [],
     user_permission_attributes: [:user_role]
@@ -101,7 +102,9 @@ def update
       row :operator if resource.operator?
       row :responsible_for_countries if resource.admin?
       row :observer if resource.ngo? || resource.ngo_manager?
-      row :managed_observers if resource.ngo? || resource.ngo_manager? || resource.admin?
+      # row :managed_observers if resource.ngo? || resource.ngo_manager? || resource.admin?
+      row :qc1_observers if resource.ngo_manager?
+      row :qc2_observers if resource.admin? || resource.ngo_manager?
       row :is_active
       row :locale
       row :country
@@ -120,7 +123,16 @@ def update
         p.input :user_role, as: :select, collection: UserPermission.user_roles.keys, include_blank: false
       end
       f.input :observer
-      f.input :managed_observers
+      # TODO: remove if removing managed_observers
+      # f.input :managed_observers
+      f.input :qc1_observers,
+        as: :select,
+        hint: "You can see the current QC person in parentheses. Setting a new QC person will replace the current one",
+        collection: Observer.left_outer_joins(:responsible_qc1).by_name_asc.map { |o| [o.responsible_qc1.present? ? "#{o.name} (QC: #{o.responsible_qc1.name})" : o.name, o.id] }
+      f.input :qc2_observers,
+        as: :select,
+        hint: "You can see the current QC person in parentheses. Setting a new QC person will replace the current one",
+        collection: Observer.left_outer_joins(:responsible_qc2).by_name_asc.map { |o| [o.responsible_qc2.present? ? "#{o.name} (QC: #{o.responsible_qc2.name})" : o.name, o.id] }
       f.input :operator
       f.input :holding
       f.input :responsible_for_countries, hint: I18n.t("active_admin.users_page.responsible_for_countries_hint"), collection: Country.active.order(:name)

app/assets/javascripts/active_admin.js

@@ -13,6 +13,7 @@
 //= require visibility
 //= require dependent_filters
 //= require sentry
+//= require quality_controls
 
 //= require active_admin/active_admin_globalize
 //= require chartkick