Skip to content

feat(adoption-insights): add estimated time saved insights for Scaffolder runs - #5426

Open
djanickova wants to merge 9 commits into
redhat-developer:mainfrom
djanickova:adoption-insights-time-saved
Open

djanickova wants to merge 9 commits into
redhat-developer:mainfrom
djanickova:adoption-insights-time-saved

Conversation

@djanickova

@djanickova djanickova commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Hey, I just made a Pull Request!

RHIDP-15190

This adds estimated time saved to the Adoption Insights backend. For each newly created Scaffolder task, the backend reads the template’s backstage.io/time-saved annotation from the catalog and stores the estimate in minutes with the analytics event. The estimate is taken from the catalog, not from the value submitted by the browser. New events are associated with the signed-in Backstage user.

GET /api/adoption-insights/events?type=time_saved query returns totals for the requested date range:

  • minutes: sum of available estimates
  • runs: Scaffolder tasks created
  • runs_with_estimate: tasks created from templates with a valid estimate

A missing or invalid estimate still counts as a run, but adds no minutes. Runs are counted when Scaffolder creates a task, before it completes; a task that later fails or is cancelled still contributes its estimate.

The existing type=top_templates response also includes runs and minutes for each template. Its existing count and last_used fields retain their meaning: clicks on the wizard’s final Create button, which occur before the task creation request. A failed submission can therefore increase count without increasing runs.

Top Templates still ranks by those clicks by default. The optional sort_by=runs ranks by created task runs instead; limit applies to the selected ranking. Stored minutes preserve the estimate from each run, so editing an annotation does not recalculate earlier savings.

How to test

  1. Start the Adoption Insights workspace locally with yarn start.
  2. Add an estimate to a catalog Template, for example:
   metadata:
     annotations:
       backstage.io/time-saved: PT15M
  1. Open /create and submit the template.
  2. Query both endpoints for a date range containing the submission. Compare responses before and after, since the range may already contain events:
GET /api/adoption-insights/events?type=time_saved&start_date=<start-date>&end_date=<end-date>&timezone=Europe%2FPrague
GET /api/adoption-insights/events?type=top_templates&start_date=<start-date>&end_date=<end-date>&timezone=Europe%2FPrague&limit=10

Expected behavior:

  • With PT15M, one created task adds 15 to minutes, 1 to runs, and 1 to runs_with_estimate. Its Top Templates row gains 1 run and 15 minutes; with the default button label, count also increases by 1.
  • With no annotation or an invalid annotation, a created task increases runs but does not increase minutes or runs_with_estimate.
  • With PT1H30M, a created task adds 90 minutes. With PT0M, it adds zero minutes but increases runs_with_estimate.
  • After the catalog reflects an annotation change from PT15M to PT30M, one submission at each value totals 45 minutes. The earlier event retains its recorded estimate.
  • A task that fails or is cancelled after creation still contributes its run and valid estimate.
  • sort_by=clicks gives the same ordering as omitting sort_by; sort_by=runs orders templates by created tasks. Normal successful submissions may produce the same ordering in both modes. To observe a difference, a final Create click whose task creation request fails increases count but not runs.

For a single PT15M run in an otherwise empty range, type=time_saved returns:

{
  "data": [
    {
      "minutes": 15,
      "runs": 1,
      "runs_with_estimate": 1
    }
  ]
}

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
@rhdh-gh-app

rhdh-gh-app Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-adoption-insights-backend workspaces/adoption-insights/plugins/adoption-insights-backend minor v1.1.1
@red-hat-developer-hub/backstage-plugin-analytics-module-adoption-insights workspaces/adoption-insights/plugins/analytics-module-adoption-insights patch v1.1.1

@codecov

codecov Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.39%. Comparing base (0b024a2) to head (f2663a1).
⚠️ Report is 43 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5426      +/-   ##
==========================================
+ Coverage   65.35%   65.39%   +0.04%     
==========================================
  Files        2758     2758              
  Lines      110579   110678      +99     
  Branches    31085    31109      +24     
==========================================
+ Hits        72264    72373     +109     
+ Misses      37739    37729      -10     
  Partials      576      576              
