assign observers from observation_report, sync observers for all obse…

…rvations having the same report

app/admin/observation.rb

@@ -63,7 +63,7 @@ def permitted_params
             :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,
-            observer_ids: [], relevant_operator_ids: [], government_ids: [],
+            relevant_operator_ids: [], government_ids: [],
             observation_documents_attributes: [:id, :name, :attachment],
             translations_attributes: [:id, :locale, :details, :concern_opinion, :litigation_status, :_destroy]
           ]
@@ -547,7 +547,7 @@ def permitted_params
 
       f.input :is_physical_place, **visibility
       f.input :location_information, **visibility if f.object.observation_type == "operator"
-      f.input :observers, **visibility
+      f.input :observers, input_html: {disabled: true}
 
       f.input :relevant_operator_ids,
         label: I18n.t("activerecord.attributes.observation.relevant_operators"),

app/admin/observation_report.rb

@@ -8,7 +8,7 @@
 
   actions :all, except: [:new]
 
-  permit_params :user_id, :title, :publication_date, :attachment
+  permit_params :user_id, :title, :publication_date, :attachment, observer_ids: []
 
   config.order_clause
   active_admin_paranoia
@@ -141,9 +141,10 @@ def apply_filtering(chain)
       f.input :title
       f.input :publication_date, as: :date_time_picker, picker_options: {timepicker: false, format: "Y-m-d"}
       f.input :attachment, as: :file, hint: f.object&.attachment&.file&.filename
-
-      f.actions
+      f.input :observers
     end
+
+    f.actions
   end
 
   show do

app/models/observation.rb

@@ -134,20 +134,20 @@ class Observation < ApplicationRecord
 
   validates :admin_comment, presence: true, if: -> { validation_status == "Needs revision" }
 
+  before_validation :assign_observers_from_report, if: :observation_report_changed?
+
   before_save :set_active_status
   before_save :check_is_physical_place
   before_save :set_centroid
   before_save :set_publication_date, if: :validation_status_changed?
   before_create :set_default_observer
 
   after_create :update_operator_scores, if: :is_active?
-  after_create :update_reports_observers
   after_update :update_operator_scores, if: -> { saved_change_to_publication_date? || saved_change_to_severity_id? || saved_change_to_is_active? || saved_change_to_operator_id? || saved_change_to_fmu_id? }
 
   after_destroy :update_operator_scores
   after_destroy :update_fmu_geojson
 
-  after_save :update_reports_observers, if: :saved_change_to_observation_report_id?
   after_save :create_history, if: :saved_changes?
 
   after_save :remove_documents, if: -> { evidence_type == "Evidence presented in the report" }
@@ -197,12 +197,6 @@ def cache_key
     super + "-" + Globalize.locale.to_s
   end
 
-  def update_reports_observers
-    return if observation_report.blank?
-
-    observation_report.update_observers
-  end
-
   HISTORICAL_ATTRIBUTES = %w[fmu_id operator_id country_id subcategory_id observation_type evidence_type location_accuracy validation_status is_active hidden deleted_at]
 
   # Creates an ObservationHistory for the current Observation
@@ -323,6 +317,13 @@ def update_fmu_geojson
     fmu.save
   end
 
+  def assign_observers_from_report
+    return if observation_report.nil?
+    return if observation_report.observers.empty?
+
+    self.observers = observation_report.observers
+  end
+
   def notify_about_creation
     return unless validation_status == "Created"
 

app/models/observation_report.rb

@@ -25,17 +25,27 @@ class ObservationReport < ApplicationRecord
   # API creating report fails as it's not providing user. Thing to investigate
   belongs_to :user, inverse_of: :observation_reports, optional: true
   has_many :observation_report_observers, dependent: :destroy
-  has_many :observers, through: :observation_report_observers
+  has_many :observers, through: :observation_report_observers, after_add: :self_touch, after_remove: :self_touch
   has_many :observations, dependent: :destroy
 
   skip_callback :commit, :after, :remove_attachment!
   after_destroy :move_attachment_to_private_directory
   after_restore :move_attachment_to_public_directory
   after_real_destroy :remove_attachment!
 
+  after_commit :sync_observation_observers
+
   scope :bigger_date, ->(date) { where("observation_reports.created_at <= ?", date + 1.day) }
 
-  def update_observers
-    self.observer_ids = observations.joins(:observers).distinct.pluck("observers.id") if observations.any?
+  private
+
+  def self_touch(_)
+    touch unless destroyed? || new_record?
+  end
+
+  def sync_observation_observers
+    observations.with_deleted.find_each do |observation|
+      observation.observers = observers
+    end
   end
 end

app/resources/v1/observation_report_resource.rb

@@ -9,16 +9,10 @@ class ObservationReportResource < BaseResource
     has_many :observers
     has_many :observations
 
-    after_create :add_observers
-
     filter :observer_id, apply: ->(records, value, _options) {
       records.where(id: ObservationReportObserver.where(observer_id: value[0].to_i).select(:observation_report_id))
     }
 
-    def add_observers
-      @model.update_observers if @model.observations.any?
-    end
-
     def fetchable_fields
       return super if observations_tool_user?
 

spec/acceptance/fmus_spec.rb

@@ -34,7 +34,7 @@
 
     before do
       create(:fmu_operator, fmu: fmu_with_obs, operator: operator)
-      create(:observation, operator: operator, fmu: fmu_with_obs, observers: [observer])
+      create(:observation, operator: operator, fmu: fmu_with_obs, observation_report: create(:observation_report, observers: [observer]))
     end
 
     context "200" do

spec/acceptance/operators_spec.rb

@@ -33,7 +33,7 @@
     let(:operator_with_obs) { create(:operator, country: country) }
 
     before do
-      create(:observation, operator: operator_with_obs, observers: [observer])
+      create(:observation, operator: operator_with_obs, observation_report: create(:observation_report, observers: [observer]))
     end
 
     example_request "Listing operators" do

spec/factories/observation_reports.rb

@@ -17,9 +17,11 @@
     sequence(:title) { |n| "ObservationReportTitle#{n}" }
     publication_date { DateTime.current }
     attachment { Rack::Test::UploadedFile.new(File.join(Rails.root, "spec", "support", "files", "image.png")) }
+    observers { build_list(:observer, 1) }
 
     after(:build) do |random_observation_report|
       random_observation_report.user ||= FactoryBot.create(:user)
+      random_observation_report.observers.each { |observer| observer.translation.name = observer.name }
     end
   end
 end

spec/factories/observations.rb

@@ -38,45 +38,6 @@
 #
 
 FactoryBot.define do
-  factory :observation_1, class: "Observation" do
-    severity
-    country
-    species { build_list(:species, 1) }
-    user { build(:admin) }
-    operator { build(:operator, name: "Operator #{Faker::Lorem.sentence}") }
-    observers { build_list(:observer, 1) }
-    observation_type { "operator" }
-    is_active { true }
-    evidence_type { "Photos" }
-    publication_date { DateTime.now.to_date }
-    location_accuracy { "Estimated location" }
-    lng { 12.2222 }
-    lat { 12.3333 }
-
-    after(:build) do |observation|
-      observation.observers.each { |observer| observer.translation.name = observer.name }
-    end
-  end
-
-  factory :gov_observation, class: "Observation" do
-    severity
-    country
-    governments { build_list(:government, 2) }
-    species { build_list(:species, 1, name: "Species #{Faker::Lorem.sentence}") }
-    observers { build_list(:observer, 1) }
-    user { build(:admin) }
-    observation_type { "government" }
-    validation_status { "Published (no comments)" }
-    is_active { true }
-    publication_date { DateTime.now.yesterday.to_date }
-    lng { 12.2222 }
-    lat { 12.3333 }
-
-    after(:build) do |observation|
-      observation.observers.each { |observer| observer.translation.name = observer.name }
-    end
-  end
-
   factory :observation, class: "Observation" do
     country
     subcategory
@@ -86,10 +47,10 @@
     severity { build(:severity, subcategory: subcategory) }
     operator { create(:operator, country: country) }
     observation_type { "operator" }
