Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
184 changes: 110 additions & 74 deletions app/controllers/projects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,32 @@ class ProjectsController < ApplicationController
fields_map.freeze
end

# When editing a specific project there are two instance variables to track
# automation highlights: @automated_fields and @overridden_fields.
# These instance variables track which project fields were changed by
# automation (Chief or query-string proposals) so the edit form can
# highlight them for the user.
#
# Both are Hash{Symbol => Hash} keyed by field symbol
# (e.g. :contribution_status):
# - @automated_fields (yellow highlight): fields that were unknown/blank
# (as old values) and then got filled in by automation.
# Values: { new_value:, explanation: }
# - @overridden_fields (orange highlight): fields that had a real user
# value (not '?' or blank) and were forcibly changed by Chief.
# Values: { old_value:, new_value:, explanation: }
#
# They are populated by categorize_automation_changes (first-edit),
# apply_query_string_automation (URL proposals), run_save_automation
# (save-time Chief), and parse_and_validate_field_list (redirect params).
# Consumed by the edit view (via projects_helper automated_field_set /
# overridden_field_set), format_override_details, and
# build_automation_metadata (JSON API).
#
# The external query parameters automated_fields_list and
# overridden_fields_list will set the keys, with the value {} if
# there is nothing more specific, to highlight those fields.

# The 'badge', 'baseline_badge', and 'show_json' actions are special and
# do NOT take a locale.
skip_before_action :redir_missing_locale, only: %i[badge baseline_badge show_json]
Expand Down Expand Up @@ -566,6 +592,9 @@ def edit
return if @criteria_level == 'permissions'

# Run first-edit automation if this level hasn't been edited yet
# (`SECTION_saved` is false). Only do this on "first edit of this section"
# because automation *can* take a while or the remote system could
# crash; we don't want to slow down simple edits from users.
run_first_edit_automation_if_needed

# Always consume query string proposals, even on revisits, so we
Expand All @@ -575,10 +604,13 @@ def edit
# Results merge into @automated_fields for highlighting.
apply_query_string_automation

# Restore override/automated highlighting from redirect query params
# (handle_overridden_fields_redirect passes overridden= and automated=)
@overridden_fields ||= parse_and_validate_field_list(params[:overridden])
@automated_fields ||= parse_and_validate_field_list(params[:automated])
# Merge override/automated highlighting from redirect query params.
# (handle_overridden_fields_redirect passes overridden_fields_list= and automated_fields_list=)
# Existing entries (from automation) take priority over URL params.
@overridden_fields =
merge_field_lists(params[:overridden_fields_list], @overridden_fields)
@automated_fields =
merge_field_lists(params[:automated_fields_list], @automated_fields)

return unless @project.notify_for_static_analysis?('0')

