[CDX-357]: added result page attribute

[niizom]

Pull Request Checklist

Before you submit a pull request, please make sure you have to following:

• I have added or updated TypeScript types for my changes, ensuring they are compatible with the existing codebase.
• I have added JSDoc comments to my TypeScript definitions for improved documentation.
• I have added tests that prove my fix is effective or that my feature works.
• I have added any necessary documentation (if appropriate).
• I have made sure my PR is up-to-date with the main branch.

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no API changes)
• Documentation content changes
• TypeScript type definitions update
• Other: Added new data attribute

[Copilot]

Pull request overview

Adds a new Constructor.io beacon data attribute to expose the current results page on PLP containers, and extends existing unit/component tests to validate it.

Changes:

• Added data-cnstrc-result-page to cnstrcDataAttrs.common.
• Included the new resultPage attribute in PLP container data attributes for both search and browse.
• Updated utility + client/server component tests to assert the new attribute is rendered.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/utils/dataAttributeHelpers.ts	Adds the resultPage attribute constant and emits it for search/browse PLP container attributes.
spec/utils/dataAttributeHelpers.test.tsx	Verifies getPlpContainerCnstrcDataAttributes includes resultPage.
spec/components/CioPlpGrid/CioPlpGrid.test.jsx	Verifies the client-rendered PLP container includes data-cnstrc-result-page.
spec/components/CioPlpGrid/CioPlpGrid.server.test.jsx	Verifies the server-rendered markup includes data-cnstrc-result-page.
.gitignore	Ignores IntelliJ .idea directory.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[constructor-claude-bedrock]

Code Review Results

✅ Strengths

Good small, focused change with tests covering both browse and search page types, plus the server-side render path.

🚨 Critical Issues

• [File: src/utils/dataAttributeHelpers.ts Line: L88] The resultPage value is read from requestConfigs.page (the user-supplied request config), but the intent of a data-cnstrc-result-page beacon attribute is to reflect the page the API actually returned. These can diverge if the API normalises the page (e.g. clamps an out-of-range value). The source of truth should be data.request.page from the API response, consistent with how resultId, numResults, and searchTerm are already read from data. The tests in spec/utils/dataAttributeHelpers.test.tsx:109 and :136 also assert against mockSearchData.request.page / mockBrowseData.request.page (the API response), so the implementation disagrees with its own test assertions — those tests only pass coincidentally because both the mock requestConfigs.page (undefined → fallback 1) and the fixture's request.page happen to equal 1.

  Suggested fix:

  // Read from the API response, not the request config
  const resultPage = data.request?.page ?? 1;

⚠️ Important Issues

• [File: spec/components/CioPlpGrid/CioPlpGrid.test.jsx Line: L267] Redundant double-conversion: String(mockBrowseData.request.page.toString()). toString() already returns a string, so wrapping it in String() is unnecessary. The equivalent search-test line (L326) correctly uses only .toString(). Use one or the other consistently.

• [File: src/utils/dataAttributeHelpers.ts Line: L88] No test covers the || 1 default fallback (i.e. when requestConfigs.page is undefined). If the fallback is intentional it should have an explicit test; if the correct source becomes data.request?.page (see critical issue above) the fallback semantics should also be verified.

💡 Suggestions

• [File: spec/utils/dataAttributeHelpers.test.tsx Lines: L109, L136] The new test assertions compare against mockSearchData.request.page / mockBrowseData.request.page (values from the API response object), but the mock RequestConfigs objects passed to the function don't set page at all. Consider adding a test where mockRequestConfigs.page is explicitly set to a different value from the fixture's response page to make the data-source distinction observable and prevent silent regressions.

• [File: .gitignore Line: L4] The addition of .idea is fine, but it is more conventionally placed in a global ~/.gitignore_global rather than the project-level .gitignore, since it is an editor preference rather than a project artifact.

Overall Assessment: ⚠️ Needs Work