Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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
12 changes: 12 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ class ApplicationController < ActionController::Base
before_action :check_access_request_notifications
before_action :check_locked_user_notifications

# Upper bound on the pending-access-requests list rendered into the navbar
# dropdown on every HTML request. See check_access_request_notifications.
NAVBAR_ACCESS_REQUESTS_CAP = 50

# AC-8: Determines if the current user must acknowledge consent.
# Returns true when consent is enabled and the session has no valid acknowledgment.
def consent_required?
Expand Down Expand Up @@ -380,9 +384,15 @@ def check_access_request_notifications

# Single query: find all access requests for projects where current user is admin.
# Replaces N+1 loop that called can_admin_project? + eager_load per project.
# `.limit(NAVBAR_ACCESS_REQUESTS_CAP)` bounds the per-request cost on the super-admin
# branch, which previously loaded the whole table on every HTML page (Problem 12
# in docs/plans/DATABASE-COMPLETE-REDESIGN-v2.md). Newest-first so the recent
# asks the admin would actually action stay visible at the cap.
pending_requests = if current_user.admin?
# Super admins see all pending requests — no need to pluck project IDs
ProjectAccessRequest.eager_load(:user, :project)
.order(created_at: :desc)
.limit(NAVBAR_ACCESS_REQUESTS_CAP)
else
admin_project_ids = Membership.where(user_id: current_user.id, role: 'admin',
membership_type: 'Project')
Expand All @@ -391,6 +401,8 @@ def check_access_request_notifications

ProjectAccessRequest.where(project_id: admin_project_ids)
.eager_load(:user, :project)
.order(created_at: :desc)
.limit(NAVBAR_ACCESS_REQUESTS_CAP)
end

@access_requests = pending_requests.map do |ar|
Expand Down
12 changes: 8 additions & 4 deletions app/controllers/components_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -583,10 +583,14 @@ def create_or_duplicate

# Defines the set_component method.
def set_component
# Loads a Component object with associated rules, reviews,
# descriptions, checks and additional answers where ID is equal to params id.
@component = Component.eager_load(
rules: [:reviews, :disa_rule_descriptions, :rule_descriptions, :checks,
# preload (separate queries) NOT eager_load (single LEFT OUTER JOIN). Multiple
# sibling has_many's on Rule create a cartesian product per rule — N reviews
# x M checks x ... — that scales with comment volume. Nested review authors
# (user/triage_set_by/adjudicated_by) avoid an N+1 in ReviewBlueprint via
# RuleBlueprint :editor. See docs/plans/DATABASE-COMPLETE-REDESIGN-v2.md Problem 11.
@component = Component.preload(
rules: [{ reviews: %i[user triage_set_by adjudicated_by] },
:disa_rule_descriptions, :rule_descriptions, :checks,
:additional_answers,
{ satisfies: :srg_rule },
{ satisfied_by: :srg_rule },
Expand Down
61 changes: 51 additions & 10 deletions app/models/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,34 @@ class Component < ApplicationRecord

COMMENT_PHASES = %w[open closed].freeze
CLOSED_REASONS = %w[adjudicating finalized].freeze

# Aggregate pending-comment counts per component, union over rule-scoped
# (commentable_type='BaseRule') AND component-scoped (commentable_type=
# 'Component') reviews. Two ? placeholders, one per UNION branch. See
# .pending_comment_counts.
PENDING_COMMENT_COUNTS_UNION_SQL = <<~SQL.squish.freeze
SELECT component_id, COUNT(*) AS cnt FROM (
SELECT base_rules.component_id
FROM reviews
INNER JOIN base_rules ON base_rules.id = reviews.commentable_id
WHERE reviews.commentable_type = 'BaseRule'
AND reviews.action = 'comment'
AND reviews.responding_to_review_id IS NULL
AND reviews.triage_status = 'pending'
AND base_rules.component_id IN (?)
UNION ALL
SELECT reviews.commentable_id AS component_id
FROM reviews
WHERE reviews.commentable_type = 'Component'
AND reviews.action = 'comment'
AND reviews.responding_to_review_id IS NULL
AND reviews.triage_status = 'pending'
AND reviews.commentable_id IN (?)
) AS pending
GROUP BY component_id
SQL
private_constant :PENDING_COMMENT_COUNTS_UNION_SQL

validates :comment_phase, inclusion: { in: COMMENT_PHASES }
validates :closed_reason, inclusion: { in: CLOSED_REASONS }, allow_nil: true
validate :closed_reason_only_when_closed
Expand Down Expand Up @@ -582,9 +610,20 @@ def search_members(query, limit: 10)
end

def reviews
rule_names = rules.pluck(:id, :rule_id).to_h.transform_values { |rid| "#{prefix}-#{rid}" }
Review.where(rule_id: rule_names.keys).order(created_at: :desc).limit(20).as_json.map do |review|
review['displayed_rule_name'] = rule_names[review['rule_id'].to_i]
rule_id_to_displayed = rules.pluck(:id, :rule_id).to_h.transform_values { |rid| "#{prefix}-#{rid}" }
# Polymorphic union: reviews on this component's rules OR on the component
# itself (commentable_type='Component'). The pre-polymorphic filter
# `rule_id IN (...)` silently dropped component-scoped reviews shipped in
# #729. See docs/plans/DATABASE-COMPLETE-REDESIGN-v2.md Problem 13.
rule_id_subquery = rules.select(:id)
rule_scoped = Review.where(commentable_type: 'BaseRule', commentable_id: rule_id_subquery)
component_scoped = Review.where(commentable_type: 'Component', commentable_id: id)
rule_scoped.or(component_scoped).order(created_at: :desc).limit(20).as_json.map do |review|
review['displayed_rule_name'] = if review['commentable_type'] == 'Component'
'(component)'
else
rule_id_to_displayed[review['rule_id'].to_i]
end
review
end
end
Expand All @@ -602,13 +641,15 @@ def reviews
def self.pending_comment_counts(component_ids)
return {} if component_ids.blank?

Review.where(action: 'comment',
responding_to_review_id: nil,
triage_status: 'pending')
.joins(:rule)
.merge(Rule.where(component_id: component_ids))
.group('base_rules.component_id')
.count
# Polymorphic union: pending top-level comments on this component's rules
# OR on the component itself. The pre-polymorphic `.joins(:rule)` INNER
# JOIN dropped commentable_type='Component' rows (rule_id IS NULL); see
# docs/plans/DATABASE-COMPLETE-REDESIGN-v2.md Problem 13. Mirrors the
# REVIEW_COMPONENT_UNION_BODY pattern on Project — single SQL statement
# so callers' query-count assertions stay green.
sql = sanitize_sql_array([PENDING_COMMENT_COUNTS_UNION_SQL, component_ids, component_ids])
rows = connection.exec_query(sql)
rows.each_with_object({}) { |r, h| h[r['component_id']] = r['cnt'] }
end

# Backs GET /components/:id/comments — the triage table.
Expand Down
Loading
Loading