-    observers { build_list(:observer, 1) }
-    species { build_list(:species, 1, name: "Species #{Faker::Lorem.sentence}") }
+    # species { build_list(:species, 1, name: "Species #{Faker::Lorem.sentence}") }
     is_active { true }
     validation_status { "Published (no comments)" }
+    is_physical_place { true }
     lng { 12.2222 }
     lat { 12.3333 }
 
@@ -110,6 +71,23 @@
     end
   end
 
+  factory :gov_observation, class: "Observation" do
+    severity
+    country
+    governments { build_list(:government, 2) }
+    # species { build_list(:species, 1, name: "Species #{Faker::Lorem.sentence}") }
+    observers { build_list(:observer, 1) }
+    user { build(:admin) }
+    observation_type { "government" }
+    validation_status { "Published (no comments)" }
+    is_active { true }
+    publication_date { DateTime.now.yesterday.to_date }
+
+    after(:build) do |observation|
+      observation.observers.each { |observer| observer.translation.name = observer.name }
+    end
+  end
+
   trait :with_translations do
     details { "details" }
     concern_opinion { "concern opinion" }

spec/integration/v1/observations_spec.rb

@@ -19,15 +19,16 @@ module V1
     let(:ngo_headers) { authorize_headers(ngo.id) }
     let!(:country) { create(:country) }
 
-    let(:observation) { create(:observation_1) }
+    let(:observation) { create(:observation) }
 
     context "Show observations" do
       before do
         poland = create(:country, name: "Poland", iso: "POL")
         spain = create(:country, name: "Spain", iso: "ESP")
         create_list(:observation, 4, country: poland)
-        create(:observation, observers: [ngo_observer], user_id: ngo.id, country: spain)
-        create(:observation, observers: [ngo_observer], user_id: ngo.id, country: spain, fmu: create(:fmu))
+        report = create(:observation_report, observers: [ngo_observer])
+        create(:observation, observation_report: report, user_id: ngo.id, country: spain)
+        create(:observation, observation_report: report, user_id: ngo.id, country: spain, fmu: create(:fmu))
         create(:gov_observation, user_id: admin.id, country: poland)
       end
 
@@ -117,7 +118,7 @@ module V1
     end
 
     context "Edit observations" do
-      let(:observation) { create(:observation_1) }
+      let(:observation) { create(:created_observation) }
 
       describe "For admin user" do
         it "Returns error object when the observation cannot be updated by admin" do
@@ -162,7 +163,7 @@ module V1
       end
 
       describe "For not admin user" do
-        let(:observation_by_user) { create(:observation_1, validation_status: "Published (no comments)", user_id: ngo.id) }
+        let(:observation_by_user) { create(:observation, validation_status: "Published (no comments)", user_id: ngo.id) }
 
         it "Do not allows to update observation by not admin user" do
           patch("/observations/#{observation.id}?app=observations-tool",

spec/integration/v1/operators_spec.rb

@@ -64,7 +64,7 @@ module V1
 
         before do
           create_list(:operator, 3, country: country) # those should not be returned
-          create(:observation, operator: operator, observers: [observer])
+          create(:observation, operator: operator, observation_report: create(:observation_report, observers: [observer]))
         end
 
         it "returns only operators linked with observer observations" do

spec/models/observation_report_spec.rb

@@ -21,6 +21,38 @@
     expect(subject).to be_valid
   end
 
+  describe "hooks" do
+    describe "sync_observation_observers" do
+      let!(:observer) { create(:observer) }
+
+      context "when adding observer to report" do
+        let!(:report) { create(:observation_report) }
+        let!(:observation) { create(:observation, observation_report: report) }
+
+        before do
+          report.observers << observer
+        end
+
+        it "adds observer to observation" do
+          expect(observation.reload.observers).to include(observer)
+        end
+      end
+
+      context "when removing observer from report" do
+        let!(:report) { create(:observation_report, observers: [observer]) }
+        let!(:observation) { create(:observation, observers: [observer], observation_report: report) }
+
+        before do
+          report.observers.delete(observer)
+        end
+
+        it "removes observer from observation" do
+          expect(observation.reload.observers).not_to include(observer)
+        end
+      end
+    end
+  end
+
   describe "soft delete" do
     let!(:report) { create(:observation_report) }