More server side validations for observation model

app/admin/observation.rb

@@ -51,7 +51,7 @@ def scoped_collection
     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")
+      redirect_back_or_to collection_path, alert: I18n.t("active_admin.observations_page.not_modified")
     end
   end
 

app/models/observation.rb

@@ -172,6 +172,13 @@ class Observation < ApplicationRecord
 
   validates :validation_status, presence: true
 
+  with_options unless: -> { Created? || Rejected? } do
+    validates :subcategory, presence: true
+    validates :severity, presence: true
+    validates :observation_report, presence: true
+    validates :evidence_type, presence: true
+  end
+
   before_validation :assign_observers_from_report, if: :observation_report_changed?
   before_validation :nullify_evidence_on_report, if: -> { evidence_type != "Evidence presented in the report" }
 

app/views/admin/observations/_attributes_table.html.arb

@@ -35,7 +35,14 @@ panel I18n.t("active_admin.observations_page.details") do
     row :litigation_status
     row :actions_taken
     row :concern_opinion
-    row :observation_report
+    row :observation_report do |o|
+      report = ObservationReport.with_deleted.find_by(id: o.observation_report_id)
+      if report
+        text = report.title
+        text = "#{text} (#{I18n.t("active_admin.shared.deleted")})" if report.deleted?
+        link_to text, admin_observation_report_path(report)
+      end
+    end
     row :qc1_needed?
     row :admin_comment_deprecated, &:admin_comment
     row :monitor_comment

lib/tasks/observations.rake

@@ -96,4 +96,119 @@ namespace :observations do
     end
     puts "Done :)"
   end
+
+  desc "Reject invalid observations that don't meet validation requirements"
+  task reject_invalid_observations: :environment do
+    for_real = ENV["FOR_REAL"] == "true"
+
+    puts for_real ? "RUNNING FOR REAL" : "DRY RUN MODE - No changes will be made"
+    puts "=" * 80
+
+    # Find a system user to use as reviewer for QualityControl records
+    system_user = User.find_by(email: ENV["RESPONSIBLE_EMAIL"]&.downcase)
+
+    if system_user.nil?
+      puts "ERROR: No user found to use as reviewer. Cannot proceed."
+      exit 1
+    end
+
+    puts "Using user #{system_user.email} as reviewer for QualityControl records"
+    puts
+
+    observations_to_check = Observation.where.not(validation_status: ["Created", "Rejected"])
+
+    total_checked = 0
+    invalid_count = 0
+    error_count = 0
+
+    users_getting_emails = {}
+
+    # Skip QualityControl callback to prevent automatic status update
+    # We'll update the status manually after creating the QC record
+    QualityControl.skip_callback(:create, :after, :update_reviewable_qc_status) if for_real
+
+    begin
+      observations_to_check.find_each do |observation|
+        total_checked += 1
+
+        # Check if observation is valid
+        next if observation.valid?
+
+        invalid_count += 1
+
+        # Get observer's locale for error messages
+        # Use first observer's first user's locale, or default locale
+        observer_user = observation.observers.first&.users&.first
+        observer_locale = observer_user&.locale || I18n.default_locale
+
+        error_message = I18n.with_locale(observer_locale) do
+          observation.errors.full_messages.join("; ")
+        end
+        if observer_locale != "en"
+          error_message += " (EN: #{I18n.with_locale(:en) { observation.errors.full_messages.join("; ") }})"
+        end
+        if observer_locale != "fr"
+          error_message += " (FR: #{I18n.with_locale(:fr) { observation.errors.full_messages.join("; ") }})"
+        end
+
+        skip_email_notification = false
+
+        if observation.validation_status != "Needs revision"
+          User.where(id: observation.observers.joins(:users).distinct.pluck("users.id")).pluck(:email).uniq.each do |email|
+            users_getting_emails[email] ||= 0
+            users_getting_emails[email] += 1
+          end
+        else
+          # if observation already marked as Needs revision, append last QC comment to provide more context
+          last_qc_message = observation.latest_quality_control&.comment
+          error_message = "#{last_qc_message} \n #{error_message}" if last_qc_message.present?
+          skip_email_notification = true
+        end
+
+        puts "Observation ##{observation.id} (status: #{observation.validation_status}) is invalid:"
+        puts "  Observer locale: #{observer_locale}"
+        puts "  Message: #{error_message}"
+
+
+        if for_real
+          ActiveRecord::Base.transaction do
+            Observation.skip_callback(:commit, :after, :notify_about_changes) if skip_email_notification
+
+            qc = QualityControl.create!(
+              reviewable: observation,
+              reviewer: system_user,
+              passed: false,
+              comment: error_message
+            )
+            observation.update!(validation_status: "Rejected")
+
+            puts "  ✓ Created QualityControl ##{qc.id} and rejected observation"
+          rescue => e
+            error_count += 1
+            puts "  ✗ Error: #{e.message}"
+            puts "    #{e.backtrace.first}" if ENV["DEBUG"]
+          ensure
+            Observation.set_callback(:commit, :after, :notify_about_changes) if skip_email_notification
+          end
+        end
+
+        puts
+      end
+    ensure
+      # Re-enable callback
+      QualityControl.set_callback(:create, :after, :update_reviewable_qc_status) if for_real
+    end
+
+    puts users_getting_emails.empty? ? "No users to notify." : "Users to be notified:"
+    users_getting_emails.each do |email, count|
+      puts "  #{email}: #{count} emails sent"
+    end
+
+    puts "=" * 80
+    puts "Summary:"
+    puts "  Total observations checked: #{total_checked}"
+    puts "  Invalid observations found: #{invalid_count}"
+    puts "  Successfully rejected: #{invalid_count - error_count}" if for_real
+    puts "  Errors encountered: #{error_count}" if error_count > 0 && for_real
+  end
 end

