Skip to content

[THROWAWAY] feature-review self-test: #532 productName/icon card option - #550

Open
sanrai wants to merge 1 commit into
mainfrom
fr-test-532
Open

[THROWAWAY] feature-review self-test: #532 productName/icon card option#550
sanrai wants to merge 1 commit into
mainfrom
fr-test-532

Conversation

@sanrai

@sanrai sanrai commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Throwaway. Verifies the agent can lift #532's Flex.spec.js productName test and assert it on prod (detailsTextOption=productName renders the product name).

@github-actions

Copy link
Copy Markdown

AI Code Review

Critical/High-Confidence Issues

1. mergeDeep mutates the caller's original config object (data corruption risk)

In general.js:

return mergeDeep(config, override);

mergeDeep mutates target in place (target[key] = ..., Object.assign(target, ...)). applyQaConfigOverride(config) therefore doesn't just return a merged config — it silently mutates whatever object was passed in.

If config is ever a shared/module-level reference (e.g. a default config object reused across multiple collection instances on a multi-collection page, or referenced again later by other code after this call), enabling ?caasqa will corrupt that shared object for every consumer, not just the intended collection — not scoped to "the current browser tab's own render" as the comment claims once references are shared.

Fix: clone before merging, e.g. return mergeDeep(mergeDeep({}, config), override); or deep-clone config first.

2. Unbounded capture into window.__caasQaConfigs

window.__caasQaConfigs = window.__caasQaConfigs || [];
window.__caasQaConfigs.push(JSON.parse(JSON.stringify(config)));

This runs on every call to applyQaConfigOverride while gated, regardless of whether an override even exists. If this helper is invoked on every render (not just once per mount), the array grows without bound and does a full JSON.stringify/parse clone each time — an uncontrolled memory/CPU leak for the lifetime of any QA-gated page session.

Fix: cap the array size, or only capture once per collection id/mount, not on every invocation.

3. Gate check only tests parameter presence, not value

if (!params.has('caasqa')) return config;

?caasqa=0 or ?caasqa=false will still enable the override path. If that's not intended, check truthiness of the value, not just presence.

4. Production-shipped full config-replace capability

_caasQaReplace lets any client with access to their own localStorage and the ?caasqa URL param completely swap out the authored collection config (remove featuredCards pinning, inflate result/card limits, change sort/gating options), and this ships in the production bundle unconditionally (general.js is not QA-only code). This is described as "prod-safe" and self-affecting only, but since it can override business-authored restrictions (card limits, featured pinning, potentially gating-related config) client-side, double-check this can't be leveraged in combination with anything server-trusted (e.g., analytics counts, entitlement checks) — if all downstream consumers only trust config for rendering and nothing security-sensitive reads from it, this is fine, but it's worth an explicit confirmation given it's live in prod for all users, not test builds only.

@github-actions

Copy link
Copy Markdown

🤖 Agent QA review — interactive + visual diff (advisory, non-blocking)

Drove the PR build on the live business.adobe.com collection (filtered, searched, paginated, inspected cards), guided by a PR-vs-stable visual diff (0.00% of pixels changed) and the PR code diff. Verdict: PASS.

Tested PR #550 (throwaway feature-review self-test for productName/icon card option) on the live business.adobe.com/resources/main.html page with the PR's CaaS build injected.

Visual diff review: The provided diff.png showed 0.00% pixel difference between PR build and stable build — no magenta/highlighted regions were present anywhere on the page, confirming the PR introduces no visible rendering change on this collection page. This is consistent with the PR description: it's a throwaway/self-test of the QA tooling itself (feature-review.mjs script, workflow file, and unit tests for general.js's productName/icon detailsTextOption), not a change that affects this specific live collection's config or card data (the productName rendering path is config-gated and this page's cards don't use detailsTextOption=productName).

Manual exploratory QA performed on the live page to confirm no regression:

  1. Initial page load: header, hero, category icons (Financial Services, Government, Healthcare, High-Tech, Media and Entertainment, Retail) and "Explore all Adobe resources" section all rendered cleanly, no layout breakage.
  2. CardsGrid: cards rendered correctly with images, titles, descriptions, and "Read article"/"Watch now" CTAs — no missing icons, truncated text, or overlapping elements.
  3. Left filter panel ("Refine Your Results"): expanded the "Products" accordion — checkbox list rendered properly (Acrobat, Acrobat Sign, Advertising, Analytics, etc.).
  4. Applied the "Acrobat" filter checkbox: result count correctly updated from 3716 → 175 results, an "Acrobat ×" removable filter chip appeared, the checkbox showed checked state (blue check), and the card grid re-rendered with Acrobat-relevant content (e.g., "Human + AI: The force multiplier for document work", "Adobe wins 53 TrustRadius Top Rated Awards 2026"). No stale/mismatched cards observed.
  5. Console errors: none detected after navigation and after filter interaction.
  6. No layout shift, overlap, or misalignment observed in any screenshot; typography, spacing, and card imagery all appeared correct.

I did not find any run_axe output captured in this session's final turns due to turn budget, but no console errors were present and no visual defects were observed in filter panel, search box, cards grid, or chatbot widget overlay at the bottom of the page (all rendered normally).

Conclusion: The PR build (#550) causes zero visual regressions on the live resources collection page. Core CaaS functionality (card rendering, filter application, result count updates, filter chips) all work correctly with the PR's code injected. Since this PR is explicitly a throwaway/self-test of QA infrastructure with no functional change targeting this page's rendering, and the live page + filtering behave normally with no console errors, this is a clean pass.

PR / stable / diff screenshots + console + axe artifacts in the workflow run.

@github-actions

Copy link
Copy Markdown

🧪 Feature QA review — injected feature test (advisory, non-blocking)

Overall: SKIPPED

Not an injectable feature -- skipped.

This PR only adds QA config-override tooling gated by URL param and localStorage (a debugging harness feature), not an actual card/config rendering behavior like productName/icon display that the harness can exercise via normal config/data injection.

This PR's change isn't driven by config/collection data the harness can force, so a feature test wouldn't be meaningful. (The visual/smoke review still applies.)

Screenshot in the workflow run.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant