Skip to content

feat(publication): GS1 Digital Link qualifiers and GTIN history - #684

Open
midego1 wants to merge 13 commits into
unopim:3.xfrom
midego1:feat/publication-gs1-qualifiers
Open

midego1 wants to merge 13 commits into
unopim:3.xfrom
midego1:feat/publication-gs1-qualifiers

Conversation

@midego1

@midego1 midego1 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Stacked on #682 (→ #681 → #679 + #680)

Only the last commit (feat(publication): GS1 Digital Link qualifiers and GTIN history) is new here.

Problem

Two gaps on the GS1 side of the same lifetime-reachability track.

  1. A scanned GS1 link can carry the lot (AI 10) or serial (AI 21) that ties a physical unit to the release it was placed on the market under, but /01/{gtin} is the only shape the resolver accepts, so that information is dropped and every scan lands on the live state.
  2. SyncPublicationGtin overwrites publications.gtin on every publish. A corrected GTIN therefore silently breaks every /01/{old-gtin} link already printed: findByGtin() finds nothing and the scan 404s.

Change

Qualifier routes: /01/{gtin}/10/{lot}, /01/{gtin}/21/{serial} and /01/{gtin}/10/{lot}/21/{serial} (GS1 order). The router only bounds the segment; the GS1 82-character set and 1–20 length are enforced in the controller via Gs1DigitalLink::isWellFormedQualifier(). A malformed qualifier is a bad link and 404s; it never falls through to a guess.

LotReleaseResolver contract (resolve(Publication, ?lot, ?serial): ?PublicationRelease). Which release a lot shipped under is not the PIM's knowledge, so the engine defines the question and binds NullLotReleaseResolver by default (bindIf, so a consumer with batch or ERP data can rebind). With the default, a qualified scan resolves exactly like an unqualified one. When a resolver names a release, the scan 302s to that release's strict per-locale URL (locale negotiated once from Accept-Language among the locales in that release). resolveByGtin() now delegates to the qualified path with null qualifiers, so both share one implementation.

publication_gtins: append-only history of every GTIN a publication has published under, written by SyncPublicationGtin (insertOrIgnore, unique per publication + GTIN) and backfilled from current values in the migration. PublicationResolver::findByGtin() first looks up the current GTIN as before; if nothing carries it today, it falls back to publications that carried it earlier, applying the same designated-passport-channel rule. Rows refuse update and delete.

No lang changes. PHPStan (level 2) clean.

Tests (Gs1QualifierTest, ProductPassport suite because only the real builder emits identifier.gtin)

  • the null resolver is the default binding, and lot, serial and lot+serial scans all redirect to the live passport
  • with a resolver bound in the test, lot L1 302s to /p/{uuid}/r/1/{locale} with Vary: Accept-Language; an unknown lot lands on the live passport
  • a 21-character lot and a | in a serial 404
  • after correcting a product's GTIN and republishing, publications.gtin is the new value, the history holds both, and both /01/{old} and /01/{new} resolve to the same passport

Existing GS1, carrier-target and check-digit tests pass unchanged.

Related

Closes the track described in #683, after #677, #678, #679, #680, #681 and #682. Remaining follow-ups noted there: an ETag on the live /carrier.svg, and JSON-LD for historical states once a version-level identity is stamped into the payload.

…moment

Versions are numbered per locale, so "version 3" names a different
state in every language. Nothing identifies one moment across locales,
which a printed carrier needs: the passport state a product was placed
on the market with is one moment, not one per language.

Add `publication_releases`: one row per minted version, with a
`sequence` that is monotonic per publication, minted inside the same
lock as the version number. `publication_versions.release_id` points at
it. Existing versions are backfilled, one release each in publish order.

`PublicationRelease::versionsAsOf()` resolves the state as of a release:
for every locale, the most recent version minted at or before it. That
is the read a per-release public route needs and nothing else; this
change alters no public behaviour.

Releases are immutable like versions (update/delete throw), and
`release_id` is sealed on the version.
… out of shared caches

`Publisher::redactAll()` redacted only the `is_current` versions before
flipping the publication to Redacted. Every superseded version kept its
sealed payload readable in the database, so a GDPR Art. 17 erasure held
only for as long as nothing ever read history. Now every not-yet-redacted
version of the publication is redacted; versions already redacted on
their own are left untouched.

The public tombstone changes with it. A redacted passport answers
`410 Gone` (the state is irreversible), while a withdrawn one stays `200`
because it can be reinstated. Both tombstones are now `private, no-store`
and `noindex`: the package has no cache purge hook, so a shared cache
must never hold a tombstone that a reinstatement would have to displace,
and the tombstone must not be indexed regardless of the channel's
`indexable` setting.

Tests: redactAll nulls superseded payloads and stamps the reason on
them; a version redacted individually earlier keeps its own reason; the
redacted route is 410 + no-store + noindex; the withdrawn route is 200 +
no-store + noindex.
Adds `GET /{prefix}/{uuid}/r/{sequence}/{locale}`: the state of the
passport as of one release, for one locale, resolved through
`PublicationRelease::versionsAsOf()`. This is the URL a printed carrier
can be bound to: it names one moment across every language and keeps
resolving to exactly that state after the passport moves on.

Semantics:
- strict locale, no Accept-Language negotiation; a locale that had no
  version yet at that release is 404 there
- 200 with a banner naming the release and whether it is still current,
  linking to the live page; the locale switcher stays inside the release
- never indexed (`noindex, noarchive, nofollow`, a `<link rel=canonical>`
  and a `Link` header pointing at the live page); no JSON-LD negotiation,
  since a historical payload's stamped identity is the publication URL
- a version redacted individually renders the tombstone with 410 and
  `no-store`, even while the publication itself stays Published; this
  also closes the same gap on the live route
- the ETag now covers release, currency and redaction state, because
  the banner and the tombstone change the HTML without changing the
  checksum; TEMPLATE_VERSION bumped
- release pages are not counted as views

`show()` and `showRelease()` share one `render()`. The four new strings
live in the Publication package and are present in all 33 locales, in
English where no translation exists yet, so the translation audit passes.
@midego1

midego1 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Context, motivation and suggested review order for this and the related PRs: #683

The public `/carrier.svg` encodes the live URL, and nothing records what
was printed. A manual holds a QR code for years; when the passport moves
on there is no durable answer to "which code is in that manual and what
did it point at".

Add `publication_carrier_issuances`: publication, release, the exact
string encoded, format, issued_at, issued_by. Immutable, like versions
and releases. `CarrierIssuer::issue()` mints the row and derives the
target from the per-channel passport base URL, the same base every
other printed link uses.

The target is a new public entry route, `/{prefix}/{uuid}/r/{sequence}`:
it negotiates the locale once from Accept-Language among the locales
that exist in that release and 302s to the strict per-locale URL, the
same way the live carrier's bare `/{uuid}` works. Negotiation is moved
into `PublicationResolver::pickVersion()` so the live and release paths
share it.

Admin: "Issue QR code" per release on the versions page (publish
rights), returning the SVG as a download and recording the issuance.
The page also lists releases (with their locales) and every issuance
with its encoded link. QR rendering is shared via `CarrierSvg` so the
public live carrier and an issued release carrier print identically for
the same target.
A scanned GS1 link can carry the lot (AI 10) and serial (AI 21) that tie
a unit to the release it was placed on the market under. Add
`/01/{gtin}/10/{lot}`, `/01/{gtin}/21/{serial}` and the combination,
validated against the GS1 82-character set and 1-20 length in the
controller (a malformed qualifier is a bad link: 404, never a guess).

Which release a lot shipped under is not the PIM's knowledge, so the
mapping sits behind a `LotReleaseResolver` contract. The default binding
answers null and a qualified scan then resolves exactly like an
unqualified one, to the live passport. A consumer with batch or ERP
data rebinds it; when it names a release, the scan 302s to that
release's strict per-locale URL, with the locale negotiated once.

`publications.gtin` was overwritten on every publish, so a corrected
GTIN silently broke every `/01/{old}` link already printed. Add
`publication_gtins`, an append-only history written by
`SyncPublicationGtin` and backfilled from current values;
`PublicationResolver::findByGtin()` falls back to it when no publication
carries the scanned GTIN today, with the same designated-channel rule.
@midego1
midego1 force-pushed the feat/publication-gs1-qualifiers branch from cadb84c to 31fbd24 Compare September 4, 2026 09:18
sandeepp-webkul pushed a commit to sandeepp-webkul/unopim that referenced this pull request Sep 21, 2026
* fix: validate empty files during product import instead of throwing code error (unopim#696)

* fix: hide broken logo image in profile dropdown when remote fetch fails (unopim#700)

* fix: disable @ attribute suggestions in profile image AI generation (unopim#701)

* fix: set HTMLPurifier cache path to storage directory to prevent vendor write error during import (unopim#350)

* fix: hide webhook logs tab and enforce ACL for unauthorized roles (unopim#545)

* fix: EditImage tool now fetches product image by SKU instead of requiring upload (unopim#683)

* fix: add XLSX export support to AI Agent ExportProducts tool (unopim#684)

* fix: add SKU validation to AI Agent bulk import to reject special characters (unopim#689)

* style: apply pint formatting to AiAgent lang file

* fix: address copilot review on PR unopim#353

@navneetkumar-pim-webkul navneetkumar-pim-webkul left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for this — the qualifier routes and the GTIN history close a real gap. Reviewed the new commit (31fbd24a) only; #679 → #680 → #681 → #682 are still open underneath and should land first, in order.

Must fix

  • Scale: the migration backfill loads all publications rows with an unbounded get() and inserts one row per iteration. On large catalogs this should stream (chunkById + batched insert, or a single insertUsing).
  • Inline comments: the project keeps method/test bodies comment-free (see e.g. Product/src/Repositories/ProductRepository.php). The rationale in the new // comments belongs in the method PHPDoc or the commit message. Flagged inline.

Should fix

  • Deterministic old-GTIN fallback: when two publications have both carried a GTIN, findByGtinWhere() can pick either one (no ORDER BY with a passport channel; a channel_id tie without one), and MySQL and PostgreSQL may disagree. Prefer the most recent carrier (recorded_at desc, id desc), plus a test.
  • No way to retire a wrong GTIN: history rows can't be updated or deleted, so a mistyped GTIN (possibly another brand's) resolves to this passport forever. A revoked_at flag that the fallback skips keeps the table append-only while allowing a correction.

Minor

  • A lot/serial containing / is valid in the GS1 82-char set but 404s: the router decodes %2F before matching [^\/].
  • SyncPublicationGtin runs insertOrIgnore on every publish; it can skip when the GTIN is unchanged.
  • A resolver returning a release from another publication silently falls back to live — worth a Log::warning so a buggy resolver is visible.

Looks good: bindIf + NullLotReleaseResolver as the extension point, 404 for malformed qualifiers instead of guessing, private, no-store + Vary: Accept-Language on the redirect, Concord proxy/contract + #[Fillable]/#[Table], explicit prefix-safe index names, portable insertOrIgnore, and good test coverage. CI is green on MySQL and PostgreSQL.

$rows = DB::table('publications')
->whereNotNull('gtin')
->where('gtin', '!=', '')
->get(['id', 'gtin', 'last_published_at', 'created_at']);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This loads every published row into memory, then inserts one row per loop iteration. On a large catalog, please stream it, e.g.:

DB::table('publications')
    ->whereNotNull('gtin')->where('gtin', '!=', '')
    ->select(['id', 'gtin', 'last_published_at', 'created_at'])
    ->chunkById(1000, function ($rows): void {
        DB::table('publication_gtins')->insert($rows->map(fn ($row): array => [/* ... */])->all());
    });

(or a single insertUsing() from the select).


$table->timestamps();

// Explicit names: auto names include the prefix and overrun MySQL's 64-char identifier limit on prefixed installs.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: the project keeps method bodies comment-free. Could this rationale (and the backfill note on L29) move to the commit message or a docblock?

}

// A GTIN the publication carried earlier: carriers printed under it must keep resolving after a correction.
$previouslyOwned = PublicationGtinProxy::modelClass()::query()->where('gtin', $gtin)->pluck('publication_id');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If more than one publication has carried this GTIN (e.g. it moved A → B, then B was corrected away too), findByGtinWhere() ends in ->first() without an ORDER BY when a passport channel is configured, or with a channel_id tie otherwise. The result is then engine-dependent. Suggest resolving to the most recent carrier (order by publication_gtins.recorded_at desc, id desc), with a test for this case. The comment on L55 could also move into the PHPDoc.

throw new ImmutableVersionException('GTIN history row '.$row->id.' is immutable.');
});

static::deleting(function (self $row): void {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Append-only makes sense, but there's currently no way to retire a GTIN that was published by mistake (e.g. a typo that belongs to another brand): it will resolve to this passport forever. A nullable revoked_at that the fallback lookup skips (plus a small command or admin action to set it) would keep the history immutable while allowing a correction.

$model::query()->whereKey($publication->id)->update(['gtin' => $gtin]);

// Append-only history: a `/01/{gtin}` link printed under an earlier GTIN must keep resolving after a correction.
PublicationGtinProxy::modelClass()::query()->insertOrIgnore([

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor: this runs a write on every publish even when the GTIN hasn't changed. It could be skipped when $publication->gtin === $gtin. Also, the comment on L49 would read better in the class docblock.

// bounds the segment here; the 82-character set and 1-20 length are enforced in the controller.
foreach (['/01/{gtin}/10/{lot}' => 'gs1.lot', '/01/{gtin}/21/{serial}' => 'gs1.serial', '/01/{gtin}/10/{lot}/21/{serial}' => 'gs1.lot.serial'] as $uri => $name) {
Route::get($uri, [PublicationController::class, 'resolveByGtinQualified'])
->where(['gtin' => Gs1DigitalLink::GTIN_PATTERN, 'lot' => '[^\/]{1,80}', 'serial' => '[^\/]{1,80}'])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

/ is part of the GS1 82-character set and isWellFormedQualifier() accepts it, but Laravel matches routes against the decoded path, so a serial like A%2F1 never reaches the controller and 404s. Fine to accept that as a limitation, but maybe worth a note/test either way. The comments on L157–158 and L42 could also move to a docblock.

->assertRedirect('/p/'.$publication->uuid.'/r/1/'.$versions[0]->locale->code)
->assertHeader('Vary', 'Accept-Language');

// Unknown lot: the resolver answers null, so the scan lands on the live passport rather than a dead end.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: the test name or a separate it() could carry this instead of an inline comment.

The backfill loaded every publication with a GTIN through one unbounded
get() and inserted one history row per iteration, which does not scale
on large catalogs. Read the publications with chunkById and insert one
batch per chunk, so memory stays flat and the number of statements drops
by the chunk size.

The explanatory comments in the migration body move into the PHPDoc of
up().
The project keeps method and test bodies comment-free. Move the rationale
added with the GS1 qualifiers into the PHPDoc of the method or class it
explains (service provider register() and registerPublicRoutes(),
SyncPublicationGtin, PublicationResolver::findByGtin()). In the qualifier
test, the unknown-lot case becomes its own it() so its name carries the
intent.
When more than one publication has carried the same GTIN (it moved from A
to B, then was corrected away from B too), the history fallback in
findByGtin() ended in first() without an ORDER BY when a passport channel
was configured, or in a channel_id tie otherwise, so the result depended
on the database engine.

Order the history by recorded_at desc, id desc and pick the first
candidate that passes the channel rule in PHP. The designated passport
channel still restricts the candidates, and the warning for the
undesignated case is kept. A data-provider test covers a later carrier on
the higher channel, on the lower channel, and an identical instant.
The GTIN history is append-only, so a GTIN published by mistake (a typo
that belongs to another brand, say) kept resolving to the passport that
once carried it. Add a nullable revoked_at to publication_gtins that the
history fallback skips, keeping the table append-only: PublicationGtin
now permits exactly one change, setting revoked_at on a row that is not
revoked yet, via revoke().

`unopim:publication:revoke-gtin {gtin} {--publication=}` retires the
active history entries for a GTIN. It refuses a malformed GTIN and leaves
alone a publication that still carries the GTIN today, since correcting
that is a republish. Revoking never touches publications.gtin, so a
publication that publishes the GTIN again resolves through its current
value as before.

The column joins the create migration of the same pull request, which has
not shipped yet.
SyncPublicationGtin ran an update and an insertOrIgnore on every publish.
Both only matter when the GTIN differs from the one the publication
already carries, so skip them when it does not. The canonical alias
handling still runs.
…'s release

A LotReleaseResolver that names a release of a different publication is a
bug in the consumer's resolver. The controller already refuses to follow
it and falls back to live, but did so silently. Log a warning with the
resolver class, the publication and the release so the bug is visible.

Also document in the PHPDoc that a lot or serial containing `/` does not
resolve: it is valid in the GS1 82-character set, but the router matches
the decoded path, so a `%2F` splits the segment and the link 404s.
…nd encoded slashes

Pin the three small behaviours of the qualifier routes and the GTIN sync:
a publish with an unchanged GTIN performs no history or GTIN write, a
resolver returning another publication's release logs a warning and lands
on live, and a serial with an encoded slash 404s (a documented limitation
of the route grammar).
@midego1

midego1 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review, much appreciated. All points are addressed on feat/publication-gs1-qualifiers (new commits only; the stack below is unchanged and still merges cleanly onto 3.x).

Must fix

  • Backfill scale – b1cb38a0: the migration now streams the publications with chunkById(1000) and inserts one batch per chunk. Checked with 2,500 rows on MySQL and PostgreSQL: three insert statements, row counts match.
  • Inline comments – b1cb38a0 (migration) and 70872077 (the rest): the rationale moved into the PHPDoc of the method or class it explains; the unknown-lot case in the test is now its own it().

Should fix

  • Deterministic old-GTIN fallback – e27fd6ee: the history is ordered by recorded_at desc, id desc and the most recent carrier that passes the channel rule wins; the choice is made in PHP so MySQL and PostgreSQL agree. A data-provider test covers a later carrier on the higher channel, on the lower channel, and an identical instant.
  • Retiring a wrong GTIN – 402cc54d: nullable revoked_at on publication_gtins (added to the create migration of this PR), skipped by the fallback. PublicationGtin stays append-only except for setting revoked_at once. New command unopim:publication:revoke-gtin {gtin} {--publication=}; it rejects a malformed GTIN and leaves alone a publication that still carries the GTIN today. Tests included.

Minor

  • / in lot/serial – 58030176 documents it in the PHPDoc and 80d7862e adds a test: a %2F is decoded before route matching, so it 404s. I kept it as a documented limitation rather than changing the route grammar.
  • Skip unchanged GTIN – 4aee3dea: SyncPublicationGtin skips the update and the history insert when the GTIN is unchanged (test in 80d7862e).
  • Foreign release – 58030176: a Log::warning (resolver class, publication, release) when a resolver returns a release of another publication and the scan falls back to live (test in 80d7862e).

Publication and ProductPassport suites pass on MySQL 8.0 and PostgreSQL 16, both on the branch and on a trial merge with current 3.x. Pint and PHPStan are clean.

Could you take another look when you have a moment?

1 similar comment
@midego1

midego1 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review, much appreciated. All points are addressed on feat/publication-gs1-qualifiers (new commits only; the stack below is unchanged and still merges cleanly onto 3.x).

Must fix

  • Backfill scale – b1cb38a0: the migration now streams the publications with chunkById(1000) and inserts one batch per chunk. Checked with 2,500 rows on MySQL and PostgreSQL: three insert statements, row counts match.
  • Inline comments – b1cb38a0 (migration) and 70872077 (the rest): the rationale moved into the PHPDoc of the method or class it explains; the unknown-lot case in the test is now its own it().

Should fix

  • Deterministic old-GTIN fallback – e27fd6ee: the history is ordered by recorded_at desc, id desc and the most recent carrier that passes the channel rule wins; the choice is made in PHP so MySQL and PostgreSQL agree. A data-provider test covers a later carrier on the higher channel, on the lower channel, and an identical instant.
  • Retiring a wrong GTIN – 402cc54d: nullable revoked_at on publication_gtins (added to the create migration of this PR), skipped by the fallback. PublicationGtin stays append-only except for setting revoked_at once. New command unopim:publication:revoke-gtin {gtin} {--publication=}; it rejects a malformed GTIN and leaves alone a publication that still carries the GTIN today. Tests included.

Minor

  • / in lot/serial – 58030176 documents it in the PHPDoc and 80d7862e adds a test: a %2F is decoded before route matching, so it 404s. I kept it as a documented limitation rather than changing the route grammar.
  • Skip unchanged GTIN – 4aee3dea: SyncPublicationGtin skips the update and the history insert when the GTIN is unchanged (test in 80d7862e).
  • Foreign release – 58030176: a Log::warning (resolver class, publication, release) when a resolver returns a release of another publication and the scan falls back to live (test in 80d7862e).

Publication and ProductPassport suites pass on MySQL 8.0 and PostgreSQL 16, both on the branch and on a trial merge with current 3.x. Pint and PHPStan are clean.

Could you take another look when you have a moment?

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.

2 participants