Skip to content

Converted UPC to gtin13 in product schema#916

Merged
tblivet merged 6 commits intoPrestaShop:developfrom
mgielecinski:fix-upc-code-in-gtin13
Jan 29, 2026
Merged

Converted UPC to gtin13 in product schema#916
tblivet merged 6 commits intoPrestaShop:developfrom
mgielecinski:fix-upc-code-in-gtin13

Conversation

@mgielecinski
Copy link
Contributor

Questions Answers
Description? Converted UPC to gtin13 by adding leading 0
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket?
Sponsor company
How to test?

Copy link
Contributor

@tblivet tblivet left a comment

Choose a reason for hiding this comment

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

Hi @mgielecinski, I left a comment here, @kpodemski or @Hlavtox do you have an opinion on this?

"mpn": "{if $product.mpn}{$product.mpn}{elseif $product.reference}{$product.reference}{else}{$product.id}{/if}"
{if $product.ean13},"gtin13": "{$product.ean13}"
{elseif $product.upc},"gtin13": "{$product.upc}"
{elseif $product.upc},"gtin13": "0{$product.upc}"
Copy link
Contributor

@tblivet tblivet Jan 29, 2026

Choose a reason for hiding this comment

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

Hi @mgielecinski, I’m not an expert on product codes, but it seems we can use gtin12 instead. This appears to be valid according to Schema.org: https://schema.org/Product

Suggested change
{elseif $product.upc},"gtin13": "0{$product.upc}"
{elseif $product.upc},"gtin12": "{$product.upc}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @tblivet
I think we can use gtin12 for the UPC code, but I was suggesting this line

@Hlavtox
Copy link
Contributor

Hlavtox commented Jan 29, 2026

I agree with @tblivet, gtin12 is a better solution IMHO, adding a zero smells :D

@Hlavtox
Copy link
Contributor

Hlavtox commented Jan 29, 2026

@mgielecinski @tblivet Is it allowed to use both gtin13 and gtin12 together? We could change the elseif into separate condition.

@mgielecinski
Copy link
Contributor Author

mgielecinski commented Jan 29, 2026

Including both is generally unnecessary if they represent the same product

But
https://support.google.com/merchants/answer/6324461?hl=en-GB

And i think gtin13 need to be change to gtin??
obraz

@tblivet
Copy link
Contributor

tblivet commented Jan 29, 2026

@mgielecinski @Hlavtox, I think we should go with something like this?

{if $product.ean13},"gtin13": "{$product.ean13}"{/if}
{if $product.upc},"gtin12": "{$product.upc}"{/if}

Comment on lines +81 to +83
"sku": "{if $product.reference}{$product.reference}{else}{$product.id}{/if}",
"mpn": "{if $product.mpn}{$product.mpn}{elseif $product.reference}{$product.reference}{else}{$product.id}{/if}",
{if $product.ean13}"gtin13": "{$product.ean13}",{elseif $product.upc}"gtin13": "0{$product.upc}",{/if}
{if $product.ean13}"gtin13": "{$product.ean13}",{elseif $product.upc}"gtin12": "{$product.upc}",{/if}
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I hadn’t noticed that the code was in two different places. I believe sku, mpn, ean13, and upc inside the Offers type can be removed, as they are duplicates.

{elseif $product.upc},"gtin13": "{$product.upc}"
{if $product.ean13},"gtin13": "{$product.ean13}"{/if}
{if $product.upc},"gtin12": "{$product.upc}"{/if}
{/if}
Copy link
Contributor

Choose a reason for hiding this comment

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

@mgielecinski Thanks for the changes, but you forgot to remove a {/if}.

Suggested change
{/if}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohhh.. 2 Fast 2 Furious 😆

Copy link
Contributor

@tblivet tblivet left a comment

Choose a reason for hiding this comment

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

Perfect thank you 👍

@mgielecinski
Copy link
Contributor Author

I made changes but I still think that gtin13 should be changed to gtin because in the ean13 field we can enter a 14-digit code

Copy link
Contributor

@tblivet tblivet left a comment

Choose a reason for hiding this comment

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

Hmm, you’re right, let’s go with gtin for ean13. Sorry for the back and forth 😕

Copy link
Contributor

@tblivet tblivet left a comment

Choose a reason for hiding this comment

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

Thank you again 🙂

@ps-jarvis ps-jarvis moved this from Ready for review to To be tested in PR Dashboard Jan 29, 2026
@AureRita AureRita self-assigned this Jan 29, 2026
Copy link

@AureRita AureRita left a comment

Choose a reason for hiding this comment

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

Hi @mgielecinski

Thank you for your PR, I tested it and it seems to works as you can see :

Image

Because the PR seems to works as expected, It's QA ✔️

Thank you

@tblivet tblivet merged commit 190debd into PrestaShop:develop Jan 29, 2026
6 checks passed
@github-project-automation github-project-automation bot moved this from To be tested to Merged in PR Dashboard Jan 29, 2026
@ps-jarvis
Copy link

PR merged, well done!

Message to @PrestaShop/committers: do not forget to milestone it before the merge.

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

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants