add static BI index page for CE - #1623
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Each widget now has its own route and partial so tag filtering only refreshes the relevant widget, not the full page. Issue bar colors reflect the tag of each finding.
Flatten the bi_index/ partial subdirectory into static_pages/ with bi_ prefix, matching the rest of the app. Refactor the controller to put private methods in alphabetical order, use Issue.all instead of a non-existent issuelib filter, short-circuit Tag.exists? before filtering, capture Time.current once, and move top project/team property data into dedicated private methods. Add Teams stat to set_bi_stats. Update CSS to scope all BI min-height overrides inside the body selector block to fix specificity.
Load the Issues and Top Issues widgets lazily via data-behavior=fetch, matching Pro's architecture. Rewrite bi_index.js to attach widget-filter change handlers on dradis:fetch and turbo:frame-render events so they bind after the fetch completes rather than at turbo:load. Add mb-auto to the top_issues turbo frame and move chart-wrap inside widget-content so justify-content-center on the fetch container no longer shifts content to the middle when filtering causes the frame to collapse. Make Teams and Contributors stat cards open the upsell modal via js-try-pro.
Add the projects/1/ prefix to match issuelib and remediationtracker.
etdsoft
left a comment
There was a problem hiding this comment.
Automated peer review -- PR #1623
Reviewers: Claude (sub-agent 1), Codex (sub-agent 2)
Verdict: request-changes
Findings: 0 High, 4 Medium, 2 Low
What I checked
This is an automated peer code review by two independent LLM sub-agents reading the same rubric. Findings flagged by both agents are higher confidence; findings flagged by only one are still worth your attention. The verdict is computed from severity counts -- there is no editorial judgment in the merge step.
Both reviewers agreed
F1 [Medium|correctness] app/assets/javascripts/hera/pages/bi_index.js:17 -- change handler accumulates on every frame render
What: Inside the turbo:frame-render handler, $widgetFilter.on('change', ...) binds a new listener each time the event fires. The widget-filter select lives outside the <turbo-frame> so it persists across frame replacements. After the first filter change triggers turbo:frame-render, the handler runs again and adds a second change listener. After N filter changes, requestSubmit() fires N+1 times per change.
Why: Each redundant requestSubmit() sends a duplicate GET to the server and toggles spinner/content visibility multiple times. UX degrades progressively -- after a few filter changes the widget flickers and the server handles unnecessary requests. Additionally, the outer $(document).on(...) at line 3 is bound inside turbo:load, so navigating away and back stacks another delegated listener on document, compounding the issue.
How to fix: Unbind before rebinding: $widgetFilter.off('change').on('change', function(e) { ... }). Or use a single delegated handler on document bound once outside turbo:load:
$(document).on('change', 'body.static_pages.bi_index [data-behavior~=widget-filter]', function(e) {
const $container = $(e.target).parents('[data-behavior~=fetch]');
$container.find('[data-behavior~=fetch-loader]').removeClass('d-none');
$container.find('[data-behavior~=widget-content]').addClass('d-none');
e.target.closest('form').requestSubmit();
});F2 [Medium|tests] spec:0 -- no specs for new controller actions or helper
What: The PR adds three controller actions (bi_index, bi_insights_issues, bi_insights_top_issues), a helper module (StaticPagesHelper), and branching logic in yoy_delta (three code paths). None have specs.
Why: set_bi_issues_data runs live DB queries with time-range filtering, tag validation (Tag.exists?), and a YoY calculation with a zero-division guard. Off-by-one errors in the date range or a missed edge case will silently produce wrong numbers on the dashboard. Without specs, regressions are only caught visually.
How to fix: Add request specs covering at minimum:
bi_insights_issuesreturns counts scoped to the current yearbi_insights_issueswith atagparam filters correctly (and ignores nonexistent tags)bi_insights_top_issuesreturns issues ordered by count, limited to 10yoy_deltaedge cases: previous == 0 with current > 0 (returns 100), both zero (returns 0), normal case
# spec/requests/static_pages/bi_spec.rb
RSpec.describe 'Static BI pages', type: :request do
before { login_as(create(:user)) }
describe 'GET /projects/1/addons/bi/insights/issues' do
it 'returns issue stats for the current year' do
create(:issue, created_at: Time.current.beginning_of_year + 1.day)
get static_bi_insights_issues_path
expect(response).to be_successful
end
it 'ignores nonexistent tag names' do
get static_bi_insights_issues_path, params: { tag: 'nonexistent' }
expect(response).to be_successful
end
end
endClaude only
F3 [Medium|performance] app/controllers/static_pages_controller.rb:146 -- Ruby-level group_by loads all current-year issues into memory
What: set_bi_issues_data builds @bi_top_issues via .includes(:tags).group_by(&:title) on the filtered AR relation. Enumerable#group_by materializes the entire result set (all issues created this year, with eager-loaded tags) into Ruby objects, then groups, sorts, and takes the top 10 in memory.
Why: On an instance with thousands of issues per year this loads every issue row plus its tag associations for every lazy-load request and every filter change. The work could be done in SQL with GROUP BY title, COUNT(*) LIMIT 10, fetching only the 10 rows needed.
How to fix: Replace the Ruby grouping with a SQL aggregate:
top_titles = filtered_issues
.where(created_at: current_year_start..now)
.group(:title)
.order('count_all DESC')
.limit(10)
.count # => { "SQL Injection" => 12, ... }
@bi_top_issues = top_titles.map do |title, count|
issue = filtered_issues.where(title: title).includes(:tags).first
{ title: title, count: count, issue: issue }
endF4 [Low|consistency] app/assets/stylesheets/hera/views.scss:2 -- SCSS import out of alphabetical order
What: @import 'hera/views/static_pages' is inserted between activities and boards. Alphabetically static_pages sorts after search and before styles.
Why: CLAUDE.md requires SCSS @import directives to be sorted alphabetically within their section.
How to fix: Move the import to after search and before styles:
@import 'hera/views/search';
@import 'hera/views/static_pages';
@import 'hera/views/styles';F5 [Low|hygiene] CHANGELOG:2 -- entry does not follow module/entity + future-tense-verb format
What: The entry reads Show don't gate: Business Intelligence. CLAUDE.md changelog convention requires Entity: future-tense-verb description.
Why: The project convention frames changelog entries as "What will this upgrade do to my instance?" starting with the entity, then a future-tense verb. "Show don't gate" is a strategy label, not an entity.
How to fix: Rewrite as: Business Intelligence: add static BI dashboard page with year-over-year insights
Codex only
F6 [Medium|performance] app/controllers/static_pages_controller.rb:127 -- both endpoints share one expensive aggregation
What: set_bi_issues_data is invoked for both bi_insights_issues and bi_insights_top_issues. The full aggregation (count + in-memory top-issues grouping) runs for both endpoints on every initial page load, even though each endpoint only needs half the data.
Why: Initial page load fires both endpoints in parallel, so the full aggregation runs twice. Large installations can incur substantial memory use and slow or time out dashboard requests.
How to fix: Split the data loading by action -- bi_insights_issues only needs the counts; bi_insights_top_issues only needs the grouped top list. Use two narrower before_action callbacks or conditional logic inside set_bi_issues_data.
Notes
- Claude F3 and Codex F6 both point to the same in-memory aggregation problem in
set_bi_issues_data. They were bucketed separately because the flagged line numbers differ by more than 5 (146 vs 127), but they describe the same root cause. Claude's finding includes the SQL fix. Tag.allis called in both partials during rendering. Tags are typically a small table so this is not a performance concern, but the convention would be to set@tagsin the controller'sset_bi_issues_dataand pass it to the view._bi_top_project_properties.html.erband_bi_top_team_properties.html.erbare near-identical. When static data is replaced with live queries, consider consolidating into a single partial with local variables.
Reviewed automatically. Raw outputs in the shared product reviews directory.
Prevent change handler accumulation by calling .off('change') before
.on('change') in bi_index.js so each turbo:frame-render doesn't stack
an additional listener on the persisted widget-filter element. Move the
SCSS import to its correct alphabetical position. Rewrite the CHANGELOG
entry to follow the Entity: future-tense-verb convention.
Cover current-year date scoping, tag filtering (valid and nonexistent), top-issues grouping and ordering, the 10-result limit, and yoy_delta edge cases (both zero, previous zero with current > 0, normal case).
Static BI routes are CE-only (gated by !defined?(Dradis::Pro)), so the path helpers don't exist in Pro and the specs would fail.
Summary
Adds a static Business Intelligence dashboard page to CE (
/projects/1/addons/bi), surfacing year-over-year insights that mirror the layout of the Pro BI dashboard.What's included:
data-behavior="fetch"+ Turbo Framesjs-try-pro)Architecture notes:
data-behavior="fetch"lazy-loading pattern, consistent with Probi_index.jslistens fordradis:fetchandturbo:frame-renderto attachwidget-filterchange handlers after content loads, matching Pro'sdashboard.jspatternprojects/1/addons/...convention used by issuelib and remediationtrackerTesting steps
Other Information
The static placeholder data (Teams, Contributors, Top Project/Team Properties) will be replaced with live queries once the corresponding Pro features are ported.
I assign all rights, including copyright, to any future Dradis work by myself to Security Roots.
Check List