spec/factories/observations.rb

@@ -48,12 +48,13 @@
     subcategory
     observation_report
     law
-    user { build(:admin) }
+    user { build(:ngo_manager) }
     severity { build(:severity, subcategory: subcategory) }
     operator { create(:operator, country: country) }
     observation_type { "operator" }
     is_active { true }
     validation_status { "Published (no comments)" }
+    evidence_type { "No evidence" }
     is_physical_place { true }
     non_concession_activity { false }
     lng { 12.2222 }
@@ -67,19 +68,26 @@
       validation_status { "Created" }
     end
 
-    after(:create) do |doc, evaluator|
-      doc.update(validation_status: evaluator.force_status) if evaluator.force_status
+    after(:build) do |observation|
+      # TODO: maybe that should be in the model?
+      observation.observers = [observation.user.observer] if observation.observers.empty? || observation.observation_report.nil?
+    end
+
+    after(:create) do |observation, evaluator|
+      observation.update(validation_status: evaluator.force_status) if evaluator.force_status
     end
   end
 
   factory :gov_observation, class: "Observation" do
-    severity
     country
+    subcategory { build(:subcategory) }
+    severity { build(:severity, subcategory: subcategory) }
+    observation_report { build(:observation_report) }
     governments { build_list(:government, 2) }
-    observers { build_list(:observer, 1) }
     user { build(:admin) }
     observation_type { "government" }
     validation_status { "Published (no comments)" }
+    evidence_type { "No evidence" }
     is_active { true }
     publication_date { DateTime.now.yesterday.to_date }
   end

spec/factories/users.rb

@@ -61,7 +61,7 @@
     end
 
     factory :ngo_manager do
-      sequence(:email) { |n| "ngo#{n}@example.com" }
+      sequence(:email) { |n| "ngo_manager#{n}@example.com" }
 
       first_name { "Test" }
       last_name { "ngo manager" }

spec/integration/v1/governments_spec.rb

@@ -50,7 +50,7 @@ module V1
 
         before do
           create_list(:government, 3, country: country) # those should not be returned
-          create(:gov_observation, observers: [observer], governments: [gov])
+          create(:gov_observation, observation_report: build(:observation_report, observers: [observer]), governments: [gov])
         end
 
         it "returns only governments linked with observer observations" do

spec/models/observation_spec.rb

@@ -267,6 +267,52 @@
         end
       end
     end
+
+    describe "government observation validations" do
+      it "should be invalid when government observation has operator_id" do
+        country = create(:country)
+        observation = build(:gov_observation, country: country)
+        observation.operator = create(:operator, country: country)
+
+        expect(observation.valid?).to eq(false)
+        expect(observation.errors[:operator_id]).to include("must be blank")
+      end
+    end
+
+    describe "required fields for non-Created and non-Rejected statuses" do
+      it "allows missing required fields when status is Created or Rejected" do
+        created_obs = build(:observation,
+          validation_status: "Created",
+          subcategory: nil,
+          severity: nil,
+          observation_report: nil,
+          evidence_type: nil)
+        expect(created_obs).to be_valid
+
+        rejected_obs = build(:observation,
+          validation_status: "Rejected",
+          subcategory: nil,
+          severity: nil,
+          observation_report: nil,
+          evidence_type: nil)
+        expect(rejected_obs).to be_valid
+      end
+
+      it "requires subcategory, severity, observation_report, and evidence_type for other statuses" do
+        observation = build(:observation,
+          validation_status: "Ready for QC2",
+          subcategory: nil,
+          severity: nil,
+          observation_report: nil,
+          evidence_type: nil)
+
+        expect(observation.valid?).to eq(false)
+        expect(observation.errors[:subcategory]).to include("can't be blank")
+        expect(observation.errors[:severity]).to include("can't be blank")
+        expect(observation.errors[:observation_report]).to include("can't be blank")
+        expect(observation.errors[:evidence_type]).to include("can't be blank")
+      end
+    end
   end
 
   describe "Hooks" do