From 5ab9b7316808d793fdeacc60663f99542a1132fc Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Mon, 18 May 2026 12:14:08 -0400 Subject: [PATCH 001/537] feat: conditional rule content preload in paginated_comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds include_rule_content: keyword arg (default false) to Component#paginated_comments. When true, extends the existing commentable preload with :disa_rule_descriptions and :checks, then serializes 6 rule-content fields into the row hash: title, severity, status, fixtext, vuln_discussion, check_content. Table mode (default) skips these entirely — zero payload increase. Always includes updated_at for optimistic locking in the triage split-pane (concurrent-edit protection via 409 Conflict). Card: vulcan-v3.x-agw.1 Authored by: Aaron Lippold --- app/models/component.rb | 27 +++++++++++++++++--- spec/models/components_spec.rb | 46 ++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/app/models/component.rb b/app/models/component.rb index e2a39af71..4fe263e27 100644 --- a/app/models/component.rb +++ b/app/models/component.rb @@ -618,7 +618,8 @@ def self.pending_comment_counts(component_ids) # The frontend translates to friendly labels via triageVocabulary.js. def paginated_comments(triage_status: 'all', section: nil, rule_id: nil, # rubocop:disable Metrics/ParameterLists author_id: nil, query: nil, page: 1, per_page: 25, - resolved: 'all', commentable_type: nil) + resolved: 'all', commentable_type: nil, + include_rule_content: false) page = [page.to_i, 1].max per_page = per_page.to_i.clamp(1, 100) @@ -634,7 +635,12 @@ def paginated_comments(triage_status: 'all', section: nil, rule_id: nil, # ruboc when 'rule' then rule_scoped else rule_scoped.or(component_scoped) end - scope = scope.preload(:user, :triage_set_by, :adjudicated_by, :commentable) + commentable_preloads = if include_rule_content + { commentable: %i[disa_rule_descriptions checks] } + else + :commentable + end + scope = scope.preload(:user, :triage_set_by, :adjudicated_by, commentable_preloads) scope = scope.where(triage_status: triage_status) unless triage_status == 'all' scope = scope.where(section: section) if section.present? && section != 'all' @@ -688,7 +694,7 @@ def paginated_comments(triage_status: 'all', section: nil, rule_id: nil, # ruboc rows = page_records.map do |r| component_scoped_row = r.commentable_type == 'Component' - { + row = { id: r.id, rule_id: component_scoped_row ? nil : r.rule_id, rule_displayed_name: component_scoped_row ? '(component)' : rule_id_to_displayed[r.rule_id], @@ -710,8 +716,21 @@ def paginated_comments(triage_status: 'all', section: nil, rule_id: nil, # ruboc commenter_imported: r.commenter_imported?, responses_count: responses_count_lookup[r.id] || 0, reactions: { up: reaction_counts[[r.id, 'up']] || 0, - down: reaction_counts[[r.id, 'down']] || 0 } + down: reaction_counts[[r.id, 'down']] || 0 }, + updated_at: r.updated_at } + + if include_rule_content + is_component_scoped = component_scoped_row + row[:rule_title] = is_component_scoped ? nil : r.commentable&.title + row[:rule_severity] = is_component_scoped ? nil : r.commentable&.rule_severity + row[:rule_status] = is_component_scoped ? nil : r.commentable&.status + row[:rule_fixtext] = is_component_scoped ? nil : r.commentable&.fixtext + row[:rule_vuln_discussion] = is_component_scoped ? nil : r.commentable&.disa_rule_descriptions&.first&.vuln_discussion + row[:rule_check_content] = is_component_scoped ? nil : r.commentable&.checks&.first&.content + end + + row end { diff --git a/spec/models/components_spec.rb b/spec/models/components_spec.rb index 9a828e865..1e3da136e 100644 --- a/spec/models/components_spec.rb +++ b/spec/models/components_spec.rb @@ -1134,5 +1134,51 @@ def build_all_srg_rows_benchmark(srg_rules_list, prefix) expect(c1_row[:commenter_imported]).to be(true) end end + + it 'includes 6 rule content fields when include_rule_content: true' do + result = shared_component.paginated_comments(triage_status: 'all', include_rule_content: true) + c1_row = result[:rows].find { |r| r[:id] == @c1.id } + rule = shared_component.rules.find_by(id: @c1.rule_id) + + expect(c1_row[:rule_title]).to eq(rule.title) + expect(c1_row[:rule_severity]).to eq(rule.rule_severity) + expect(c1_row[:rule_status]).to eq(rule.status) + expect(c1_row[:rule_fixtext]).to eq(rule.fixtext) + expect(c1_row[:rule_vuln_discussion]).to eq(rule.disa_rule_descriptions.first&.vuln_discussion) + expect(c1_row[:rule_check_content]).to eq(rule.checks.first&.content) + end + + it 'returns nil rule content fields for component-scoped comments with include_rule_content: true' do + comp_review = Review.create!( + action: 'comment', comment: 'component-level feedback', + user: pc_viewer, commentable: shared_component, section: nil + ) + result = shared_component.paginated_comments(triage_status: 'all', include_rule_content: true) + comp_row = result[:rows].find { |r| r[:id] == comp_review.id } + expect(comp_row[:rule_title]).to be_nil + expect(comp_row[:rule_severity]).to be_nil + expect(comp_row[:rule_check_content]).to be_nil + end + + it 'omits rule content keys in default table mode (no include_rule_content)' do + result = shared_component.paginated_comments(triage_status: 'all') + c1_row = result[:rows].find { |r| r[:id] == @c1.id } + expect(c1_row).not_to have_key(:rule_title) + expect(c1_row).not_to have_key(:rule_severity) + expect(c1_row).not_to have_key(:rule_fixtext) + expect(c1_row).not_to have_key(:rule_check_content) + expect(c1_row).not_to have_key(:rule_vuln_discussion) + expect(c1_row).not_to have_key(:rule_status) + end + + it 'always includes updated_at for optimistic locking regardless of include_rule_content' do + result = shared_component.paginated_comments(triage_status: 'all') + c1_row = result[:rows].find { |r| r[:id] == @c1.id } + expect(c1_row[:updated_at]).to eq(@c1.updated_at) + + result_with = shared_component.paginated_comments(triage_status: 'all', include_rule_content: true) + c1_row_with = result_with[:rows].find { |r| r[:id] == @c1.id } + expect(c1_row_with[:updated_at]).to eq(@c1.updated_at) + end end end From c34b87412e2c1589f21ba94e7980a0c30451a447 Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Mon, 18 May 2026 13:49:48 -0400 Subject: [PATCH 002/537] refactor: extract CommentTriageForm + reconcile terminal constants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move TERMINAL_BY_RULE into triageVocabulary.js as two exports: TERMINAL_AUTO_ADJUDICATE (matches Ruby — duplicate, informational, withdrawn) and SINGLE_BUTTON_STATUSES (adds needs_clarification for UI single-button behavior). Extract decision core (radios, response textarea, duplicate picker, save/cancel, dirty tracking) into CommentTriageForm.vue. Modal becomes thin wrapper — admin actions stay in modal. 24 new form tests, 42 existing modal tests updated. Authored by: Aaron Lippold --- .../components/CommentTriageModal.vue | 187 +++----------- .../components/triage/CommentTriageForm.vue | 190 ++++++++++++++ app/javascript/constants/triageVocabulary.js | 10 + .../components/CommentTriageModal.spec.js | 75 +++--- .../triage/CommentTriageForm.spec.js | 233 ++++++++++++++++++ 5 files changed, 508 insertions(+), 187 deletions(-) create mode 100644 app/javascript/components/triage/CommentTriageForm.vue create mode 100644 spec/javascript/components/triage/CommentTriageForm.spec.js diff --git a/app/javascript/components/components/CommentTriageModal.vue b/app/javascript/components/components/CommentTriageModal.vue index 15ffd5a1c..b07123a80 100644 --- a/app/javascript/components/components/CommentTriageModal.vue +++ b/app/javascript/components/components/CommentTriageModal.vue @@ -5,6 +5,7 @@ :title="modalTitle" centered no-close-on-backdrop + hide-footer @hidden="$emit('hidden')" > + + diff --git a/spec/javascript/components/components/ComponentComments.spec.js b/spec/javascript/components/components/ComponentComments.spec.js index 06ef26c3e..187b95b4c 100644 --- a/spec/javascript/components/components/ComponentComments.spec.js +++ b/spec/javascript/components/components/ComponentComments.spec.js @@ -143,19 +143,15 @@ describe("ComponentComments", () => { ); }); - it("opens the triage modal via $bvModal.show when openTriageFor is called", async () => { + it("enters split mode when openTriageFor is called", async () => { const wrapper = mount(ComponentComments, { propsData: { componentId: 42 }, stubs: SHARED_STUBS, }); await flushPromises(); - // BootstrapVue installs $bvModal as a read-only instance property, so - // we spy on its show method directly after mount. - const showSpy = vi.spyOn(wrapper.vm.$bvModal, "show").mockImplementation(() => {}); wrapper.vm.openTriageFor(mockResponse.data.rows[0]); - expect(wrapper.vm.selectedRow.id).toBe(142); - expect(showSpy).toHaveBeenCalledWith("comment-triage-modal"); - showSpy.mockRestore(); + expect(wrapper.vm.splitMode).toBe(true); + expect(wrapper.vm.splitCommentId).toBe(142); }); // REQUIREMENT: rule deep-link must URL-encode the rule_displayed_name so diff --git a/spec/javascript/components/triage/TriageSplitView.spec.js b/spec/javascript/components/triage/TriageSplitView.spec.js new file mode 100644 index 000000000..973388842 --- /dev/null +++ b/spec/javascript/components/triage/TriageSplitView.spec.js @@ -0,0 +1,250 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { mount } from "@vue/test-utils"; +import { localVue } from "@test/testHelper"; +import axios from "axios"; +import TriageSplitView from "@/components/triage/TriageSplitView.vue"; + +vi.mock("axios"); + +const flushPromises = async (wrapper) => { + await new Promise((resolve) => setTimeout(resolve, 0)); + if (wrapper) await wrapper.vm.$nextTick(); +}; + +const rows = [ + { + id: 1, + rule_id: 10, + rule_displayed_name: "CNTR-01-000001", + commentable_type: "Rule", + section: "check_content", + author_name: "Demo Viewer", + comment: "The check text mentions runc 1.0", + triage_status: "pending", + created_at: "2026-05-01T00:00:00Z", + adjudicated_at: null, + updated_at: "2026-05-01T00:00:00Z", + rule_title: "The container platform must limit privileges", + rule_severity: "CAT II", + rule_status: "Applicable - Configurable", + rule_fixtext: "Configure the container platform to restrict access", + rule_check_content: "Verify that the container runtime enforces privilege restrictions", + rule_vuln_discussion: "Without proper privilege restriction, containers could escalate", + }, + { + id: 2, + rule_id: 10, + rule_displayed_name: "CNTR-01-000001", + commentable_type: "Rule", + section: "fixtext", + author_name: "Demo Reviewer", + comment: "The fix text could include the seccomp profile path", + triage_status: "pending", + created_at: "2026-05-01T01:00:00Z", + adjudicated_at: null, + updated_at: "2026-05-01T01:00:00Z", + rule_title: "The container platform must limit privileges", + rule_severity: "CAT II", + rule_status: "Applicable - Configurable", + rule_fixtext: "Configure the container platform to restrict access", + rule_check_content: "Verify that the container runtime enforces privilege restrictions", + rule_vuln_discussion: "Without proper privilege restriction, containers could escalate", + }, + { + id: 3, + rule_id: 11, + rule_displayed_name: "CNTR-01-000002", + commentable_type: "Rule", + section: null, + author_name: "Demo Reviewer", + comment: "Could we soften the severity?", + triage_status: "concur", + created_at: "2026-05-01T02:00:00Z", + adjudicated_at: null, + updated_at: "2026-05-01T02:00:00Z", + rule_title: "Test rule 2", + rule_severity: "CAT III", + rule_status: "Applicable - Configurable", + rule_fixtext: null, + rule_check_content: null, + rule_vuln_discussion: null, + }, +]; + +function baseProps(overrides = {}) { + return { + rows, + initialCommentId: 1, + componentId: 5, + effectivePermissions: "admin", + ...overrides, + }; +} + +describe("TriageSplitView", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + // ── Reactivity: activeCommentId as data, object via computed ──────── + + it("derives activeComment from activeCommentId via computed", () => { + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + expect(w.vm.activeCommentId).toBe(1); + expect(w.vm.activeComment).toBeTruthy(); + expect(w.vm.activeComment.id).toBe(1); + expect(w.vm.activeComment.rule_displayed_name).toBe("CNTR-01-000001"); + }); + + it("updates activeComment when activeCommentId changes", async () => { + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + w.vm.activeCommentId = 2; + await w.vm.$nextTick(); + expect(w.vm.activeComment.id).toBe(2); + expect(w.vm.activeComment.section).toBe("fixtext"); + }); + + // ── Rule content passed to RuleContextPanel ──────────────────────── + + it("passes rule content from the active row to RuleContextPanel", () => { + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + const panel = w.findComponent({ name: "RuleContextPanel" }); + expect(panel.exists()).toBe(true); + expect(panel.props("ruleContent")).toBeTruthy(); + expect(panel.props("ruleContent").rule_title).toBe( + "The container platform must limit privileges", + ); + }); + + it("passes focusedSection from the active comment's section", () => { + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + const panel = w.findComponent({ name: "RuleContextPanel" }); + expect(panel.props("focusedSection")).toBe("check_content"); + }); + + // ── CommentTriageForm integration ────────────────────────────────── + + it("renders CommentTriageForm for the active comment", () => { + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + const form = w.findComponent({ name: "CommentTriageForm" }); + expect(form.exists()).toBe(true); + expect(form.props("review").id).toBe(1); + }); + + // ── TriageQueueNav integration ───────────────────────────────────── + + it("renders TriageQueueNav with rows and currentId", () => { + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + const nav = w.findComponent({ name: "TriageQueueNav" }); + expect(nav.exists()).toBe(true); + expect(nav.props("currentId")).toBe(1); + expect(nav.props("comments")).toHaveLength(3); + }); + + // ── Dirty-form guard ─────────────────────────────────────────────── + + it("prompts before switching when form is dirty", async () => { + window.confirm = vi.fn().mockReturnValue(false); + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + w.vm.isDirty = true; + w.vm.onQueueSelect(2); + expect(window.confirm).toHaveBeenCalled(); + expect(w.vm.activeCommentId).toBe(1); + }); + + it("switches when form is dirty and user confirms", async () => { + window.confirm = vi.fn().mockReturnValue(true); + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + w.vm.isDirty = true; + w.vm.onQueueSelect(2); + expect(w.vm.activeCommentId).toBe(2); + }); + + it("switches without prompting when form is clean", () => { + window.confirm = vi.fn(); + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + w.vm.isDirty = false; + w.vm.onQueueSelect(2); + expect(window.confirm).not.toHaveBeenCalled(); + expect(w.vm.activeCommentId).toBe(2); + }); + + // ── Save with optimistic locking ─────────────────────────────────── + + it("sends updated_at with triage PATCH for optimistic locking", async () => { + axios.patch.mockResolvedValue({ + data: { review: { ...rows[0], triage_status: "concur" } }, + }); + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + await w.vm.onTriageSave({ triage_status: "concur" }); + await flushPromises(w); + expect(axios.patch).toHaveBeenCalledWith( + "/reviews/1/triage", + expect.objectContaining({ + triage_status: "concur", + expected_updated_at: "2026-05-01T00:00:00Z", + }), + ); + }); + + it("emits triaged on successful save", async () => { + axios.patch.mockResolvedValue({ + data: { review: { ...rows[0], triage_status: "concur" } }, + }); + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + await w.vm.onTriageSave({ triage_status: "concur" }); + await flushPromises(w); + expect(w.emitted("triaged")).toBeTruthy(); + }); + + // ── Save button disabled during request ──────────────────────────── + + it("sets saving=true during pending request", async () => { + axios.patch.mockImplementation(() => new Promise(() => {})); + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + w.vm.onTriageSave({ triage_status: "concur" }); + await w.vm.$nextTick(); + expect(w.vm.saving).toBe(true); + }); + + // ── Error handling ───────────────────────────────────────────────── + + it("shows conflict message on 409 (optimistic lock failure)", async () => { + axios.patch.mockRejectedValue({ + response: { status: 409, data: { error: "Record was modified" } }, + }); + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + await w.vm.onTriageSave({ triage_status: "concur" }); + await flushPromises(w); + expect(w.vm.conflictAlert).toBeTruthy(); + }); + + it("surfaces 422 errors via AlertMixin", async () => { + axios.patch.mockRejectedValue({ + response: { status: 422, data: { error: "Non-concur requires a response" } }, + }); + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + const alertSpy = vi.spyOn(w.vm, "alertOrNotifyResponse").mockImplementation(() => {}); + await w.vm.onTriageSave({ triage_status: "non_concur" }); + await flushPromises(w); + expect(alertSpy).toHaveBeenCalled(); + alertSpy.mockRestore(); + }); + + // ── Exit when active comment filtered out ────────────────────────── + + it("emits exit when active comment is removed from rows", async () => { + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + await w.setProps({ rows: rows.filter((r) => r.id !== 1) }); + await w.vm.$nextTick(); + expect(w.emitted("exit")).toBeTruthy(); + }); + + // ── Cancel emits exit ────────────────────────────────────────────── + + it("emits exit on cancel", () => { + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + w.vm.onCancel(); + expect(w.emitted("exit")).toBeTruthy(); + }); +}); From 0a16ae72108422151b774c4d78e0117852cec3b1 Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Mon, 18 May 2026 18:40:50 -0400 Subject: [PATCH 007/537] =?UTF-8?q?fix(triage):=20UX=20polish=20=E2=80=94?= =?UTF-8?q?=20back=20button,=20inline=20sections,=20role=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Back button: shows "Back to Triage Table" in split mode, exits to table view. "Back to Component Editor" in table mode. RuleContextPanel: Title/Severity/Status render as inline key:value pairs (no accordion waste). Fix/Check/VulnDiscussion stay collapsible. Fixed indentation on focused body (v-text avoids template whitespace with pre-wrap). TriageSplitView: sort rows by id ascending so queue position matches table order. Role gate — viewer sees read-only hint instead of triage form. CommentThread integrated for inline replies. TriageQueueNav: dropdown shows #id not array position. 5 new tests (sortedRows, role gating, reply thread, dropdown #id format). Authored by: Aaron Lippold --- .../components/ComponentComments.vue | 3 + .../components/ComponentTriagePage.vue | 21 ++++++- .../components/triage/RuleContextPanel.vue | 41 ++++++++++--- .../components/triage/TriageQueueNav.vue | 4 +- .../components/triage/TriageSplitView.vue | 35 ++++++++--- .../triage/RuleContextPanel.spec.js | 60 ++++++++++++++----- .../components/triage/TriageQueueNav.spec.js | 7 +++ .../components/triage/TriageSplitView.spec.js | 43 +++++++++++++ 8 files changed, 181 insertions(+), 33 deletions(-) diff --git a/app/javascript/components/components/ComponentComments.vue b/app/javascript/components/components/ComponentComments.vue index ffe22c28b..584718ed4 100644 --- a/app/javascript/components/components/ComponentComments.vue +++ b/app/javascript/components/components/ComponentComments.vue @@ -69,6 +69,7 @@ @triaged="onTriaged" @adjudicated="onAdjudicated" @response-posted="onTriageResponsePosted" + @open-reply-composer="openReplyComposer" /> @@ -456,11 +457,13 @@ export default { openTriageFor(row) { this.splitCommentId = row.id; this.splitMode = true; + this.$emit("split-mode-changed", true); this.fetch(); }, exitSplitMode() { this.splitMode = false; this.splitCommentId = null; + this.$emit("split-mode-changed", false); this.fetch(); }, // Button label clarifies the lifecycle stage: pending → triage, diff --git a/app/javascript/components/components/ComponentTriagePage.vue b/app/javascript/components/components/ComponentTriagePage.vue index 02ea2012e..694a77dcf 100644 --- a/app/javascript/components/components/ComponentTriagePage.vue +++ b/app/javascript/components/components/ComponentTriagePage.vue @@ -14,16 +14,26 @@

- + + Back to Triage Table + + Back to Component Editor
@@ -44,6 +54,7 @@ export default { data() { return { component: this.initialComponentState, + isSplitMode: false, }; }, computed: { @@ -56,5 +67,13 @@ export default { ]; }, }, + methods: { + exitSplit() { + this.isSplitMode = false; + if (this.$refs.comments) { + this.$refs.comments.exitSplitMode(); + } + }, + }, }; diff --git a/app/javascript/components/triage/RuleContextPanel.vue b/app/javascript/components/triage/RuleContextPanel.vue index fbe54a1fa..2011df979 100644 --- a/app/javascript/components/triage/RuleContextPanel.vue +++ b/app/javascript/components/triage/RuleContextPanel.vue @@ -2,11 +2,28 @@
@@ -52,6 +68,8 @@ diff --git a/spec/javascript/components/triage/TriageSplitView.spec.js b/spec/javascript/components/triage/TriageSplitView.spec.js index 3c425750c..f3d07b6c1 100644 --- a/spec/javascript/components/triage/TriageSplitView.spec.js +++ b/spec/javascript/components/triage/TriageSplitView.spec.js @@ -307,4 +307,72 @@ describe("TriageSplitView", () => { expect(thread.exists()).toBe(true); expect(thread.props("parentReviewId")).toBe(1); }); + + // ── Admin actions (migrated from modal) ──────────────────────────── + + it("shows admin actions disclosure for admin role", () => { + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + expect(w.find('[data-testid="open-admin-actions"]').exists()).toBe(true); + }); + + it("hides admin actions for non-admin roles", () => { + const w = mount(TriageSplitView, { + localVue, + propsData: baseProps({ effectivePermissions: "author" }), + }); + expect(w.find('[data-testid="open-admin-actions"]').exists()).toBe(false); + }); + + it("posts to admin_withdraw with audit comment", async () => { + axios.patch.mockResolvedValue({ + data: { review: { ...rows[0], triage_status: "withdrawn" } }, + }); + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + w.vm.adminAction = "force-withdraw"; + w.vm.adminAuditComment = "spam content"; + await w.vm.submitAdminAction(); + await flushPromises(w); + expect(axios.patch).toHaveBeenCalledWith( + "/reviews/1/admin_withdraw", + expect.objectContaining({ audit_comment: "spam content" }), + ); + expect(w.emitted("triaged")).toHaveLength(1); + }); + + it("posts DELETE to admin_destroy with audit comment and typed-id confirmation", async () => { + axios.delete.mockResolvedValue({ data: { ok: true } }); + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + w.vm.adminAction = "hard-delete"; + w.vm.adminAuditComment = "PII removed"; + w.vm.adminConfirmationId = "1"; + await w.vm.submitAdminAction(); + await flushPromises(w); + expect(axios.delete).toHaveBeenCalledWith( + "/reviews/1/admin_destroy", + expect.objectContaining({ + data: expect.objectContaining({ audit_comment: "PII removed" }), + }), + ); + expect(w.emitted("destroyed")).toHaveLength(1); + expect(w.emitted("destroyed")[0][0]).toBe(1); + }); + + it("canSubmitAdminAction requires audit comment", () => { + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + w.vm.adminAction = "force-withdraw"; + w.vm.adminAuditComment = ""; + expect(w.vm.canSubmitAdminAction).toBe(false); + w.vm.adminAuditComment = "reason"; + expect(w.vm.canSubmitAdminAction).toBe(true); + }); + + it("canSubmitAdminAction for hard-delete requires typed-id match", () => { + const w = mount(TriageSplitView, { localVue, propsData: baseProps() }); + w.vm.adminAction = "hard-delete"; + w.vm.adminAuditComment = "reason"; + w.vm.adminConfirmationId = "wrong"; + expect(w.vm.canSubmitAdminAction).toBe(false); + w.vm.adminConfirmationId = "1"; + expect(w.vm.canSubmitAdminAction).toBe(true); + }); }); From aeb7363b506e27f0b7c821e8bdffa600f2f98c92 Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Mon, 18 May 2026 20:09:53 -0400 Subject: [PATCH 011/537] refactor: move command bar + admin panel to page level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ComponentTriagePage owns the BaseCommandBar with "Back to Table", "Back to Editor", and "Admin" toggle — matching the ProjectCommandBar pattern used on the project page. Admin panel state (adminPanelOpen) flows down as a prop through ComponentComments → TriageSplitView → b-sidebar. TriageSplitView no longer owns the command bar or admin toggle — it just renders the sidebar and emits close. Authored by: Aaron Lippold --- .../components/ComponentComments.vue | 3 + .../components/ComponentTriagePage.vue | 74 ++++-- .../components/triage/TriageSplitView.vue | 215 +++++++++--------- .../components/triage/TriageSplitView.spec.js | 11 +- 4 files changed, 163 insertions(+), 140 deletions(-) diff --git a/app/javascript/components/components/ComponentComments.vue b/app/javascript/components/components/ComponentComments.vue index 16db35702..522781d88 100644 --- a/app/javascript/components/components/ComponentComments.vue +++ b/app/javascript/components/components/ComponentComments.vue @@ -65,12 +65,14 @@ :initial-comment-id="splitCommentId" :component-id="componentId" :effective-permissions="effectivePermissions" + :admin-panel-open="adminPanelOpen" @exit="exitSplitMode" @triaged="onTriaged" @adjudicated="onAdjudicated" @response-posted="onTriageResponsePosted" @destroyed="onDestroyed" @open-reply-composer="openReplyComposer" + @admin-panel-close="$emit('admin-panel-close')" /> @@ -244,6 +246,7 @@ export default { // triage queue but cannot mutate — author+ can triage / adjudicate // / re-open. Mirrors the backend authorize_author_project gates. effectivePermissions: { type: String, default: null }, + adminPanelOpen: { type: Boolean, default: false }, }, data() { const persisted = this.loadPersistedFilters(); diff --git a/app/javascript/components/components/ComponentTriagePage.vue b/app/javascript/components/components/ComponentTriagePage.vue index 694a77dcf..5206e001c 100644 --- a/app/javascript/components/components/ComponentTriagePage.vue +++ b/app/javascript/components/components/ComponentTriagePage.vue @@ -1,31 +1,45 @@ diff --git a/spec/javascript/components/triage/TriageQueueNav.spec.js b/spec/javascript/components/triage/TriageQueueNav.spec.js index f34f29a2c..df1e27af6 100644 --- a/spec/javascript/components/triage/TriageQueueNav.spec.js +++ b/spec/javascript/components/triage/TriageQueueNav.spec.js @@ -143,6 +143,47 @@ describe("TriageQueueNav", () => { expect(lastHeader.text()).toContain("1"); }); + // ── 2D Navigation: rule arrows (left/right) ───────────────────────── + + describe("rule-level navigation (left/right)", () => { + it("prev-rule jumps to first comment of previous rule", async () => { + const w = mount(TriageQueueNav, { localVue, propsData: baseProps({ currentId: 4 }) }); + await w.find('[data-testid="prev-rule"]').trigger("click"); + expect(w.emitted("select")[0][0]).toBe(1); + }); + + it("next-rule jumps to first comment of next rule", async () => { + const w = mount(TriageQueueNav, { localVue, propsData: baseProps({ currentId: 1 }) }); + await w.find('[data-testid="next-rule"]').trigger("click"); + expect(w.emitted("select")[0][0]).toBe(4); + }); + + it("prev-rule disabled on first rule", () => { + const w = mount(TriageQueueNav, { localVue, propsData: baseProps({ currentId: 1 }) }); + expect(w.find('[data-testid="prev-rule"]').attributes("disabled")).toBeDefined(); + }); + + it("next-rule disabled on last rule", () => { + const w = mount(TriageQueueNav, { localVue, propsData: baseProps({ currentId: 6 }) }); + expect(w.find('[data-testid="next-rule"]').attributes("disabled")).toBeDefined(); + }); + + it("next-rule from middle of a rule jumps to first comment of next rule", async () => { + const w = mount(TriageQueueNav, { localVue, propsData: baseProps({ currentId: 2 }) }); + await w.find('[data-testid="next-rule"]').trigger("click"); + expect(w.emitted("select")[0][0]).toBe(4); + }); + }); + + // ── Bold rule headers in dropdown ────────────────────────────────── + + it("dropdown rule headers have bold rule name", () => { + const w = mount(TriageQueueNav, { localVue, propsData: baseProps() }); + const headers = w.findAll('[data-testid="queue-dropdown-rule-header"]'); + expect(headers.at(0).find("strong").exists()).toBe(true); + expect(headers.at(0).find("strong").text()).toBe("CNTR-01-000001"); + }); + // ── Scale test ───────────────────────────────────────────────────── it("handles 200 comments across 40 rules without crashing", () => { From ddaab8e4414696feb9cfe05dbe33a4b6b71c1531 Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Tue, 19 May 2026 14:40:29 -0400 Subject: [PATCH 021/537] feat: add reaction buttons to split-pane comment view Wire ReactionButtons component into TriageSplitView below the comment blockquote. Toggle emits POST to reactions endpoint + refresh. 27/27 TriageSplitView tests passing. Authored by: Aaron Lippold --- .../components/triage/TriageSplitView.vue | 20 +++++++++++++++++++ .../components/triage/TriageSplitView.spec.js | 14 +++++++++++++ 2 files changed, 34 insertions(+) diff --git a/app/javascript/components/triage/TriageSplitView.vue b/app/javascript/components/triage/TriageSplitView.vue index 438b282af..c99fef68b 100644 --- a/app/javascript/components/triage/TriageSplitView.vue +++ b/app/javascript/components/triage/TriageSplitView.vue @@ -46,6 +46,11 @@
{{ activeComment.comment }}
+ { w.vm.adminConfirmationId = "1"; expect(w.vm.canSubmitAdminAction).toBe(true); }); + + // ── Reaction buttons ────────────────────────────────────────────── + + it("renders ReactionButtons for the active comment", async () => { + const w = mount(TriageSplitView, { + localVue, + propsData: baseProps({ initialCommentId: 1 }), + }); + await flushPromises(w); + const reactionBtns = w.findComponent({ name: "ReactionButtons" }); + expect(reactionBtns.exists()).toBe(true); + expect(reactionBtns.props("reviewId")).toBe(1); + }); }); From ca277f84114064322ccca7986321a34784435ae2 Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Tue, 19 May 2026 14:44:55 -0400 Subject: [PATCH 022/537] fix: use ReactionToggleMixin for split-pane reactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace broken manual handler with existing mixin pattern. Optimistic update, mine tracking, error rollback now work correctly — matching CommentThread and RuleReviews. Authored by: Aaron Lippold --- .../components/triage/TriageSplitView.vue | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/app/javascript/components/triage/TriageSplitView.vue b/app/javascript/components/triage/TriageSplitView.vue index c99fef68b..b32cfe953 100644 --- a/app/javascript/components/triage/TriageSplitView.vue +++ b/app/javascript/components/triage/TriageSplitView.vue @@ -200,6 +200,7 @@ import RuleContextPanel from "./RuleContextPanel.vue"; import CommentTriageForm from "./CommentTriageForm.vue"; import RulePicker from "../components/RulePicker.vue"; import ReactionButtons from "../shared/ReactionButtons.vue"; +import ReactionToggleMixin from "../../mixins/ReactionToggleMixin.vue"; export default { name: "TriageSplitView", @@ -212,7 +213,7 @@ export default { RulePicker, ReactionButtons, }, - mixins: [AlertMixin, FormMixin, RoleComparisonMixin], + mixins: [AlertMixin, FormMixin, RoleComparisonMixin, ReactionToggleMixin], props: { rows: { type: Array, required: true }, initialCommentId: { type: [Number, String], required: true }, @@ -398,18 +399,18 @@ export default { onCancel() { this.$emit("exit"); }, - async onReactionToggle(kind) { + onReactionToggle(kind) { if (!this.activeComment) return; - try { - await axios.post( - `/reviews/${this.activeComment.id}/reactions`, - { kind }, - { headers: { Accept: "application/json" } }, - ); - this.$emit("refresh"); - } catch (err) { - this.showAlert("danger", "Reaction failed", err?.response?.data?.error || "Unknown error"); - } + const prev = { ...this.activeComment.reactions }; + const apply = (reactions) => { + this.$set(this.activeComment, "reactions", reactions); + }; + this.submitReactionToggle({ + reviewId: this.activeComment.id, + prev, + kind, + apply, + }); }, onTargetRuleSelected(ruleId) { this.adminTargetRuleId = ruleId; From 059497e669fe911237d6c32937100265f2500d8b Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Tue, 19 May 2026 15:28:29 -0400 Subject: [PATCH 023/537] feat: add ReplyComposerMixin with unified composerState Single composerState object replaces both Pattern A (composerReplyRow) and Pattern B (composerReplyToId + composerSection). Provides composerProps computed for modal binding + afterComposerPosted hook for consumers. 16/16 mixin tests passing. Authored by: Aaron Lippold --- app/javascript/mixins/ReplyComposerMixin.vue | 93 +++++++ .../2026-05-19-comment-interaction-dry.md | 235 ++++++++++++++++++ .../mixins/ReplyComposerMixin.spec.js | 198 +++++++++++++++ 3 files changed, 526 insertions(+) create mode 100644 app/javascript/mixins/ReplyComposerMixin.vue create mode 100644 docs/superpowers/plans/2026-05-19-comment-interaction-dry.md create mode 100644 spec/javascript/mixins/ReplyComposerMixin.spec.js diff --git a/app/javascript/mixins/ReplyComposerMixin.vue b/app/javascript/mixins/ReplyComposerMixin.vue new file mode 100644 index 000000000..640a4ad45 --- /dev/null +++ b/app/javascript/mixins/ReplyComposerMixin.vue @@ -0,0 +1,93 @@ + diff --git a/docs/superpowers/plans/2026-05-19-comment-interaction-dry.md b/docs/superpowers/plans/2026-05-19-comment-interaction-dry.md new file mode 100644 index 000000000..ea6af88de --- /dev/null +++ b/docs/superpowers/plans/2026-05-19-comment-interaction-dry.md @@ -0,0 +1,235 @@ +# Centralize Comment Interaction System — DRY Plan + +**Goal:** Unify the two divergent composer patterns (row-based vs id-based) into a single `composerState` object managed by `ReplyComposerMixin`, then migrate all 4 consumers and update CommentComposerModal to read from the unified state. + +**Why:** 4 screens mount CommentComposerModal with 2 different state shapes (Pattern A: `composerReplyRow` row object; Pattern B: `composerReplyToId` + `composerSection` + `componentComposerActive` scattered data). Any bug fix or feature change must be applied 4 times. The event chain has an inconsistency: RuleReviews emits a bare reviewId while other sources emit full row objects. + +**Audit date:** 2026-05-19 (updated). Full inventory of 4 consumers, 3 event passthrough layers, 7 test files. + +--- + +## Architecture + +### Current (two incompatible patterns) + +``` +Pattern A (row-based): + ComponentComments: composerReplyRow (Object) + composerNewComponent (Boolean) + UserComments: composerReplyRow (Object) + +Pattern B (id-based): + ProjectComponent: composerReplyToId (Number) + composerSection (String) + componentComposerActive (Boolean) + RulesCodeEditorView: composerReplyToId (Number) + composerSection (String) + componentComposerActive (Boolean) +``` + +### Target (unified composerState) + +``` +ReplyComposerMixin: + data: + composerState: { + mode: null, // 'reply' | 'new-comment' | 'component' | null + reviewId: null, // parent review ID (reply mode) + ruleId: null, // target rule + componentId: null, // target component + section: null, // section (new-comment mode) + ruleName: null, // display string for modal header + } + + methods: + openReplyComposer({ reviewId, ruleId, componentId, ruleName }) + openSectionComposer({ ruleId, componentId, section, ruleName }) + openComponentComposer(componentId) + closeComposer() + onComposerPosted() — calls afterComposerPosted() hook + onComposerHidden() — alias for closeComposer() + + computed: + composerActive — Boolean, true when mode !== null + composerProps — Object, maps composerState to CommentComposerModal props + +CommentComposerModal: + Accepts composerState directly (or individual props mapped by composerProps computed) + No change to its internal logic — just standardize what flows in + +Consumers: + ComponentComments: mixin + afterComposerPosted() override (fetch + split-mode) + ProjectComponent: mixin + afterComposerPosted() override (axios rule refresh) + RulesCodeEditorView: mixin + afterComposerPosted() override ($root.$emit rule refresh) + UserComments: mixin + afterComposerPosted() override (fetch) +``` + +### Event chain standardization + +``` +CommentThread emits @reply(parentReviewId) + ↓ +RuleReviews / TriageSplitView receives — builds {reviewId, ruleId, componentId, ruleName} + ↓ +$emit('open-reply-composer', composerPayload) ← ALWAYS an object, never a bare ID + ↓ +Consumer calls this.openReplyComposer(payload) +``` + +--- + +## Full File Inventory + +### Files to CREATE +| File | Purpose | +|------|---------| +| `app/javascript/mixins/ReplyComposerMixin.vue` | Unified mixin | +| `spec/javascript/mixins/ReplyComposerMixin.spec.js` | Mixin tests | + +### Files to MODIFY (consumers — Task 2) +| File | Current pattern | Changes | +|------|----------------|---------| +| `ComponentComments.vue` | Pattern A: composerReplyRow, composerNewComponent | Replace with mixin. Keep afterComposerPosted (fetch + thread refresh + split-mode) | +| `ProjectComponent.vue` | Pattern B: composerReplyToId, composerSection, componentComposerActive | Replace with mixin. Keep afterComposerPosted (axios rule refresh) | +| `RulesCodeEditorView.vue` | Pattern B: composerReplyToId, composerSection, componentComposerActive | Replace with mixin. Keep afterComposerPosted ($root.$emit refresh) | +| `UserComments.vue` | Pattern A: composerReplyRow | Replace with mixin. Keep afterComposerPosted (fetch + thread refresh) | + +### Files to MODIFY (event emitters — Task 2) +| File | Current emission | Changes | +|------|-----------------|---------| +| `RuleReviews.vue` | `$emit('open-reply-composer', parentId)` bare Number | Change to emit `{ reviewId: parentId, ruleId, componentId, ruleName }` object | +| `ControlsSidepanels.vue` | `$emit('open-reply-composer', $event)` passthrough | No change needed (passes through whatever it receives) | +| `TriageSplitView.vue` | `$emit('open-reply-composer', activeComment)` full row | Change to emit standardized `{ reviewId, ruleId, componentId, ruleName }` | +| `CommentTriageModal.vue` | `$emit('open-reply-composer', review)` full review | Change to emit standardized object | + +### Files to MODIFY (CommentComposerModal — Task 1) +| File | Changes | +|------|---------| +| `CommentComposerModal.vue` | No API change needed — props stay the same. The `composerProps` computed in the mixin maps composerState to the existing props. | + +### Files to MODIFY (auto-refresh — Task 3) +| File | Changes | +|------|---------| +| `CommentThread.vue` | Add watcher on responsesCount → auto-refetch | +| `ComponentComments.vue` | Remove `$refs.thread.refresh()` | +| `UserComments.vue` | Remove `$refs.thread.refresh()` | + +### Test files to UPDATE +| Test file | What changes | +|-----------|-------------| +| `spec/javascript/mixins/ReplyComposerMixin.spec.js` | New — tests unified composerState | +| `spec/javascript/components/triage/TriageSplitView.spec.js` | Update open-reply-composer emission shape | +| `spec/javascript/components/rules/RuleReviews.spec.js` | Update open-reply-composer emission shape | +| `spec/javascript/components/components/CommentTriageModal.spec.js` | Update open-reply-composer emission shape | +| `spec/javascript/components/shared/CommentThread.spec.js` | Add auto-refresh watcher test | +| `spec/javascript/components/components/CommentComposerModal.spec.js` | May need minor prop test updates | + +### Test files that should NOT need changes +| Test file | Why | +|-----------|-----| +| `spec/javascript/components/components/CommentDedupBanner.spec.js` | Internal reply click handled within modal | + +--- + +## Task 1: Extract ReplyComposerMixin with unified composerState (sp:3, ~20 min) + +Create the mixin with the unified state object and standard methods. + +### composerState shape +```javascript +{ + mode: null, // 'reply' | 'new-comment' | 'component' | null + reviewId: null, // parent review ID (reply mode only) + ruleId: null, // rule being commented on + componentId: null, // component being commented on + section: null, // pre-selected section (new-comment mode) + ruleName: null, // display name for modal header +} +``` + +### Methods +```javascript +openReplyComposer({ reviewId, ruleId, componentId, ruleName }) +openSectionComposer({ ruleId, componentId, section, ruleName }) +openComponentComposer(componentId) +closeComposer() +onComposerPosted() // clears state, calls afterComposerPosted(reviewId) +onComposerHidden() // alias for closeComposer +``` + +### Computed +```javascript +composerActive // mode !== null +composerProps // maps composerState → CommentComposerModal prop names +``` + +### Hook +```javascript +afterComposerPosted(parentReviewId) // no-op in mixin, consumers override +``` + +### Files +- Create: `app/javascript/mixins/ReplyComposerMixin.vue` +- Create: `spec/javascript/mixins/ReplyComposerMixin.spec.js` + +--- + +## Task 2: Migrate all consumers + standardize event emissions (sp:5, ~35 min) + +### Order of migration (one at a time, test after each) +1. **UserComments.vue** (simplest — reply-only, Pattern A) +2. **ComponentComments.vue** (Pattern A + split-mode + component composer) +3. **ProjectComponent.vue** (Pattern B — biggest shape change) +4. **RulesCodeEditorView.vue** (Pattern B — mirrors ProjectComponent) + +### For each consumer +1. Add `ReplyComposerMixin` to mixins +2. Delete local composer data properties +3. Delete local composer methods (openReplyComposer, onComposerPosted, etc.) +4. Keep `afterComposerPosted()` override with screen-specific refresh logic +5. Update template: `` props bind via `composerProps` computed +6. Update `v-if` mount condition to use `composerActive` + +### Event emitters to standardize +- `RuleReviews.vue` — change `$emit('open-reply-composer', parentId)` to emit object +- `TriageSplitView.vue` — change to emit standardized object +- `CommentTriageModal.vue` — change to emit standardized object + +### Files +- Modify: ComponentComments.vue, ProjectComponent.vue, RulesCodeEditorView.vue, UserComments.vue +- Modify: RuleReviews.vue, TriageSplitView.vue, CommentTriageModal.vue +- Test: TriageSplitView.spec.js, RuleReviews.spec.js, CommentTriageModal.spec.js (emission shape) + +--- + +## Task 3: Auto-refresh CommentThread on responses_count change (sp:2, ~12 min) + +### Current (duplicated) +```javascript +// In ComponentComments + UserComments: +const thread = this.$refs[`thread-${id}`]; +thread?.refresh?.(); +``` + +### Target +```javascript +// In CommentThread.vue: +watch: { + responsesCount(newVal, oldVal) { + if (newVal > oldVal && this.expanded) this.fetchResponses(); + } +} +``` + +### Files +- Modify: CommentThread.vue (add watcher) +- Modify: ComponentComments.vue, UserComments.vue (remove $refs.thread.refresh()) +- Test: CommentThread.spec.js (new test for auto-refresh) + +--- + +## Dependency Graph +``` +Task 1 (extract mixin + composerState) + ↓ +Task 2 (migrate consumers + standardize events) + ↓ +Task 3 (auto-refresh CommentThread) +``` + +## Estimated Total: sp:10, ~67 min Claude-pace diff --git a/spec/javascript/mixins/ReplyComposerMixin.spec.js b/spec/javascript/mixins/ReplyComposerMixin.spec.js new file mode 100644 index 000000000..44d691b2b --- /dev/null +++ b/spec/javascript/mixins/ReplyComposerMixin.spec.js @@ -0,0 +1,198 @@ +import { describe, it, expect, vi } from "vitest"; +import { mount } from "@vue/test-utils"; +import { localVue } from "@test/testHelper"; +import ReplyComposerMixin from "@/mixins/ReplyComposerMixin.vue"; + +function createHost(methodOverrides = {}) { + const Host = { + mixins: [ReplyComposerMixin], + template: "
", + methods: methodOverrides, + }; + return mount(Host, { + localVue, + mocks: { $bvModal: { show: vi.fn() } }, + }); +} + +const replyPayload = { + reviewId: 42, + ruleId: 10, + componentId: 8, + ruleName: "CNTR-01-000001", +}; + +const sectionPayload = { + ruleId: 10, + componentId: 8, + section: "check_content", + ruleName: "CNTR-01-000001", +}; + +describe("ReplyComposerMixin", () => { + // ── Initial state ────────────────────────────────────────────────── + + it("initializes composerState with all null fields and mode null", () => { + const w = createHost(); + expect(w.vm.composerState).toEqual({ + mode: null, + reviewId: null, + ruleId: null, + componentId: null, + section: null, + ruleName: null, + }); + }); + + it("composerActive is false initially", () => { + const w = createHost(); + expect(w.vm.composerActive).toBe(false); + }); + + // ── openReplyComposer ────────────────────────────────────────────── + + it("sets mode to 'reply' with reviewId and rule context", async () => { + const w = createHost(); + w.vm.openReplyComposer(replyPayload); + await w.vm.$nextTick(); + expect(w.vm.composerState.mode).toBe("reply"); + expect(w.vm.composerState.reviewId).toBe(42); + expect(w.vm.composerState.ruleId).toBe(10); + expect(w.vm.composerState.componentId).toBe(8); + expect(w.vm.composerState.ruleName).toBe("CNTR-01-000001"); + expect(w.vm.composerState.section).toBe(null); + }); + + it("shows the modal after openReplyComposer", async () => { + const w = createHost(); + const showSpy = vi.spyOn(w.vm.$bvModal, "show"); + w.vm.openReplyComposer(replyPayload); + await w.vm.$nextTick(); + await w.vm.$nextTick(); + expect(showSpy).toHaveBeenCalledWith("comment-composer-modal"); + }); + + it("composerActive is true after openReplyComposer", async () => { + const w = createHost(); + w.vm.openReplyComposer(replyPayload); + await w.vm.$nextTick(); + expect(w.vm.composerActive).toBe(true); + }); + + // ── openSectionComposer ──────────────────────────────────────────── + + it("sets mode to 'new-comment' with section and rule context", async () => { + const w = createHost(); + w.vm.openSectionComposer(sectionPayload); + await w.vm.$nextTick(); + expect(w.vm.composerState.mode).toBe("new-comment"); + expect(w.vm.composerState.section).toBe("check_content"); + expect(w.vm.composerState.ruleId).toBe(10); + expect(w.vm.composerState.reviewId).toBe(null); + }); + + // ── openComponentComposer ────────────────────────────────────────── + + it("sets mode to 'component' with componentId only", async () => { + const w = createHost(); + w.vm.openComponentComposer(8); + await w.vm.$nextTick(); + expect(w.vm.composerState.mode).toBe("component"); + expect(w.vm.composerState.componentId).toBe(8); + expect(w.vm.composerState.ruleId).toBe(null); + expect(w.vm.composerState.reviewId).toBe(null); + }); + + // ── closeComposer ────────────────────────────────────────────────── + + it("resets all composerState fields to null", async () => { + const w = createHost(); + w.vm.openReplyComposer(replyPayload); + await w.vm.$nextTick(); + w.vm.closeComposer(); + expect(w.vm.composerState.mode).toBe(null); + expect(w.vm.composerState.reviewId).toBe(null); + expect(w.vm.composerActive).toBe(false); + }); + + // ── onComposerHidden (alias) ─────────────────────────────────────── + + it("onComposerHidden clears state (alias for closeComposer)", async () => { + const w = createHost(); + w.vm.openReplyComposer(replyPayload); + await w.vm.$nextTick(); + w.vm.onComposerHidden(); + expect(w.vm.composerState.mode).toBe(null); + }); + + // ── onComposerPosted ─────────────────────────────────────────────── + + it("clears state and calls afterComposerPosted with reviewId", async () => { + const afterSpy = vi.fn(); + const w = createHost({ afterComposerPosted: afterSpy }); + w.vm.openReplyComposer(replyPayload); + await w.vm.$nextTick(); + w.vm.onComposerPosted(); + expect(w.vm.composerState.mode).toBe(null); + expect(afterSpy).toHaveBeenCalledWith(42); + }); + + it("calls afterComposerPosted with null for component mode", async () => { + const afterSpy = vi.fn(); + const w = createHost({ afterComposerPosted: afterSpy }); + w.vm.openComponentComposer(8); + await w.vm.$nextTick(); + w.vm.onComposerPosted(); + expect(afterSpy).toHaveBeenCalledWith(null); + }); + + it("calls afterComposerPosted with null when composer was never opened", () => { + const afterSpy = vi.fn(); + const w = createHost({ afterComposerPosted: afterSpy }); + w.vm.onComposerPosted(); + expect(afterSpy).toHaveBeenCalledWith(null); + }); + + // ── composerProps computed ───────────────────────────────────────── + + it("maps composerState to CommentComposerModal props for reply mode", async () => { + const w = createHost(); + w.vm.openReplyComposer(replyPayload); + await w.vm.$nextTick(); + const props = w.vm.composerProps; + expect(props.componentId).toBe(8); + expect(props.ruleId).toBe(10); + expect(props.ruleDisplayedName).toBe("CNTR-01-000001"); + expect(props.replyToReviewId).toBe(42); + expect(props.initialSection).toBe(null); + }); + + it("maps composerState to props for new-comment mode", async () => { + const w = createHost(); + w.vm.openSectionComposer(sectionPayload); + await w.vm.$nextTick(); + const props = w.vm.composerProps; + expect(props.ruleId).toBe(10); + expect(props.initialSection).toBe("check_content"); + expect(props.replyToReviewId).toBe(null); + }); + + it("maps composerState to props for component mode", async () => { + const w = createHost(); + w.vm.openComponentComposer(8); + await w.vm.$nextTick(); + const props = w.vm.composerProps; + expect(props.componentId).toBe(8); + expect(props.ruleId).toBe(null); + expect(props.replyToReviewId).toBe(null); + expect(props.initialSection).toBe(null); + }); + + it("returns empty props when composer is closed", () => { + const w = createHost(); + const props = w.vm.composerProps; + expect(props.componentId).toBe(null); + expect(props.ruleId).toBe(null); + expect(props.replyToReviewId).toBe(null); + }); +}); From d496c538c6b0897d02c75659aeea1d9975f3eaba Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Tue, 19 May 2026 15:39:36 -0400 Subject: [PATCH 024/537] refactor: migrate all consumers to ReplyComposerMixin Replace Pattern A (composerReplyRow) and Pattern B (composerReplyToId + composerSection) across 4 consumers with unified composerState. Zero old pattern references remaining. afterComposerPosted receives state snapshot. 258/258 tests, ESLint clean, Playwright verified. Authored by: Aaron Lippold --- .../components/ComponentComments.vue | 56 ++++++--------- .../components/ProjectComponent.vue | 68 +++++++------------ .../components/rules/RulesCodeEditorView.vue | 61 +++++++---------- .../components/users/UserComments.vue | 34 ++++------ app/javascript/mixins/ReplyComposerMixin.vue | 9 +-- .../mixins/ReplyComposerMixin.spec.js | 18 +++-- 6 files changed, 100 insertions(+), 146 deletions(-) diff --git a/app/javascript/components/components/ComponentComments.vue b/app/javascript/components/components/ComponentComments.vue index 1d4b0b249..bd8e13344 100644 --- a/app/javascript/components/components/ComponentComments.vue +++ b/app/javascript/components/components/ComponentComments.vue @@ -53,7 +53,7 @@ variant="primary" size="sm" aria-label="Add component-level comment" - @click="openComponentComposer" + @click="openComponentComposerLocal" > Comment @@ -74,7 +74,7 @@ @adjudicated="onAdjudicated" @response-posted="onTriageResponsePosted" @destroyed="onDestroyed" - @open-reply-composer="openReplyComposer" + @open-reply-composer="openReplyComposerFromRow" @admin-panel-close="$emit('admin-panel-close')" /> @@ -125,7 +125,7 @@ :responses-count="item.responses_count || 0" :can-reply="canReply" class="mt-1" - @reply="openReplyComposer(item)" + @reply="openReplyComposerFromRow(item)" /> diff --git a/spec/javascript/components/triage/TriageSplitView.spec.js b/spec/javascript/components/triage/TriageSplitView.spec.js index eaccc82fb..399eee7ad 100644 --- a/spec/javascript/components/triage/TriageSplitView.spec.js +++ b/spec/javascript/components/triage/TriageSplitView.spec.js @@ -311,12 +311,21 @@ describe("TriageSplitView", () => { // ── Admin actions (migrated from modal) ──────────────────────────── - it("renders admin sidebar when adminPanelOpen prop is true", () => { + it("renders inline admin actions for admin users", () => { const w = mount(TriageSplitView, { localVue, - propsData: baseProps({ adminPanelOpen: true }), + propsData: baseProps({ effectivePermissions: "admin" }), }); - expect(w.find("#sidebar-admin-actions").exists()).toBe(true); + expect(w.find("[data-testid='admin-actions-inline']").exists()).toBe(true); + expect(w.find("[data-testid='admin-action-force-withdraw']").exists()).toBe(true); + }); + + it("hides admin actions for non-admin users", () => { + const w = mount(TriageSplitView, { + localVue, + propsData: baseProps({ effectivePermissions: "author" }), + }); + expect(w.find("[data-testid='admin-actions-inline']").exists()).toBe(false); }); it("posts to admin_withdraw with audit comment", async () => { From ec54b8e8ed4777ef1cf754a279dbc7dae6907b8d Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Wed, 20 May 2026 01:11:35 -0400 Subject: [PATCH 034/537] feat: move Commented/All toggle into rule context panel (Will #3) - Toggle moved from command bar to RuleContextPanel header - Sits next to rule title, directly above the sections it controls - Uses update:contextMode event chain through component hierarchy - Admin button already removed (previous commit) Authored by: Aaron Lippold --- .../components/ComponentComments.vue | 1 + .../components/ComponentTriagePage.vue | 21 ++-------------- .../components/triage/RuleContextPanel.vue | 24 ++++++++++++++++--- .../components/triage/TriageSplitView.vue | 1 + 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/app/javascript/components/components/ComponentComments.vue b/app/javascript/components/components/ComponentComments.vue index 51d010ea4..de847710d 100644 --- a/app/javascript/components/components/ComponentComments.vue +++ b/app/javascript/components/components/ComponentComments.vue @@ -91,6 +91,7 @@ :effective-permissions="effectivePermissions" :admin-panel-open="adminPanelOpen" :context-mode="contextMode" + @update:contextMode="$emit('update:contextMode', $event)" @exit="exitSplitMode" @triaged="onTriaged" @adjudicated="onAdjudicated" diff --git a/app/javascript/components/components/ComponentTriagePage.vue b/app/javascript/components/components/ComponentTriagePage.vue index 8de2e0c96..2d438ddc6 100644 --- a/app/javascript/components/components/ComponentTriagePage.vue +++ b/app/javascript/components/components/ComponentTriagePage.vue @@ -17,24 +17,7 @@ Back to Component Editor - + diff --git a/app/javascript/components/triage/RuleContextPanel.vue b/app/javascript/components/triage/RuleContextPanel.vue index 27a23bf64..708660e24 100644 --- a/app/javascript/components/triage/RuleContextPanel.vue +++ b/app/javascript/components/triage/RuleContextPanel.vue @@ -1,45 +1,39 @@ + +

+ Read-only — author or higher role required to triage. +

+ + +
+ + + +
+ + - Move to rule + placeholder="Comment ID" + data-testid="admin-action-confirmation-id" + /> +
+
+ + Cancel - Hard-delete + Confirm {{ adminConfirmLabel }}
-
- - - -
- - -
-
- - Cancel - - - Confirm {{ adminConfirmLabel }} - -
-
From f846c776d62a4ea41008c366a3ad76f8c81a66ca Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Wed, 20 May 2026 01:20:39 -0400 Subject: [PATCH 036/537] feat: rename Triage Queue to Comments (Will #6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Heading: Triage Queue → Comments / Project Comments - Breadcrumb: Triage → Comments - Back button: Back to Triage Table → Back to Comments Table - Aria label: triage queue → comments table Authored by: Aaron Lippold --- app/javascript/components/components/ComponentComments.vue | 2 +- .../components/components/ComponentTriagePage.vue | 6 +++--- app/javascript/components/project/ProjectTriagePage.vue | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/javascript/components/components/ComponentComments.vue b/app/javascript/components/components/ComponentComments.vue index de847710d..e0877c7b4 100644 --- a/app/javascript/components/components/ComponentComments.vue +++ b/app/javascript/components/components/ComponentComments.vue @@ -124,7 +124,7 @@ stacked="md" show-empty role="table" - aria-label="Component comments triage queue" + aria-label="Component comments table" > diff --git a/app/javascript/components/shared/RuleFormGroup.vue b/app/javascript/components/shared/RuleFormGroup.vue index 43dbb9d69..e465f3e00 100644 --- a/app/javascript/components/shared/RuleFormGroup.vue +++ b/app/javascript/components/shared/RuleFormGroup.vue @@ -3,12 +3,7 @@
- - -
+ +
Date: Fri, 22 May 2026 13:27:43 -0400 Subject: [PATCH 098/537] fix: by-rule view adds show more/less + triage button - Long comments truncated at 200 chars with show more/less toggle (matches table view behavior) - Triage button on each comment opens split-pane triage form - @triage event wired to openTriageFor in ComponentComments Authored by: Aaron Lippold --- .../components/components/CommentsByRule.vue | 21 ++++++++++++++++++- .../components/ComponentComments.vue | 1 + 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/javascript/components/components/CommentsByRule.vue b/app/javascript/components/components/CommentsByRule.vue index eb485d311..d7fd8c6d9 100644 --- a/app/javascript/components/components/CommentsByRule.vue +++ b/app/javascript/components/components/CommentsByRule.vue @@ -55,7 +55,17 @@ Posted on {{ comment.rule_displayed_name }} -

{{ comment.comment }}

+
+ {{ expanded[comment.id] ? comment.comment : comment.comment.substring(0, 200) + "…" }} + + {{ expanded[comment.id] ? "show less" : "show more" }} + +
+

{{ comment.comment }}

+ + Triage +
@@ -98,6 +116,7 @@ export default { data() { return { collapsed: {}, + expanded: {}, }; }, computed: { diff --git a/app/javascript/components/components/ComponentComments.vue b/app/javascript/components/components/ComponentComments.vue index 4026f3be9..1494ab726 100644 --- a/app/javascript/components/components/ComponentComments.vue +++ b/app/javascript/components/components/ComponentComments.vue @@ -151,6 +151,7 @@ :rows="rows" :all-expanded="allExpanded" @reaction-updated="updateRowInPlace" + @triage="openTriageFor($event)" /> From 81856fa688f635ea911cb583080ee59323e67e1a Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Fri, 22 May 2026 13:30:23 -0400 Subject: [PATCH 099/537] fix: add show more/less to split-pane triage view - Truncate long comments at 200 chars in the blockquote - Toggle expands/collapses with show more/show less link - Reset expanded state when navigating to next comment Authored by: Aaron Lippold --- .../components/triage/TriageSplitView.vue | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/app/javascript/components/triage/TriageSplitView.vue b/app/javascript/components/triage/TriageSplitView.vue index 25e712e21..64d5387ef 100644 --- a/app/javascript/components/triage/TriageSplitView.vue +++ b/app/javascript/components/triage/TriageSplitView.vue @@ -72,7 +72,21 @@ class="border-left pl-3 py-2 mb-2 bg-light" :class="triageBgClass(activeComment.triage_status)" > - {{ activeComment.comment }} + + Date: Fri, 22 May 2026 13:35:05 -0400 Subject: [PATCH 100/537] fix: compact sidebar badges + tooltip on rule names - Badge shortened from "12 pending / 12 total" to "12/12" - Full text in tooltip on hover - Warning color when pending > 0, secondary when resolved - Tooltip on rule name for truncated IDs Authored by: Aaron Lippold --- .../components/triage/TriageRuleSidebar.vue | 16 +++++++++++++--- .../components/triage/TriageRuleSidebar.spec.js | 3 +-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/app/javascript/components/triage/TriageRuleSidebar.vue b/app/javascript/components/triage/TriageRuleSidebar.vue index 129b6c46c..6af2c20c9 100644 --- a/app/javascript/components/triage/TriageRuleSidebar.vue +++ b/app/javascript/components/triage/TriageRuleSidebar.vue @@ -37,9 +37,19 @@ @keydown.enter="selectGroup(item.group)" @keydown.space.prevent="selectGroup(item.group)" > - {{ item.group.ruleName }} - - {{ item.group.pendingCount }} pending / {{ item.group.comments.length }} total + {{ item.group.ruleName }} + + {{ item.group.pendingCount }}/{{ item.group.comments.length }}
{ it("shows pending and total counts per rule", () => { const w = mount(TriageRuleSidebar, { localVue, propsData: baseProps() }); const headers = w.findAll("[data-testid='sidebar-rule-header']"); - expect(headers.at(1).text()).toContain("1 pending"); - expect(headers.at(1).text()).toContain("2 total"); + expect(headers.at(1).text()).toContain("1/2"); }); it("highlights the rule group containing the current comment", () => { From 868574197e88279d2f7a5efce453fc62ef599b21 Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Fri, 22 May 2026 13:44:22 -0400 Subject: [PATCH 101/537] fix: load all comments in split-pane + by-rule modes - Split-pane sidebar was showing 25/119 because per_page=25 - Now both split-pane and by-rule modes send per_page=1000 - Backend clamp raised from 100 to 1000 to allow full load - Pagination hidden in split-pane mode (same as by-rule) - Sidebar counts now match progress bar totals Authored by: Aaron Lippold --- .../components/components/ComponentComments.vue | 8 ++++---- app/models/component.rb | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/javascript/components/components/ComponentComments.vue b/app/javascript/components/components/ComponentComments.vue index 1494ab726..e2a72c3d5 100644 --- a/app/javascript/components/components/ComponentComments.vue +++ b/app/javascript/components/components/ComponentComments.vue @@ -283,7 +283,7 @@
Date: Fri, 22 May 2026 15:07:14 -0400 Subject: [PATCH 102/537] fix: wire TriageQueueNav into split-pane triage view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add prev/next rule and prev/next comment navigation bar - Shows "Rule X of Y — Comment X of Y" counter - Browse dropdown for jumping to any comment - Component existed but was never imported into TriageSplitView Authored by: Aaron Lippold --- app/javascript/components/triage/TriageSplitView.vue | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/javascript/components/triage/TriageSplitView.vue b/app/javascript/components/triage/TriageSplitView.vue index 64d5387ef..7c5328a6c 100644 --- a/app/javascript/components/triage/TriageSplitView.vue +++ b/app/javascript/components/triage/TriageSplitView.vue @@ -17,6 +17,14 @@
+ +
@@ -102,11 +105,11 @@ export default { const groups = []; const seen = new Map(); for (const c of this.comments) { - const key = c.rule_id || "component"; + const key = c.group_rule_displayed_name || c.rule_displayed_name || "(component)"; if (!seen.has(key)) { const group = { key, - ruleName: c.rule_displayed_name || "(component)", + ruleName: key, comments: [], pendingCount: 0, }; @@ -118,8 +121,8 @@ export default { if (c.triage_status === "pending") g.pendingCount++; } return groups.sort((a, b) => { - const aComp = a.key === "component"; - const bComp = b.key === "component"; + const aComp = a.key === "(component)"; + const bComp = b.key === "(component)"; if (aComp && !bComp) return -1; if (!aComp && bComp) return 1; return a.ruleName.localeCompare(b.ruleName, undefined, { numeric: true }); @@ -133,7 +136,8 @@ export default { activeGroupKey() { if (this.normalizedCurrentId == null) return null; const comment = this.comments.find((c) => c.id === this.normalizedCurrentId); - return comment ? comment.rule_id || "component" : null; + if (!comment) return null; + return comment.group_rule_displayed_name || comment.rule_displayed_name || "(component)"; }, flatItems() { const items = []; diff --git a/spec/javascript/components/triage/TriageRuleSidebar.spec.js b/spec/javascript/components/triage/TriageRuleSidebar.spec.js index 410dac5d6..26eaa857f 100644 --- a/spec/javascript/components/triage/TriageRuleSidebar.spec.js +++ b/spec/javascript/components/triage/TriageRuleSidebar.spec.js @@ -184,4 +184,81 @@ describe("TriageRuleSidebar", () => { const w = mount(TriageRuleSidebar, { localVue, propsData: baseProps() }); expect(w.find("[data-testid='sidebar-header']").text()).toContain("3 pending"); }); + + describe("parent/child grouping", () => { + const nestedComments = [ + { + id: 10, + rule_id: 100, + rule_displayed_name: "CNTR-00-000010", + group_rule_displayed_name: "CNTR-00-000010", + parent_rule_displayed_name: null, + triage_status: "pending", + comment: "Parent's own comment", + }, + { + id: 11, + rule_id: 200, + rule_displayed_name: "CNTR-00-001002", + group_rule_displayed_name: "CNTR-00-000010", + parent_rule_displayed_name: "CNTR-00-000010", + triage_status: "pending", + comment: "Child comment on 001002", + }, + { + id: 12, + rule_id: 300, + rule_displayed_name: "CNTR-00-001003", + group_rule_displayed_name: "CNTR-00-000010", + parent_rule_displayed_name: "CNTR-00-000010", + triage_status: "pending", + comment: "Child comment on 001003", + }, + { + id: 20, + rule_id: 400, + rule_displayed_name: "CNTR-00-000020", + group_rule_displayed_name: "CNTR-00-000020", + parent_rule_displayed_name: null, + triage_status: "pending", + comment: "Standalone parent comment", + }, + ]; + + function nestedProps(overrides = {}) { + return { comments: nestedComments, currentId: 10, ...overrides }; + } + + it("groups child comments under their parent control", () => { + const w = mount(TriageRuleSidebar, { localVue, propsData: nestedProps() }); + const headers = w.findAll("[data-testid='sidebar-rule-header']"); + expect(headers.length).toBe(2); + expect(headers.at(0).text()).toContain("CNTR-00-000010"); + expect(headers.at(1).text()).toContain("CNTR-00-000020"); + }); + + it("shows rolled-up count including children", () => { + const w = mount(TriageRuleSidebar, { localVue, propsData: nestedProps() }); + const headers = w.findAll("[data-testid='sidebar-rule-header']"); + expect(headers.at(0).text()).toContain("3/3"); + }); + + it("tracks active group correctly when current comment is on a child", () => { + const w = mount(TriageRuleSidebar, { + localVue, + propsData: nestedProps({ currentId: 11 }), + }); + const headers = w.findAll("[data-testid='sidebar-rule-header']"); + expect(headers.at(0).classes()).toContain("sidebar-rule--active"); + }); + + it("expands parent group when a child comment is active", () => { + const w = mount(TriageRuleSidebar, { + localVue, + propsData: nestedProps({ currentId: 11 }), + }); + const items = w.findAll("[data-testid='sidebar-comment-item']"); + expect(items.length).toBe(3); + }); + }); }); From a5597db5565ae582b0555aba1609a9b945d07ee5 Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Fri, 22 May 2026 23:06:43 -0400 Subject: [PATCH 105/537] feat: sidebar collapse toggle + flex scroll fix - Click parent group to expand/collapse (toggle) - Enter always selects (expand + navigate to first comment) - Space toggles (same as click) - Auto-expand group when navigating to its comment externally - Flex layout replaces hardcoded max-height for scroll - expandedGroups object map decoupled from activeGroupKey Authored by: Aaron Lippold --- .../components/triage/TriageRuleSidebar.vue | 47 +++++++++++++++++-- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/app/javascript/components/triage/TriageRuleSidebar.vue b/app/javascript/components/triage/TriageRuleSidebar.vue index ede4534cf..ac2e78192 100644 --- a/app/javascript/components/triage/TriageRuleSidebar.vue +++ b/app/javascript/components/triage/TriageRuleSidebar.vue @@ -33,9 +33,9 @@ role="option" :aria-selected="String(isActiveGroup(item.group))" tabindex="-1" - @click="selectGroup(item.group)" + @click="toggleGroup(item.group)" @keydown.enter="selectGroup(item.group)" - @keydown.space.prevent="selectGroup(item.group)" + @keydown.space.prevent="toggleGroup(item.group)" > 0 ? this.focusedIndex - 1 : items.length - 1; } this.$nextTick(() => this.scrollFocusedIntoView()); - } else if (event.key === "Enter" || event.key === " ") { + } else if (event.key === "Enter") { event.preventDefault(); if (this.focusedIndex >= 0 && this.focusedIndex < items.length) { const item = items[this.focusedIndex]; @@ -181,6 +201,16 @@ export default { this.$emit("select", item.comment.id); } } + } else if (event.key === " ") { + event.preventDefault(); + if (this.focusedIndex >= 0 && this.focusedIndex < items.length) { + const item = items[this.focusedIndex]; + if (item.type === "group") { + this.toggleGroup(item.group); + } else { + this.$emit("select", item.comment.id); + } + } } }, scrollFocusedIntoView() { @@ -194,9 +224,16 @@ export default { diff --git a/app/javascript/components/triage/TriageRuleSidebar.vue b/app/javascript/components/triage/TriageRuleSidebar.vue index ac2e78192..88b49ee55 100644 --- a/app/javascript/components/triage/TriageRuleSidebar.vue +++ b/app/javascript/components/triage/TriageRuleSidebar.vue @@ -162,6 +162,13 @@ export default { } }, }, + normalizedCurrentId(id) { + if (id == null) return; + const idx = this.flatItems.findIndex( + (item) => item.type === "comment" && item.comment.id === id, + ); + if (idx >= 0) this.focusedIndex = idx; + }, }, methods: { isActiveGroup(group) { diff --git a/spec/javascript/components/components/ComponentComments.spec.js b/spec/javascript/components/components/ComponentComments.spec.js index 2bcc9d325..539e58540 100644 --- a/spec/javascript/components/components/ComponentComments.spec.js +++ b/spec/javascript/components/components/ComponentComments.spec.js @@ -927,4 +927,56 @@ describe("ComponentComments", () => { expect(callArgs[1].params.rule_id).toBeUndefined(); }); }); + + // ── Fix 1: Hide Expand All in split mode ─────────────────────────── + + it("hides Expand All toggle when in split-pane mode", async () => { + const wrapper = mount(ComponentComments, { + propsData: { componentId: 42 }, + stubs: SHARED_STUBS, + }); + await flushPromises(wrapper); + wrapper.vm.viewMode = "by-rule"; + wrapper.vm.splitMode = true; + await wrapper.vm.$nextTick(); + expect(wrapper.find("[data-testid='expand-all']").exists()).toBe(false); + }); + + it("shows Expand All toggle in by-rule mode when NOT in split mode", async () => { + const wrapper = mount(ComponentComments, { + propsData: { componentId: 42 }, + stubs: SHARED_STUBS, + }); + await flushPromises(wrapper); + wrapper.vm.viewMode = "by-rule"; + wrapper.vm.splitMode = false; + await wrapper.vm.$nextTick(); + expect(wrapper.find("[data-testid='expand-all']").exists()).toBe(true); + }); + + // ── Fix 3: Right-align Export CSV + Comment in split mode ────────── + + it("adds ml-auto to export button in split mode", async () => { + const wrapper = mount(ComponentComments, { + propsData: { componentId: 42, effectivePermissions: "author" }, + stubs: SHARED_STUBS, + }); + await flushPromises(wrapper); + wrapper.vm.splitMode = true; + await wrapper.vm.$nextTick(); + expect(wrapper.vm.canExportDisposition).toBe(true); + expect(wrapper.vm.splitMode).toBe(true); + }); + + it("adds ml-auto to comment button when export hidden in split mode", async () => { + const wrapper = mount(ComponentComments, { + propsData: { componentId: 42, effectivePermissions: "viewer" }, + stubs: SHARED_STUBS, + }); + await flushPromises(wrapper); + wrapper.vm.splitMode = true; + await wrapper.vm.$nextTick(); + expect(wrapper.vm.canExportDisposition).toBe(false); + expect(wrapper.vm.splitMode).toBe(true); + }); }); diff --git a/spec/javascript/components/triage/RuleContextPanel.spec.js b/spec/javascript/components/triage/RuleContextPanel.spec.js index 1dd7853e2..15cea7f89 100644 --- a/spec/javascript/components/triage/RuleContextPanel.spec.js +++ b/spec/javascript/components/triage/RuleContextPanel.spec.js @@ -289,89 +289,107 @@ describe("RuleContextPanel", () => { }); }); - // ── Related comments list in expanded section (agw.11) ───────────── - - describe("related comments list", () => { - const relatedComments = [ - { id: 1, section: "check_content", author_name: "Viewer", comment: "First comment", triage_status: "pending" }, - { id: 2, section: "check_content", author_name: "Reviewer", comment: "Second comment", triage_status: "concur" }, - { id: 3, section: "fixtext", author_name: "Author", comment: "Fix comment", triage_status: "pending" }, + // ── Fix 2: No inline comments in triage panel ────────────────────── + // REQUIREMENT: Sidebar handles comment navigation — inline comment + // stubs in the rule content panel are a duplicate affordance that + // violates Nielsen's consistency heuristic. + + it("does NOT render inline comment stubs under section headers", () => { + const sectionComments = [ + { id: 1, section: "check_content", author_name: "Viewer", comment: "First", triage_status: "pending" }, + { id: 2, section: "check_content", author_name: "Reviewer", comment: "Second", triage_status: "concur" }, ]; + const w = mount(RuleContextPanel, { + localVue, + propsData: props({ + focusedSection: "check_content", + sectionComments, + activeCommentId: 1, + sectionCommentCounts: { check_content: 2 }, + }), + }); + expect(w.findAll(".related-comment").length).toBe(0); + expect(w.findAll(".related-comments-list").length).toBe(0); + }); - it("renders related comments below the expanded section body", () => { - const w = mount(RuleContextPanel, { - localVue, - propsData: props({ - focusedSection: "check_content", - sectionComments: relatedComments, - activeCommentId: 1, - sectionCommentCounts: { check_content: 2, fixtext: 1 }, - }), - }); - const section = w.find('[data-section="check_content"]'); - const relatedList = section.findAll(".related-comment"); - expect(relatedList.length).toBe(2); + it("still shows section comment count badges after removing inline comments", () => { + const w = mount(RuleContextPanel, { + localVue, + propsData: props({ sectionCommentCounts: { check_content: 3, fixtext: 1 } }), }); + const checkHeader = w.find('[data-section="check_content"] .section-header'); + expect(checkHeader.text()).toContain("(3)"); + }); - it("shows author name and truncated text for each related comment", () => { + // ── Fix 4: Advanced fields toggle (default collapsed) ────────────── + // REQUIREMENT: Advanced fields (status_justification, version, + // rule_weight, etc.) should be hidden by default in triage mode. + // Triagers can expand them but shouldn't see the full kitchen sink. + + describe("advanced fields toggle", () => { + it("hides advanced fields by default", () => { + const contentWithAdvanced = { + ...ruleContent, + status_justification: "Because reasons", + version: "003.001", + rule_weight: "10.0", + }; const w = mount(RuleContextPanel, { localVue, - propsData: props({ - focusedSection: "check_content", - sectionComments: relatedComments, - activeCommentId: 1, - sectionCommentCounts: { check_content: 2 }, - }), + propsData: props({ ruleContent: contentWithAdvanced }), }); - const items = w.findAll('[data-section="check_content"] .related-comment'); - expect(items.at(0).text()).toContain("Viewer"); - expect(items.at(1).text()).toContain("Reviewer"); + expect(w.find('[data-section="status_justification"]').exists()).toBe(false); + expect(w.find('[data-section="version"]').exists()).toBe(false); }); - it("highlights the active comment in the related list", () => { + it("shows advanced fields when toggle is enabled", async () => { + const contentWithAdvanced = { + ...ruleContent, + status_justification: "Because reasons", + version: "003.001", + }; const w = mount(RuleContextPanel, { localVue, - propsData: props({ - focusedSection: "check_content", - sectionComments: relatedComments, - activeCommentId: 1, - sectionCommentCounts: { check_content: 2 }, - }), + propsData: props({ ruleContent: contentWithAdvanced }), }); - const items = w.findAll('[data-section="check_content"] .related-comment'); - expect(items.at(0).classes()).toContain("related-comment--active"); - expect(items.at(1).classes()).not.toContain("related-comment--active"); + w.vm.showAdvanced = true; + await w.vm.$nextTick(); + expect(w.find('[data-section="status_justification"]').exists()).toBe(true); + expect(w.find('[data-section="version"]').exists()).toBe(true); }); - it("emits select-comment when a related comment is clicked", async () => { - const w = mount(RuleContextPanel, { - localVue, - propsData: props({ - focusedSection: "check_content", - sectionComments: relatedComments, - activeCommentId: 1, - sectionCommentCounts: { check_content: 2 }, - }), - }); - const items = w.findAll('[data-section="check_content"] .related-comment'); - await items.at(1).trigger("click"); - expect(w.emitted("select-comment")).toBeTruthy(); - expect(w.emitted("select-comment")[0][0]).toBe(2); + it("renders an Advanced Fields toggle control", () => { + const w = mount(RuleContextPanel, { localVue, propsData: props() }); + expect(w.find("[data-testid='advanced-fields-toggle']").exists()).toBe(true); + }); + }); + + // ── Fix 5: Focused section background highlight ──────────────────── + + it("applies focus background tint to the focused section body", () => { + const w = mount(RuleContextPanel, { + localVue, + propsData: props({ focusedSection: "check_content" }), }); + const body = w.find('[data-section="check_content"] .section-body'); + expect(body.classes()).toContain("section-body--focused"); + }); - it("does not render related comments for collapsed sections", () => { - const w = mount(RuleContextPanel, { - localVue, - propsData: props({ - focusedSection: "check_content", - sectionComments: relatedComments, - activeCommentId: 1, - sectionCommentCounts: { fixtext: 1 }, - }), - }); - const fixSection = w.find('[data-section="fixtext"]'); - expect(fixSection.findAll(".related-comment").length).toBe(0); + it("does NOT apply focus tint to non-focused sections", () => { + const w = mount(RuleContextPanel, { + localVue, + propsData: props({ focusedSection: "check_content" }), }); + const body = w.find('[data-section="fixtext"] .section-body'); + expect(body.classes()).not.toContain("section-body--focused"); + }); + + // ── Fix 6: Toggle label is "Focus Section" not "All Fields" ─────── + + it("labels the context mode toggle as 'Focus Section'", () => { + const w = mount(RuleContextPanel, { localVue, propsData: props() }); + expect(w.text()).toContain("Focus Section"); + expect(w.text()).not.toContain("All Fields"); }); // ── Locked status ────────────────────────────────────────────────── diff --git a/spec/javascript/components/triage/TriageRuleSidebar.spec.js b/spec/javascript/components/triage/TriageRuleSidebar.spec.js index 26eaa857f..752f97a55 100644 --- a/spec/javascript/components/triage/TriageRuleSidebar.spec.js +++ b/spec/javascript/components/triage/TriageRuleSidebar.spec.js @@ -185,6 +185,30 @@ describe("TriageRuleSidebar", () => { expect(w.find("[data-testid='sidebar-header']").text()).toContain("3 pending"); }); + // ── Fix 7: Keyboard nav syncs with clicks ────────────────────────── + // REQUIREMENT: When user clicks a comment, keyboard navigation + // should continue from that position, not restart from the top. + + it("syncs focusedIndex to the clicked comment position", async () => { + const w = mount(TriageRuleSidebar, { localVue, propsData: baseProps({ currentId: 1 }) }); + await w.setProps({ currentId: 3 }); + await w.vm.$nextTick(); + const idx = w.vm.flatItems.findIndex( + (item) => item.type === "comment" && item.comment.id === 3, + ); + expect(w.vm.focusedIndex).toBe(idx); + }); + + it("keyboard ArrowDown from synced position goes to next item", async () => { + const w = mount(TriageRuleSidebar, { localVue, propsData: baseProps({ currentId: 1 }) }); + await w.setProps({ currentId: 2 }); + await w.vm.$nextTick(); + const syncedIdx = w.vm.focusedIndex; + const list = w.find("[data-testid='sidebar-list']"); + await list.trigger("keydown", { key: "ArrowDown" }); + expect(w.vm.focusedIndex).toBe(syncedIdx + 1); + }); + describe("parent/child grouping", () => { const nestedComments = [ { From 5fa0c6df3505e2209a83dcce24839625a14f99fb Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Sat, 23 May 2026 09:40:34 -0400 Subject: [PATCH 107/537] fix: constrain split-pane columns to viewport height Each column (sidebar, rule content, triage form) now scrolls independently within the viewport instead of extending the page. Uses calc(100vh - 320px) with min-height: 400px fallback. Authored by: Aaron Lippold --- .../components/triage/TriageSplitView.vue | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/app/javascript/components/triage/TriageSplitView.vue b/app/javascript/components/triage/TriageSplitView.vue index 81097eb03..c3997acc4 100644 --- a/app/javascript/components/triage/TriageSplitView.vue +++ b/app/javascript/components/triage/TriageSplitView.vue @@ -25,9 +25,9 @@ @select="onQueueSelect" /> - - - - +
{{ activeComment.rule_displayed_name }} — {{ activeComment.section || "Overall" }}
@@ -55,7 +55,7 @@ @@ -530,6 +530,19 @@ export default { overflow-y: auto; } +/* Three-tier dark mode: sidebar = secondary-bg, content = body-bg, form = tertiary-bg */ +.triage-columns ::v-deep > .border-right { + border-color: var(--vulcan-border-color) !important; + background-color: var(--vulcan-secondary-bg, var(--vulcan-component-bg)); + padding-left: 0.5rem; +} + +.triage-form-panel { + border-left: 1px solid var(--vulcan-border-color); + background-color: var(--vulcan-tertiary-bg, var(--vulcan-component-bg-alt)); + padding-left: 1rem; +} + @media (min-width: 992px) { .triage-columns { /* 320px = navbar ~56px + breadcrumb ~40px + page header ~60px From 26396e1607201b4cd34dc7dbeb8c65f6fb50f03d Mon Sep 17 00:00:00 2001 From: Aaron Lippold Date: Tue, 2 Jun 2026 21:14:24 -0400 Subject: [PATCH 267/537] =?UTF-8?q?fix:=20triage=20split-pane=20shared=20p?= =?UTF-8?q?anel=20pattern=20=E2=80=94=20padding=20+=20three-tier=20bg=20+?= =?UTF-8?q?=20borders?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaced per-component padding hacks with shared .triage-panel base class: - .triage-panel: consistent padding (0.75rem 1rem) on all three panes - .triage-panel--sidebar: secondary-bg + right border - .triage-panel--content: inherits body-bg (no override) - .triage-panel--form: tertiary-bg + left border Reverted RuleContextPanel padding hack — padding now comes from panel base. All three panes visually distinct in dark mode with consistent interior spacing. IRL verified via Playwright: content text properly indented, panels separated. Authored by: Aaron Lippold --- .../components/triage/TriageSplitView.vue | 35 +++++++++++++------ 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/app/javascript/components/triage/TriageSplitView.vue b/app/javascript/components/triage/TriageSplitView.vue index f6969df92..6111f83b4 100644 --- a/app/javascript/components/triage/TriageSplitView.vue +++ b/app/javascript/components/triage/TriageSplitView.vue @@ -27,7 +27,7 @@
- + - +
{{ activeComment.rule_displayed_name }} — {{ activeComment.section || "Overall" }}
@@ -55,7 +61,7 @@ @@ -530,17 +536,26 @@ export default { overflow-y: auto; } -/* Three-tier dark mode: sidebar = secondary-bg, content = body-bg, form = tertiary-bg */ -.triage-columns ::v-deep > .border-right { - border-color: var(--vulcan-border-color) !important; +/* ── Shared panel base ───────────────────────────────────────────── + ONE class for padding + overflow on all three panes. + Modifiers add border + background per the three-tier hierarchy. */ +.triage-panel { + padding: 0.75rem 1rem; +} + +.triage-panel--sidebar { background-color: var(--vulcan-secondary-bg, var(--vulcan-component-bg)); - padding-left: 0.5rem; + border-right: 1px solid var(--vulcan-border-color); + padding-right: 0.5rem; } -.triage-form-panel { - border-left: 1px solid var(--vulcan-border-color); +.triage-panel--content { + /* body-bg — inherited, no override needed */ +} + +.triage-panel--form { background-color: var(--vulcan-tertiary-bg, var(--vulcan-component-bg-alt)); - padding-left: 1rem; + border-left: 1px solid var(--vulcan-border-color); } @media (min-width: 992px) { From 0ff19347a1d410d39d0e0eeabc7b60b34fb8490e Mon Sep 17 00:00:00 2001 From: Will Dower Date: Tue, 2 Jun 2026 22:40:57 -0400 Subject: [PATCH 268/537] In-app verification fixes: bulk filter, projectId, reply tint, docs nav Four fixes from the verification round: - Bulk-triage/merge switch filter to 'all' when applied status would hide affected rows (was: bulk-Accepted rows vanished from Pending). - ComponentTriagePage now passes projectId so the response-template dropdown reaches the triage form (was: silently hidden). - CommentThread wrapper picks up the parent triage tint (was: only inner reply cards tinted). - VitePress nav gains Comment Triage + Sidebar-Nested Requirements; sidebar shared between Getting Started + User Guide. Signed-off-by: Will Dower --- .../components/ComponentComments.vue | 15 ++++++++ .../components/ComponentTriagePage.vue | 1 + .../components/shared/CommentThread.vue | 2 +- docs/.vitepress/config.js | 34 +++++++++++++----- .../components/ComponentComments.spec.js | 35 +++++++++++++++++++ 5 files changed, 78 insertions(+), 9 deletions(-) diff --git a/app/javascript/components/components/ComponentComments.vue b/app/javascript/components/components/ComponentComments.vue index 3a94ad9d8..d885d9043 100644 --- a/app/javascript/components/components/ComponentComments.vue +++ b/app/javascript/components/components/ComponentComments.vue @@ -799,6 +799,14 @@ export default { try { await submitBulkTriage(ids, payload); this.clearSelection(); + // If the bulk-applied status would hide the just-triaged rows from + // the current filter (e.g. filter=pending, applied=concur), drop + // the filter to 'all' so the triager can verify the action took + // effect in place rather than the rows silently vanishing. + if (this.filterStatus !== "all" && this.filterStatus !== payload.triage_status) { + this.filterStatus = "all"; + this.page = 1; + } await this.fetch(); this.alertOrNotifyResponse({ data: { @@ -823,6 +831,13 @@ export default { await submitMerge(payload.review_ids, payload.survivor_id); this.clearSelection(); this.selectedMergeReviews = []; + // Merge produces two statuses on the affected rows (secondaries = + // 'duplicate', survivor keeps its current status) — to keep both + // visible for verification, drop any narrow filter to 'all'. + if (this.filterStatus !== "all") { + this.filterStatus = "all"; + this.page = 1; + } await this.fetch(); this.alertOrNotifyResponse({ data: { diff --git a/app/javascript/components/components/ComponentTriagePage.vue b/app/javascript/components/components/ComponentTriagePage.vue index 14eb5ceb8..8ad330ba6 100644 --- a/app/javascript/components/components/ComponentTriagePage.vue +++ b/app/javascript/components/components/ComponentTriagePage.vue @@ -35,6 +35,7 @@ ref="comments" scope="component" :component-id="component.id" + :project-id="project.id" :component-displayed-name="component.name" :component-prefix="component.prefix" :effective-permissions="effectivePermissions" diff --git a/app/javascript/components/shared/CommentThread.vue b/app/javascript/components/shared/CommentThread.vue index edceb4ac0..28df94c53 100644 --- a/app/javascript/components/shared/CommentThread.vue +++ b/app/javascript/components/shared/CommentThread.vue @@ -25,7 +25,7 @@
-
+
Loading replies…
- -
- -
+ + + - - diff --git a/app/javascript/components/shared/CommentThread.vue b/app/javascript/components/shared/CommentThread.vue index 28df94c53..003123fef 100644 --- a/app/javascript/components/shared/CommentThread.vue +++ b/app/javascript/components/shared/CommentThread.vue @@ -36,7 +36,7 @@ v-for="reply in replies" v-else :key="reply.id" - class="ml-3 mt-2 pl-3 border-left border-info" + class="ml-3 mt-2 pl-3 comment-thread__reply" :class="parentTriageBgClass" >

@@ -197,4 +197,8 @@ export default { .white-space-pre-wrap { white-space: pre-wrap; } + +.comment-thread__reply { + border-left: 2px solid var(--vulcan-info); +} diff --git a/app/javascript/components/shared/SectionCommentIcon.vue b/app/javascript/components/shared/SectionCommentIcon.vue index 3ada4e189..3045f55a6 100644 --- a/app/javascript/components/shared/SectionCommentIcon.vue +++ b/app/javascript/components/shared/SectionCommentIcon.vue @@ -97,7 +97,7 @@ export default { } .section-comment-icon .clickable:focus-visible, .section-comment-icon .clickable:hover { - outline: 2px solid var(--primary, #007bff); + outline: 2px solid var(--vulcan-primary); outline-offset: 2px; border-radius: 2px; } diff --git a/app/javascript/components/triage/BulkTriageBar.vue b/app/javascript/components/triage/BulkTriageBar.vue index cecb94022..0f3a83ee7 100644 --- a/app/javascript/components/triage/BulkTriageBar.vue +++ b/app/javascript/components/triage/BulkTriageBar.vue @@ -1,26 +1,27 @@