Skip to content

Conversation

@rodorgas
Copy link
Contributor

@rodorgas rodorgas commented Aug 18, 2025

User description

What problem is this solving?

This PR is part of the Ads Frictionless Integration on the Store Framework (RFC).

In order to allow components such as shelves, search result and autocomplete to make ad requests, we must wrap the store around the AdsProvider component. Then, the store will use the publisherId setting that is initialized earlier on the process, when the admin user join VTEX Ads (via vtex/ads-onboarding)

The requests uses the Ads SDK, that implements the client to request ads and merge product data with SKUs from ads.

An important change this feature will enable in comparison to the current solution is hydrating the ads on the client-side. By doing on the client-side, we're able to hit the CDN cache for product data, as opposed to returning ads with full product data that cannot be cached (since ads are picked by a real-time auction).

How to test it?

In this workspace, https://leal--biggy.myvtex.com/ search for the term "camisa".

Screenshots or example usage:

Sponsored products being displayed as usual when searching for product keywords of an active campaign:

image

Related to / Depends on

vtex-apps/product-summary#411

vtex-apps/search-result#711


PR Type

Enhancement


Description

  • Integrate VTEX Ads provider across store components

  • Add publisher ID configuration for ads functionality

  • Extract user session data utilities for reuse

  • Update manifest schema for ads configuration


Diagram Walkthrough

flowchart LR
  A["Store Configuration"] --> B["AdsProviderSF Component"]
  B --> C["StoreWrapper"]
  B --> D["SearchContext"]
  E["User Session Data"] --> B
  F["Publisher ID Setting"] --> B
  B --> G["Ads Display"]
Loading

File Walkthrough

Relevant files
Enhancement
7 files
StoreWrapper.js
Wrap store content with AdsProviderSF component                   
+6/-3     
global.d.ts
Add type declaration for product search query                       
+5/-0     
vtex.render-runtime.d.ts
Add publisherId to StoreSettings interface                             
+1/-0     
SearchContext.jsx
Wrap SearchQuery with AdsProviderSF component                       
+89/-81 
AdsProviderSF.tsx
Create new ads provider component with product matching   
+103/-0 
UserDataPixel.tsx
Extract user data utilities to separate hook                         
+5/-65   
getUserData.tsx
Create reusable user session data utilities                           
+74/-0   
Configuration changes
2 files
manifest.json
Replace sponsored products setting with publisherId           
+10/-25 
contentSchemas.json
Replace sponsored products with publisherId setting           
+5/-5     
Miscellaneous
1 files
SearchWrapper.tsx
Add debug text to SearchWrapper component                               
+1/-0     
Dependencies
1 files
package.json
Add VTEX ads React dependency                                                       
+1/-0     

@vtex-io-ci-cd
Copy link
Contributor

vtex-io-ci-cd bot commented Aug 18, 2025

Hi! I'm VTEX IO CI/CD Bot and I'll be helping you to publish your app! 🤖

Please select which version do you want to release:

  • Patch (backwards-compatible bug fixes)

  • Minor (backwards-compatible functionality)

  • Major (incompatible API changes)

And then you just need to merge your PR when you are ready! There is no need to create a release commit/tag.

  • No thanks, I would rather do it manually 😞

@vtex-io-docs-bot
Copy link

vtex-io-docs-bot bot commented Aug 18, 2025

Beep boop 🤖

I noticed you didn't make any changes at the docs/ folder

  • There's nothing new to document 🤔
  • I'll do it later 😞

In order to keep track, I'll create an issue if you decide now is not a good time

  • I just updated 🎉🎉

@github-actions
Copy link

github-actions bot commented Aug 18, 2025

Warnings
⚠️

👀 The size of this pull request seems relatively large (>420 modifications). Consider splitting it into smaller pull requests to help make reviews easier and faster.

Generated by 🚫 dangerJS against f41268b

@rodorgas rodorgas requested a review from brenonovelli August 18, 2025 18:10
@rodorgas rodorgas marked this pull request as ready for review August 29, 2025 17:28
@rodorgas rodorgas requested a review from a team as a code owner August 29, 2025 17:28
@rodorgas rodorgas requested review from RodrigoTadeuF, gabpaladino and vmourac-vtex and removed request for a team August 29, 2025 17:28
@qodo-code-review qodo-code-review bot changed the title feat: wrap the store around AdsProvider feat: wrap the store around AdsProvider Aug 29, 2025
@qodo-code-review
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

The GraphQL fetcher builds a fullText query using joined SKU IDs; ensure this matches the expected syntax for ProductSearchV3 and handles large offer arrays or empty results gracefully. Also, verify that returning children directly when publisherId is missing is valid for React nodes (could be undefined).

const fetcher = async (offers: Offer[]): Promise<Product[]> => {
  const skuIds = offers.map(o => o.skuId).join(';')
  const { data } = await client.query<{
    productSearch: { products: Product[] }
  }>({
    query: productSearchV3,
    variables: { fullText: `sku.id:${skuIds}` },
  })
  return data.productSearch.products
}

if (!publisherId) {
  return children
}

return (
  <AdsProvider
    identity={{
      accountName: account,
      publisherId,
      userId: userId ?? 'mock-user',
      sessionId,
      channel: 'site',
    }}
    hydrationStrategy={{ fetcher, matcher, key: 'sf' }}
  >
    {children}
  </AdsProvider>
Debug Artifact

Literal text 'SearchWrapper' was added inside the UI and will render to users. Confirm if this is intentional; likely a leftover debug string.

  SearchWrapper
  {React.cloneElement(children, props)}
</CustomContextElement>
State Defaults

Defaulting userId to 'mock-user' and sessionId to 'session-id' may pollute identity in production; consider safer fallbacks or gating by environment to avoid mixing audiences in Ads.

const [userId, setUserId] = useState<string | undefined>(undefined)
const [sessionId, setSessionId] = useState<string>('session-id')

useEffect(() => {
  getSessionPromiseFromWindow().then((data: SessionResponse) => {
    const profileFields = data?.response?.namespaces?.profile

    if (!profileFields) {
      return
    }

    setSessionId(data.response?.id ?? 'session-id')

    const userData = getUserData(profileFields)
    setUserId(userData.id as string | undefined)
  })
}, [])

const client = useApolloClient()
const fetcher = async (offers: Offer[]): Promise<Product[]> => {
  const skuIds = offers.map(o => o.skuId).join(';')
  const { data } = await client.query<{
    productSearch: { products: Product[] }
  }>({
    query: productSearchV3,
    variables: { fullText: `sku.id:${skuIds}` },
  })
  return data.productSearch.products
}

if (!publisherId) {
  return children
}

return (
  <AdsProvider
    identity={{
      accountName: account,
      publisherId,
      userId: userId ?? 'mock-user',
      sessionId,
      channel: 'site',
    }}
    hydrationStrategy={{ fetcher, matcher, key: 'sf' }}
  >
    {children}
  </AdsProvider>

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Aug 29, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Remove nested Ads provider

Avoid nesting multiple Ads providers; StoreWrapper already wraps the app with
AdsProviderSF. Remove the inner provider here to prevent duplicated contexts and
unexpected hydration behavior.

react/SearchContext.jsx [107-169]

 return (
-  <AdsProviderSF>
-    <SearchQuery
-      maxItemsPerPage={maxItemsPerPage}
-      query={queryValue}
-      map={mapValue}
-      orderBy={orderBy}
-      priceRange={priceRange}
-      hideUnavailableItems={hideUnavailableItems}
-      facetsBehavior={facetsBehavior}
-      categoryTreeBehavior={__unstableCategoryTreeBehavior}
-      pageQuery={pageQuery}
-      skusFilter={skusFilter}
-      simulationBehavior={simulationBehavior}
-      installmentCriteria={installmentCriteria}
-      excludedPaymentSystems={excludedPaymentSystems}
-      includedPaymentSystems={includedPaymentSystems}
-      operator={operator}
-      fuzzy={fuzzy}
-      searchState={state}
-      __unstableProductOriginVtex={__unstableProductOriginVtex}
-      sponsoredProductsBehavior={sponsoredProductsBehavior}
-    >
-      {(searchQuery, extraParams) => {
-        return React.cloneElement(children, {
-          ...
-        })
-      }}
-    </SearchQuery>
-  </AdsProviderSF>
+  <SearchQuery
+    maxItemsPerPage={maxItemsPerPage}
+    query={queryValue}
+    map={mapValue}
+    orderBy={orderBy}
+    priceRange={priceRange}
+    hideUnavailableItems={hideUnavailableItems}
+    facetsBehavior={facetsBehavior}
+    categoryTreeBehavior={__unstableCategoryTreeBehavior}
+    pageQuery={pageQuery}
+    skusFilter={skusFilter}
+    simulationBehavior={simulationBehavior}
+    installmentCriteria={installmentCriteria}
+    excludedPaymentSystems={excludedPaymentSystems}
+    includedPaymentSystems={includedPaymentSystems}
+    operator={operator}
+    fuzzy={fuzzy}
+    searchState={state}
+    __unstableProductOriginVtex={__unstableProductOriginVtex}
+    sponsoredProductsBehavior={sponsoredProductsBehavior}
+  >
+    {(searchQuery, extraParams) => {
+      return React.cloneElement(children, {
+        ...
+      })
+    }}
+  </SearchQuery>
 )

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that AdsProviderSF is being nested, as it's already provided by StoreWrapper. Removing the redundant provider prevents potential issues with context and unexpected behavior.

High
Possible issue
Guard against null session data

Protect against null/undefined session data to avoid runtime crashes. Use
optional chaining when accessing data.response and set sessionId only if
present. This makes the effect safe when the session promise resolves to null or
an unexpected shape.

react/components/AdsProviderSF.tsx [58-71]

 useEffect(() => {
-  getSessionPromiseFromWindow().then((data: SessionResponse) => {
-    const profileFields = data?.response?.namespaces?.profile
+  getSessionPromiseFromWindow().then((data: SessionResponse | null) => {
+    const response = data?.response
 
+    if (response?.id) {
+      setSessionId(response.id)
+    }
+
+    const profileFields = response?.namespaces?.profile
     if (!profileFields) {
       return
     }
-
-    setSessionId(data.response?.id ?? 'session-id')
 
     const userData = getUserData(profileFields)
     setUserId(userData.id as string | undefined)
   })
 }, [])
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential runtime error where data.response would be accessed on a null object, and provides a robust fix, preventing a crash.

Medium
Remove debug text output

Remove the stray "SearchWrapper" text node rendered to the page. It appears to
be debug output and will leak into production UI.

react/SearchWrapper.tsx [467-472]

 <LoadingContextProvider value={loadingValue}>
   <CustomContextElement>
-    SearchWrapper
     {React.cloneElement(children, props)}
   </CustomContextElement>
 </LoadingContextProvider>
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out a stray "SearchWrapper" text node that was likely left from debugging and would be rendered in the UI, improving the code's production-readiness.

Low
  • Update

@vmourac-vtex vmourac-vtex requested a review from Copilot October 8, 2025 17:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR integrates VTEX Ads functionality across the store by wrapping components with an AdsProvider. The main purpose is to enable ad serving capabilities throughout the store interface.

  • Adds AdsProviderSF component that wraps store content and search functionality
  • Refactors user session data utilities into reusable hooks for ads and pixel components
  • Replaces sponsored products configuration with publisherId setting for ads integration

Reviewed Changes

Copilot reviewed 9 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
manifest.json Updates version and replaces fetchSponsoredProductsOnSearch with publisherId setting
store/contentSchemas.json Adds publisherId configuration field for VTEX Ads
react/package.json Adds @vtex/ads-react dependency
react/hooks/getUserData.tsx Creates reusable utilities for extracting user session data
react/components/UserDataPixel.tsx Refactors to use extracted getUserData utilities
react/components/AdsProviderSF.tsx Creates new ads provider component with product matching logic
react/StoreWrapper.js Wraps store content with AdsProviderSF component
react/SearchWrapper.tsx Adds debug text to SearchWrapper
react/SearchContext.jsx Wraps SearchQuery with AdsProviderSF component

Copy link
Contributor

@vmourac-vtex vmourac-vtex left a comment

Choose a reason for hiding this comment

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

  • Adicionar CHANGELOG.md
  • Adicionar uma descrição no PR de acordo com o template do repositório
    • Precisamos de workspaces para validar os comportamentos antes e depois das mudanças

Comment on lines 15 to 22
account?: {
accountName?: {
value: string
}
id?: {
value: string
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@rodorgas qual a necessidade de adicionar novos atributos no SessionResponse. Adicionando esses novos atributos o setSessionId vai atualizar a session com esses novos atributos, sendo enviados nos próximos requests. Isso pode impactar o cache hit rate da plataforma, dado que a session é um dos itens da chave de cache.

Qual a necessidade de alterar a session? Tem alguma outra forma disso ser feito?

Copy link
Contributor

@MatheusLealv MatheusLealv Oct 20, 2025

Choose a reason for hiding this comment

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

@vmourac-vtex Bom ponto, os atributos novos não têm a necessidade de serem adicionados, acabei de removê-los.

EDIT: Precisei re-adicionar o campo "id" pois precisamos para obter o session_id.

@vsseixaso
Copy link
Contributor

Oi @MatheusLealv. Tem alguma forma de testar essa feature? Vi que há outros PRs associados, pode ser um workspace com tudo linkado. Aí pode adicionar na descrição do PR as instruções de como testar.

@MatheusLealv
Copy link
Contributor

Oi @vsseixaso Instalei/Linkei todos os apps necessários no account biggy e workspace leal. Criamos uma campanha de ads para alguns produtos, você consegue encontrar buscando por "camisa".

Você pode conferir aqui: https://leal--biggy.myvtex.com/ e pesquisar pelo termo "camisa"

@vsseixaso
Copy link
Contributor

@MatheusLealv, não deveria estar aparecendo a tag "Patrocinado"?

image

vsseixaso added a commit to vtex-apps/product-summary that referenced this pull request Nov 5, 2025
#### What problem is this solving?

<!--- What is the motivation and context for this change? -->
We're changing the encoded event parameters from `adId` to
`eventParameters`, since it's not the ad ID. The data attribute is still
using the data-van-aid since that's what the Activity Flow use to track
events (to be changed in future).

This is backwards compatible as it keeps using the `adId` as fallback to
the `eventParameters` parameter.

It also removes many data properties that are not being used anymore,
since the `data-van-aid` is the only one considered by Activity Flow
currentyl

#### How to test it?

<!--- Don't forget to add a link to a Workspace where this branch is
linked -->
In this workspace, https://leal--biggy.myvtex.com/ search for the term
"camisa".



#### Screenshots or example usage:

<img width="1624" height="1060" alt="image"
src="https://github.com/user-attachments/assets/3593a558-1ac6-4cb8-859c-93fb6fea0277"
/>

<!--- Add some images or gifs to showcase changes in behaviour or
layout. Example: before and after images -->


#### Related to / Depends on

vtex-apps/store#605

vtex-apps/search-result#711

---------

Co-authored-by: Matheus Leal <[email protected]>
Co-authored-by: Vinícius Seixas <[email protected]>
@MatheusLealv
Copy link
Contributor

MatheusLealv commented Nov 10, 2025

Rodei um rebase com a master para resolver os conflitos 🙂

@vmourac-vtex
Copy link
Contributor

@copilot can you review latest changes since last approval?

Copy link
Contributor

Copilot AI commented Dec 3, 2025

@vmourac-vtex I've opened a new pull request, #630, to work on those changes. Once the pull request is ready, I'll request review from you.

@MatheusLealv MatheusLealv merged commit 7397573 into master Dec 4, 2025
10 checks passed
@MatheusLealv MatheusLealv deleted the feat/ads-wrapper branch December 4, 2025 13:15
@vtex-io-ci-cd
Copy link
Contributor

vtex-io-ci-cd bot commented Dec 4, 2025

Your PR has been merged! App is being published. 🚀
Version 2.144.0 → 2.145.0

After the publishing process has been completed (check #vtex-io-releases) and doing A/B tests with the new version, you can deploy your release by running:

vtex deploy [email protected]

After that your app will be updated on all accounts.

For more information on the deployment process check the docs. 📖

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.

6 participants