Flag Coverage Δ *Carryforward flag
adoption-insights 88.39% <96.66%> (+1.07%) ⬆️
ai-integrations 90.56% <ø> (ø) Carriedforward from 2a45ff9
app-defaults 74.89% <ø> (ø) Carriedforward from 2a45ff9
augment 46.67% <ø> (ø) Carriedforward from 2a45ff9
boost 93.37% <ø> (ø) Carriedforward from 2a45ff9
bulk-import 73.12% <ø> (ø) Carriedforward from 2a45ff9
cost-management 13.56% <ø> (ø) Carriedforward from 2a45ff9
dcm 74.35% <ø> (ø) Carriedforward from 2a45ff9
e2e-adoption-insights 60.00% <ø> (ø) Carriedforward from 2a45ff9
e2e-extensions 62.42% <ø> (ø) Carriedforward from 2a45ff9
e2e-global-header 52.43% <ø> (ø) Carriedforward from 2a45ff9
e2e-homepage 61.25% <ø> (ø) Carriedforward from 2a45ff9
e2e-intelligent-assistant 46.02% <ø> (ø) Carriedforward from 2a45ff9
e2e-orchestrator 49.49% <ø> (ø) Carriedforward from 2a45ff9
e2e-orchestrator-plugin 49.48% <ø> (ø) Carriedforward from 2a45ff9
e2e-quickstart 54.83% <ø> (ø) Carriedforward from 2a45ff9
e2e-scorecard 49.88% <ø> (ø) Carriedforward from 2a45ff9
e2e-theme 16.43% <ø> (ø) Carriedforward from 2a45ff9
extensions 58.11% <ø> (ø) Carriedforward from 2a45ff9
global-floating-action-button 71.18% <ø> (ø) Carriedforward from 2a45ff9
global-header 69.15% <ø> (ø) Carriedforward from 2a45ff9
homepage 86.56% <ø> (ø) Carriedforward from 2a45ff9
install-dynamic-plugins 84.02% <ø> (ø) Carriedforward from 2a45ff9
intelligent-assistant 79.49% <ø> (ø) Carriedforward from 2a45ff9
konflux 91.98% <ø> (ø) Carriedforward from 2a45ff9
lightspeed 69.02% <ø> (ø) Carriedforward from 2a45ff9
mcp-integrations 84.46% <ø> (ø) Carriedforward from 2a45ff9
orchestrator 77.69% <ø> (ø) Carriedforward from 2a45ff9
quickstart 65.83% <ø> (ø) Carriedforward from 2a45ff9
sandbox 79.56% <ø> (ø) Carriedforward from 2a45ff9
scorecard 92.32% <ø> (ø) Carriedforward from 2a45ff9
theme 88.21% <ø> (ø) Carriedforward from 2a45ff9
translations 7.91% <ø> (ø) Carriedforward from 2a45ff9
x2a 79.43% <ø> (ø) Carriedforward from 2a45ff9

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b024a2...f2663a1. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@djanickova

Copy link
Copy Markdown
Member Author

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:33 PM UTC · Completed 4:00 PM UTC

Commit: 7081aa4 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $8.59

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Nov 2, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Feature addition across 16 files with a large blast radius and 911 lines of change, but limited to a single low-churn plugin workspace with one recent author, solid test coverage (38%), no security-sensitive paths, and no regression history — composite 1.68 rounds to moderate (2).

Previous run

Risk Assessment: moderate (2/5)

Details

A moderate-sized feature addition (13 files, ~400 lines) confined to the adoption-insights workspace with no protected paths, no security-sensitive changes, a reasonable test ratio (0.38), and negligible recent churn on the affected files.

@fullsend-ai-review

fullsend-ai-review Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Review

Findings

