Merge pull request #2698 from coreinfrastructure/fix_automation_bugs

Fix automation bugs

Author: david-a-wheeler

app/controllers/projects_controller.rb

@@ -1733,18 +1733,17 @@ def get_justification_for_status(status_field_name)
     @project.public_send(justification_field)
   end
 
-  # Detect if a criteria justification field was automated (changed value)
-  # URL proposals always count as automated when they change a field
+  # Detect if a criteria justification field was automated (changed value).
+  # Any actual change to justification counts as automation — Chief already
+  # prevents overwriting non-empty justifications when the status is
+  # unchanged, so only legitimate changes reach this point.
   # @param field_name [Symbol] The justification field name
   # @param old_value [String] Original justification
   # @param new_value [String] Current justification
   # @return [Hash, nil] Field info if automated, nil otherwise
   def detect_criteria_justification_automation(field_name, old_value, new_value)
-    # Any change to justification counts as automation
-    # (typically from URL proposal or Chief detective)
     return if old_value == new_value
 
-    # Get the corresponding status field to track as parent
     status_field = field_name.to_s.sub('_justification', '_status').to_sym
     { field: status_field, new_value: new_value, explanation: new_value }
   end

app/lib/chief.rb

@@ -295,20 +295,21 @@ def apply_changes(project, changes)
       next if ALLOWED_FIELDS.exclude?(key)
       next unless update_value?(project, key, data)
 
-      # Store change:
+      old_value = project.attribute_present?(key) ? project[key] : nil
       project[key] = data[:value]
-      # Now add the explanation, if we can.
+      # Set justification if the status actually changed, or if the
+      # justification is empty. If the status is unchanged and there's
+      # already a justification, leave it alone — the user's rationale
+      # should not be modified when no status change occurred.
       next unless key.to_s.end_with?('_status') && data.key?(:explanation)
 
+      status_changed = old_value != data[:value]
       justification_key = (key.to_s.chomp('_status') + '_justification').to_sym
-      if project.attribute_present?(justification_key)
-        unless project[justification_key].end_with?(data[:explanation])
-          project[justification_key] =
-            project[justification_key] + ' ' + data[:explanation]
-        end
-      else
-        project[justification_key] = data[:explanation]
-      end
+      existing = project.attribute_present?(justification_key) &&
+                 project[justification_key]
+      next if !status_changed && existing.present?
+
+      project[justification_key] = data[:explanation]
     end
   end
   # rubocop:enable Metrics/MethodLength, Metrics/AbcSize

db/migrate/20260212221406_backfill_baseline_saved_flags.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+# Copyright the OpenSSF Best Practices badge contributors
+# SPDX-License-Identifier: MIT
+
+# Backfill baseline_N_saved flags using badge_percentage_baseline_N.
+# The original backfill in AddLevelSavedFlags had two bugs:
+# 1. Symbol/String mismatch in criteria name intersection (always empty)
+# 2. Compared integer status columns to string '?' instead of integer 0
+# As a result, baseline saved flags were never set for existing projects.
+# This migration uses the same badge_percentage > 10 approach that works
+# correctly for metal levels.
+class BackfillBaselineSavedFlags < ActiveRecord::Migration[8.1]
+  # rubocop:disable Rails/SkipsModelValidations
+  def up
+    Project.where('badge_percentage_baseline_1 > ?', 10)
+           .update_all(baseline_1_saved: true)
+    Project.where('badge_percentage_baseline_2 > ?', 10)
+           .update_all(baseline_2_saved: true)
+    Project.where('badge_percentage_baseline_3 > ?', 10)
+           .update_all(baseline_3_saved: true)
+  end
+  # rubocop:enable Rails/SkipsModelValidations
+
+  def down
+    # No-op: we can't distinguish which flags were set by this migration
+    # vs. set by normal user edits since the original migration ran.
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.1].define(version: 2026_02_12_174356) do
+ActiveRecord::Schema[8.1].define(version: 2026_02_12_221406) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
   enable_extension "pg_catalog.plpgsql"