Expand Down Expand Up @@ -1652,7 +1684,7 @@ def apply_query_string_automation
# Track which proposals actually changed values.
# Map _justification fields to their _status counterpart so the
# view (which highlights by status symbol) can find them.
@automated_fields ||= []
@automated_fields ||= {}
modified.each do |field_name|
new_value = @project.public_send(field_name)
next if original_values[field_name] == new_value
Expand All @@ -1663,8 +1695,8 @@ def apply_query_string_automation
else
field_name
end
@automated_fields << {
field: highlight_field, new_value: new_value, explanation: nil
@automated_fields[highlight_field] = {
new_value: new_value, explanation: nil
}
end
end
Expand Down Expand Up @@ -1733,10 +1765,10 @@ def capture_original_values
# - Non-criteria field filled from blank: automated
# Both lists are filtered to the current section.
# @param original_values [Hash] Field name => value before automation
# rubocop:disable Metrics/MethodLength, Metrics/CyclomaticComplexity
# rubocop:disable Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/AbcSize
def categorize_automation_changes(original_values)
automated = []
overridden = []
automated = {}
overridden = {}

original_values.each do |field_name, old_value|
new_value = @project.public_send(field_name)
Expand All @@ -1749,21 +1781,21 @@ def categorize_automation_changes(original_values)
)
elsif field_str.end_with?('_justification')
status_field = field_str.sub('_justification', '_status').to_sym
automated << {
field: status_field, new_value: new_value, explanation: new_value
automated[status_field] = {
new_value: new_value, explanation: new_value
}
elsif ALWAYS_AUTOMATABLE.include?(field_name) &&
old_value.blank? && new_value.present?
automated << {
field: field_name, new_value: new_value, explanation: nil
automated[field_name] = {
new_value: new_value, explanation: nil
}
end
end

@automated_fields = filter_to_current_section(automated)
@overridden_fields = filter_to_current_section(overridden)
end
# rubocop:enable Metrics/MethodLength, Metrics/CyclomaticComplexity
# rubocop:enable Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/AbcSize

# Categorize a status field change as automated or overridden.
# Unknown -> real value is automated (yellow); real -> real is
Expand All @@ -1778,14 +1810,14 @@ def categorize_status_automation(
if @project.respond_to?(justification_field)
@project.public_send(justification_field)
end
info = {
field: field_name, new_value: new_value, explanation: explanation
}
if old_value == CriterionStatus::UNKNOWN
automated << info
automated[field_name] = {
new_value: new_value, explanation: explanation
}
else
info[:old_value] = old_value
overridden << info
overridden[field_name] = {
old_value: old_value, new_value: new_value, explanation: explanation
}
end
end
# rubocop:enable Metrics/MethodLength
Expand Down Expand Up @@ -1836,34 +1868,46 @@ def parse_status_value(value)
end
end

# Parse comma-separated field list and validate field names
# Only accepts fields valid for the current section being edited
# Merge field list from URL params with existing field data.
# URL params provide field names (for highlighting), while existing_fields
# may have rich metadata (old_value, new_value, explanation).
# Rich metadata always takes priority over empty {} from URL params.
# If we're just highlighting we don't *need* the rich metadata
# (mere presence is enough), but we don't want to lose the rich metadata,
# in case later changes to our code use the rich metadata.
# @param field_list_param [String, nil] Comma-separated field names from URL
# @param existing_fields [Hash, nil] Existing field data with metadata
# @return [Hash{Symbol => Hash}] Merged fields preserving rich data
def merge_field_lists(field_list_param, existing_fields)
url_fields = parse_and_validate_field_list(field_list_param)
return existing_fields || {} if url_fields.empty?
return url_fields if existing_fields.blank?

# Merge: existing rich data takes priority over URL's empty {} values
url_fields.merge(existing_fields)
end

# Parse comma-separated field list and validate field names.
# Only accepts fields valid for the current section being edited.
# @param field_string [String, nil] Comma-separated field names
# @return [Array<Hash>] Array of validated field hashes with :field key
# @return [Hash{Symbol => Hash}] Validated fields keyed by field symbol
def parse_and_validate_field_list(field_string)
return [] if field_string.blank?
return {} if field_string.blank?

# Split by comma, strip whitespace, remove empty strings
field_names = field_string.split(',').map(&:strip).compact_blank

# Get valid fields for current section
valid_fields = fields_for_current_section
return [] if valid_fields.nil? # Invalid section level
return {} if valid_fields.nil? # Invalid section level

# Validate each field name
validated_fields = []
# Validate each field name; silently skip invalid ones
validated = {}
field_names.each do |field_name|
# Convert to symbol
field_sym = field_name.to_sym

# Only accept if field is in the valid set for this section
if valid_fields.include?(field_sym)
validated_fields << { field: field_sym }
end
# Silently skip invalid field names (don't raise error, just ignore)
validated[field_sym] = {} if valid_fields.include?(field_sym)
end

validated_fields
validated
end

# Categorize a single Chief proposal (override vs automated)
Expand All @@ -1881,17 +1925,15 @@ def categorize_chief_proposal(field, data, user_value, track_automated = true)
if data[:forced] && user_value.present? && user_value != CriterionStatus::UNKNOWN
# Forced Chief override of user's real value - ORANGE
# Always track overrides (needed for warning messages on any save)
@overridden_fields << {
field: field,
@overridden_fields[field] = {
old_value: user_value,
new_value: chief_value,
explanation: data[:explanation]
}
elsif (user_value == CriterionStatus::UNKNOWN || user_value.blank?) && track_automated
# Chief filling an unknown value - YELLOW
# Only track on save-and-continue so user reviews before accepting
@automated_fields << {
field: field,
@automated_fields[field] = {
new_value: chief_value,
explanation: data[:explanation]
}
Expand All @@ -1918,8 +1960,8 @@ def run_save_automation(changed_fields, user_set_values, chief_instance: nil, tr
# Categorize all proposals BEFORE applying changes.
# Forced overrides of real user values are always tracked (for warnings).
# Non-forced fills are tracked only on save-and-continue (for highlighting).
@overridden_fields = []
@automated_fields = []
@overridden_fields = {}
@automated_fields = {}

current_section_changes.each do |field, data|
categorize_chief_proposal(field, data, user_set_values[field], track_automated)
Expand All @@ -1944,22 +1986,16 @@ def run_save_automation(changed_fields, user_set_values, chief_instance: nil, tr
end
# rubocop:enable Metrics/MethodLength

# Filter a collection to only fields in the current section.
# Accepts a Hash (Chief proposals keyed by field name) or an Array
# of hashes with a :field key (automated/overridden field lists).
# @param collection [Hash, Array<Hash>] Items to filter
# @return [Hash, Array<Hash>] Filtered collection (same type as input)
# Filter a Hash to only include fields in the current section.
# @param collection [Hash] Items keyed by field symbol
# @return [Hash] Filtered to fields valid for the current section
def filter_to_current_section(collection)
valid_fields = fields_for_current_section

# If we don't have a valid field set, don't filter (safety fallback)
return collection if valid_fields.nil?

if collection.is_a?(Hash)
collection.slice(*valid_fields)
else
collection.select { |entry| valid_fields.include?(entry[:field]) }
end
collection.slice(*valid_fields)
end

# Handle Chief failures during save
Expand Down Expand Up @@ -2013,16 +2049,16 @@ def status_value_to_string(value)
def format_override_details
criteria_level_to_internal(@criteria_level)
details =
@overridden_fields.map do |r|
@overridden_fields.map do |field, data|
# Derive criterion key from status field: license_location_status -> license_location
criterion_key = r[:field].to_s.delete_suffix('_status').to_sym
criterion_key = field.to_s.delete_suffix('_status').to_sym
# Show user-visible criterion ID (e.g., license_location or OSPS-LE-03.01)
display_id = helpers.baseline_id_to_display(criterion_key)
t('projects.edit.automation.override_detail',
criterion: display_id,
old: status_value_to_string(r[:old_value]),
new: status_value_to_string(r[:new_value]),
explanation: r[:explanation])
old: status_value_to_string(data[:old_value]),
new: status_value_to_string(data[:new_value]),
explanation: data[:explanation])
end
details.join("\n")
end
Expand All @@ -2040,21 +2076,21 @@ def handle_overridden_fields_redirect(section)
# Log overrides for monitoring
Rails.logger.info(
"Chief override: project=#{@project.id} user=#{current_user&.id} " \
"fields=#{@overridden_fields.pluck(:field).join(',')}"
"fields=#{@overridden_fields.keys.join(',')}"
)

# Redirect with highlight parameters
overridden_field_names = @overridden_fields.map { |r| r[:field].to_s }
automated_field_names = @automated_fields.map { |r| r[:field].to_s }
overridden_field_names = @overridden_fields.keys.map(&:to_s)
automated_field_names = @automated_fields.keys.map(&:to_s)
first_overridden = overridden_field_names.first

redirect_to edit_project_section_path(
@project,
section,
locale: params[:locale],
anchor: first_overridden,
overridden: overridden_field_names.join(','),
automated: automated_field_names.join(',')
overridden_fields_list: overridden_field_names.join(','),
automated_fields_list: automated_field_names.join(',')
)
end
# rubocop:enable Metrics/AbcSize, Metrics/MethodLength
Expand All @@ -2078,8 +2114,8 @@ def perform_html_redirect_after_save(section)
flash[:info] = t('projects.edit.successfully_updated')
redirect_params = { locale: params[:locale] }
if @automated_fields&.any?
redirect_params[:automated] =
@automated_fields.map { |r| r[:field].to_s }.join(',')
redirect_params[:automated_fields_list] =
@automated_fields.keys.join(',')
end
redirect_to edit_project_section_path(@project, section,
**redirect_params) + url_anchor
Expand All @@ -2097,19 +2133,19 @@ def perform_html_redirect_after_save(section)
# rubocop:disable Metrics/MethodLength
def build_automation_metadata
{
overridden: @overridden_fields&.map do |r|
overridden: @overridden_fields&.map do |field, data|
{
field: r[:field],
old_value: r[:old_value],
new_value: r[:new_value],
explanation: r[:explanation]
field: field,
old_value: data[:old_value],
new_value: data[:new_value],
explanation: data[:explanation]
}
end || [],
automated: @automated_fields&.map do |r|
automated: @automated_fields&.map do |field, data|
{
field: r[:field],
value: r[:new_value],
explanation: r[:explanation]
field: field,
value: data[:new_value],
explanation: data[:explanation]
}
end || []
}
Expand Down
12 changes: 6 additions & 6 deletions app/helpers/projects_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,16 @@ def markdown(content)
MarkdownProcessor.render(content)
end

# Precomputed Set of automated field symbols for O(1) lookup.
# Memoized per request (computed once, used by every status_chooser).
# Hash of automated fields for O(1) lookup via Hash#include?.
# Returns empty hash when @automated_fields is nil.
def automated_field_set
@automated_field_set ||= Set.new(@automated_fields&.pluck(:field))
@automated_fields || {}
end

# Precomputed Set of overridden field symbols for O(1) lookup.
# Memoized per request (computed once, used by every status_chooser).
# Hash of overridden fields for O(1) lookup via Hash#include?.
# Returns empty hash when @overridden_fields is nil.
def overridden_field_set
@overridden_field_set ||= Set.new(@overridden_fields&.pluck(:field))
@overridden_fields || {}
end

# Convert a status integer value to its string representation.
Expand Down
Loading