Low

  • [Naming conventions / interface-break] workspaces/adoption-insights/plugins/adoption-insights-backend/src/database/event-database.ts:62 — The method is renamed from getTopTemplateViews to getTopTemplates, breaking the *Views suffix convention of peer methods (getTopPluginViews, getTopTechDocsViews, getTopCatalogEntitiesViews). The rename is intentional — the method now returns task run counts and estimated minutes saved in addition to click counts, so Views would be misleading. The EventDatabase interface is internal to this package with a single implementation (BaseDatabaseAdapter), both updated in this PR.

  • [scope-expansion] workspaces/adoption-insights/plugins/adoption-insights-backend/src/controllers/EventApiController.ts:163 — The user_ref override from the authenticated principal applies to every accepted analytics event, not just Scaffolder creates. This is broader behavioral scope than the PR title suggests. The changeset and analytics module README explicitly document this behavior: "Newly accepted events of every type use the authenticated user identity for user_ref."

  • [architectural-coherence] workspaces/adoption-insights/plugins/adoption-insights-backend/src/database/event-database.ts:59 — getTimeSaved() returns Promise<TimeSavedTotals> (the actual resolved shape) while all peer methods declare Promise<Knex.QueryBuilder<T>> (misleading — they also resolve to ResponseData<T>). This exposes a pre-existing type convention inconsistency without worsening it.

  • [Error handling gap] workspaces/adoption-insights/plugins/adoption-insights-backend/src/controllers/EventApiController.ts:168 — The processIncomingEvents forEach loop queues valid events via addEvent() before validating subsequent ones. If event N fails validation, events 0–N-1 are already queued and will persist despite the 400 response. Pre-existing behavior not introduced by this PR.

  • [intent-alignment] workspaces/adoption-insights/plugins/adoption-insights-backend/src/controllers/EventApiController.ts:194 — The trackEvents handler now requires user authentication (httpAuth.credentials with allow: ['user']) where it previously accepted any caller and filtered out events without context.userId. Non-user clients (service accounts, backend-to-backend calls) previously tolerated will now receive 401/403 errors.

  • [Edge case] workspaces/adoption-insights/plugins/adoption-insights-backend/src/database/adapters/BaseAdapter.ts:194 — CAST(value as DOUBLE PRECISION) on the TEXT value column could throw invalid input syntax on PostgreSQL if non-numeric strings were manually inserted for scaffolder create events. SQLite is unaffected due to type affinity.

  • [Code organization] workspaces/adoption-insights/plugins/adoption-insights-backend/src/database/adapters/BaseAdapter.ts:279 — getTimeSaved() is the only public query method in BaseDatabaseAdapter that lacks the async keyword. All peer methods (getTopTemplates, getTopSearches, getDailyUsers, getUsers, getTopPluginViews, getTopTechDocsViews, getTopCatalogEntitiesViews) are declared async.
    Remediation: Add async to the method signature: async getTimeSaved(): Promise<TimeSavedTotals>.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run

Review

Findings

Medium

  • [missing-authorization] workspaces/adoption-insights/.changeset/open-shoes-rule.md:1 — No linked issue for this non-trivial feature (397 lines, 13 files). The PR adds a new analytics query type, CatalogService dependency, catalog lookups on Scaffolder create events, and server-side user attribution for all analytics events. Non-trivial changes require explicit authorization via a linked issue.
    Remediation: Open or reference an issue that authorizes the "estimated time saved" feature.

  • [scope-creep] workspaces/adoption-insights/plugins/adoption-insights-backend/src/controllers/EventApiController.ts:157 — The processIncomingEvents method overwrites context.userName with the server-side userRef for every analytics event, not just Scaffolder creates. This changes how user_ref is populated for all event types (active_users, top_plugins, top_searches, etc.). While the PR description mentions "New events are associated with the signed-in Backstage user," the breadth of this behavioral change across all event types warrants explicit acknowledgment in the changeset.
    Remediation: Either constrain the userName overwrite to Scaffolder create events only, or explicitly document the broader user attribution change as an intentional behavioral change.

  • [architectural-coherence] workspaces/adoption-insights/plugins/adoption-insights-backend/src/database/adapters/BaseAdapter.ts:189 — getTimeSaved() aggregates the value column across all historical scaffolder create rows. Before this PR, value stored client-submitted data; after, it stores catalog annotation minutes or NULL. Date ranges spanning the deployment boundary mix incompatible semantics. On PostgreSQL, CAST(value AS DOUBLE PRECISION) will throw for non-numeric historical text; on SQLite it silently returns 0.0.
    Remediation: Consider adding a dedicated database column for time-saved values, or document the epoch date at which the aggregate becomes reliable.

Low

  • [error-handling-gap] workspaces/adoption-insights/plugins/adoption-insights-backend/src/controllers/EventApiController.ts:206 — The catch block unconditionally accesses error.details.fieldErrors, assuming every thrown error is a ValidationError. While the router middleware provides first-line credential checking, the pattern is fragile if additional throwing calls are added to the try block.
    Remediation: Guard the error type before accessing .details.fieldErrors.

  • [sql-portability] workspaces/adoption-insights/plugins/adoption-insights-backend/src/database/adapters/BaseAdapter.ts:197 — CAST(value AS DOUBLE PRECISION) on a TEXT column behaves differently across database engines for non-numeric input (PostgreSQL throws, SQLite returns 0.0).

  • [naming-convention] workspaces/adoption-insights/plugins/adoption-insights-backend/src/types/event.ts:77 — TimeSavedTotals uses the suffix "Totals" which has no precedent in this file; other types use "Count" or domain nouns.
    Remediation: Consider a name consistent with the existing pattern, though note that "Count" may be misleading for this aggregate type.

  • [code-organization] workspaces/adoption-insights/plugins/adoption-insights-backend/src/types/event.ts:77 — The row shape is an inline anonymous type; every other item shape in this file is a named interface.
    Remediation: Extract a named interface (e.g., TimeSavedRow) and reference it in the response type alias.

  • [naming-convention] workspaces/adoption-insights/plugins/adoption-insights-backend/src/database/adapters/BaseAdapter.ts:194 — Uppercase AS in CAST expressions is inconsistent with the lowercase as used in all other CAST calls in this file.
    Remediation: Change AS to lowercase as in the three db.raw strings inside getTimeSaved().

  • [naming-alignment] workspaces/adoption-insights/plugins/adoption-insights-backend/src/types/event-request.ts:22 — The new time_saved query type uses a past-participle form that differs from the existing noun-phrase convention (top_*, total_*, active_*).
    Remediation: Rename the query type to align with the existing naming convention before the feature ships.

  • [code-organization] workspaces/adoption-insights/plugins/adoption-insights-backend/src/types/event-request.ts:21 — time_saved splits the top_* group in the QUERY_TYPES array.
    Remediation: Move time_saved to immediately after active_users so the top_* entries form an unbroken group.

  • [code-organization] workspaces/adoption-insights/plugins/adoption-insights-backend/src/database/event-database.ts:60 — getTimeSaved() fragments the getTop* method cluster in the EventDatabase interface.
    Remediation: Move getTimeSaved() to follow getDailyUsers() so all getTop* methods remain contiguous.

  • [stale-doc] workspaces/adoption-insights/plugins/analytics-module-adoption-insights/README.md:89 — States omitting identityApi prevents user context from being sent to the backend. The backend now independently stores the server-authenticated user's entity ref, making this partially misleading.
    Remediation: Update the sentence to clarify that the backend now independently stores server-authenticated user identity regardless of client-side identityApi configuration.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
@djanickova

Copy link
Copy Markdown
Member Author

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:19 PM UTC · Completed 3:46 PM UTC

Commit: f63c308 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.84

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
@djanickova

Copy link
Copy Markdown
Member Author

Seems like the sonarqube check did not run on the latest commit? The two new issues are outdated

@djanickova
djanickova marked this pull request as ready for review November 5, 2026 12:30
@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Add catalog-backed time-saved insights for Scaffolder runs

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Resolve Scaffolder time-saved estimates from catalog templates and associate accepted events with
 authenticated users.
• Add time-saved totals and per-template run metrics while preserving default click-based ranking.
• Cover aggregation, sorting, authentication, and CSV responses; document the new query behavior.
Diagram

graph TD
  Analytics["Analytics batches"] --> Controller["Event controller"] --> Catalog["Catalog service"]
  Controller --> Batch["Batch processor"] --> Store[("Events database")] --> Queries["Insights queries"] --> Client["JSON or CSV"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Resolve estimates when querying insights
  • ➕ Avoid catalog lookups during event ingestion.
  • ➖ Annotation edits would recalculate historical savings.
  • ➖ Aggregate queries would depend on catalog availability.
2. Accept browser-submitted estimates
  • ➕ Avoid backend catalog access and ingestion latency.
  • ➖ Clients could submit incorrect or manipulated values.

Recommendation: Keep the PR's ingestion-time catalog lookup and stored estimate: it makes historical totals stable and avoids trusting browser values. Missing or unavailable estimates still leave a countable run.

Files changed (16) +927 / -58

Enhancement (7) +266 / -27
EventApiController.tsEnrich task events and expose time-saved queries +103/-13

Enrich task events and expose time-saved queries

• Resolves Scaffolder task estimates from catalog annotations with user credentials and stores the authenticated user ref on accepted events. Dispatches the new aggregate and expanded top-template queries, while distinguishing validation errors from other failures.

workspaces/adoption-insights/plugins/adoption-insights-backend/src/controllers/EventApiController.ts

BaseAdapter.tsAggregate task savings and template runs +125/-8

Aggregate task savings and template runs

• Adds date-filtered task totals and joins per-template runs and stored minutes to existing click metrics. Supports ranking and limiting by either clicks or runs.

workspaces/adoption-insights/plugins/adoption-insights-backend/src/database/adapters/BaseAdapter.ts

event-database.tsExtend the event database query contract +6/-2

Extend the event database query contract

• Adds the time-saved query and replaces the top-template method contract with one supporting run metrics and an optional sort mode.

workspaces/adoption-insights/plugins/adoption-insights-backend/src/database/event-database.ts

plugin.tsInject catalog service into event handling +4/-0

Inject catalog service into event handling

• Registers the catalog service dependency and passes it to the event controller for authenticated template lookups.

workspaces/adoption-insights/plugins/adoption-insights-backend/src/plugin.ts

event-request.tsDefine time-saved and template-sort request types +5/-0

Define time-saved and template-sort request types

• Adds time_saved to supported query types and defines the clicks-or-runs sort option.

workspaces/adoption-insights/plugins/adoption-insights-backend/src/types/event-request.ts

event.tsType new savings and template response fields +13/-3

Type new savings and template response fields

• Defines aggregate minutes, runs, and estimated-run counts, plus per-template runs and minutes. Allows a null last-used click timestamp for templates ranked solely by runs.

workspaces/adoption-insights/plugins/adoption-insights-backend/src/types/event.ts

event-request.tsValidate the optional top-template sort mode +10/-1

Validate the optional top-template sort mode

• Accepts clicks or runs for top_templates and rejects sort_by on other insight types.

workspaces/adoption-insights/plugins/adoption-insights-backend/src/validation/event-request.ts

Tests (6) +604 / -19
EventApiController.test.tsTest authenticated event enrichment and controller errors +159/-15

Test authenticated event enrichment and controller errors

• Covers catalog estimates overriding client values, missing or invalid annotations, failed lookups, and authenticated attribution for other events. Also tests validation, authentication, and unexpected-error handling.

workspaces/adoption-insights/plugins/adoption-insights-backend/src/controllers/EventApiController.test.ts

EventBatchProcessor.test.tsAlign processor database mocks with new queries +2/-1

Align processor database mocks with new queries

• Updates the mocked database interface for time-saved and renamed top-template methods.

workspaces/adoption-insights/plugins/adoption-insights-backend/src/domain/EventBatchProcessor.test.ts

plugin.api.test.tsExercise insight responses against seeded databases +391/-1

Exercise insight responses against seeded databases

• Tests permission enforcement, totals, sorting, limits, run-only templates, and CSV output. Checks value aggregation on both SQLite and PostgreSQL.

workspaces/adoption-insights/plugins/adoption-insights-backend/src/plugin.api.test.ts

plugin.test.tsUpdate allowed-query validation expectation +1/-1

Update allowed-query validation expectation

• Adds time_saved to the expected list of supported insight types.

workspaces/adoption-insights/plugins/adoption-insights-backend/src/plugin.test.ts

router.test.tsSupply authenticated credentials and catalog mock +7/-1

Supply authenticated credentials and catalog mock

• Updates router test setup for the controller's new catalog dependency and authenticated event processing.

workspaces/adoption-insights/plugins/adoption-insights-backend/src/router.test.ts

event-request.test.tsTest template sort-parameter validation +44/-0

Test template sort-parameter validation

• Verifies both supported sort values and rejection of unsupported values or use outside top_templates.

workspaces/adoption-insights/plugins/adoption-insights-backend/src/validation/event-request.test.ts

Documentation (3) +57 / -12
open-shoes-rule.mdRecord package releases and behavior changes +8/-0

Record package releases and behavior changes

• Declares a minor backend release and patch analytics-module release. Summarizes catalog-backed estimates, query additions, and authenticated user attribution.

workspaces/adoption-insights/.changeset/open-shoes-rule.md

README.mdDocument time-saved and top-template queries +48/-9

Document time-saved and top-template queries

• Describes response fields, click-versus-run ranking, CSV support, permissions, and how stored estimates behave when annotations are missing or later edited.

workspaces/adoption-insights/plugins/adoption-insights-backend/README.md

README.mdClarify analytics identity requirements +1/-3

Clarify analytics identity requirements

• Documents the required identity API, hashed client user ID, credential forwarding, and backend use of the authenticated principal rather than client-supplied userName.

workspaces/adoption-insights/plugins/analytics-module-adoption-insights/README.md

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Code Review by Qodo


View low (1)
Informational
1. Users can still inflate runs and minutes saved 🐞 Bug
Description
processIncomingEvents accepts client-supplied Scaffolder create fields, while getTimeSaved and
buildTemplateRunTotalsQuery count every matching stored event without linking it to a real task or
deduplicating it. Any signed-in user can repeatedly submit forged events for a readable template,
increasing reported runs and, when its catalog annotation supplies an estimate, minutes.
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Run and time-saved totals treat client-reported Scaffolder create events as evidence of task creation, allowing a signed-in user to inflate runs and catalog-derived minutes with forged events.

## Fix Focus Areas
- workspaces/adoption-insights/plugins/adoption-insights-backend/src/controllers/EventApiController.ts[145-164]
- workspaces/adoption-insights/plugins/adoption-insights-backend/src/database/adapters/BaseAdapter.ts[278-294]
- workspaces/adoption-insights/plugins/adoption-insights-backend/README.md[156-158]

## Recommended Fix
Populate run metrics from a trusted Scaffolder backend task-creation signal, or include a task id in each event and verify and deduplicate it against a server-side Scaffolder task record before counting it. Catalog annotation resolution must not serve as task verification. If run counts remain client-reported instead, state clearly in the README that they can be forged.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


@rhdh-qodo-merge

Copy link
Copy Markdown

Important

The /generate_labels command by Qodo was deprecated on the 1st of October 2026 and is no longer available. We recommend switching to the latest Qodo review capabilities. Learn more

Signed-off-by: Diana Janickova <djanicko@redhat.com>
@djanickova

Copy link
Copy Markdown
Member Author

Just addressed the high and medium points from Qodo's review.

I hardened POST /events against unbounded catalog lookups from a single batch:

  • Validate before enrich - events are now validated up front, so rejected batches never trigger catalog calls.
  • Dedup + bounded concurrency - distinct template refs are resolved once per batch (max 5 concurrent lookups) instead of one unbounded Promise.all per event.
  • Batch-size cap - oversized batches are rejected with 400 before any lookup. Configurable via app.analytics.adoptionInsights.maxEventsPerRequest (default 100); also added an explicit 1mb JSON body limit

Note: validation is now all-or-nothing per batch (previously valid events before the first invalid one could be partially persisted). Documented in the README/changeset and covered by new tests.

Signed-off-by: Diana Janickova <djanicko@redhat.com>
@sonarqubecloud

sonarqubecloud Bot commented Nov 6, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant