feat(orocommerce): add the OroCommerce piece - #3
Open
mbessolov wants to merge 101 commits into
Open
Conversation
- added API client - added auth - added new order trigger
…lize/deserialize actions
Four actions take a password as an ordinary step input. There is no step-level secret property to move them to — SecretTextProperty exists only as a PieceAuthProperty and is deliberately absent from the InputProperty union that createAction's props must satisfy — so the descriptions now say plainly that the value is stored in clear text and visible to anyone who can open the flow. Two behaviours that were only visible by reading the code are now stated where a flow builder will see them: create-customer-user defaults enabled and confirmed to true, so it produces an active account without an email confirmation round trip; update-user can change username, email, password and auth status in one call, which is enough to lock an existing user out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…umber() Both order and invoice creation turned user input into numbers with a bare Number(). Number(undefined) is NaN and JSON.stringify serialises NaN as null, so a missing or non-numeric quantity reached Oro as null — no error, no failed run, just a wrong record. The invoice action had no validation at all and passed description through untyped; the order action filtered partially in toOrderLineItemRow and still coerced with Number() afterwards, and fell back to an empty productSku when the required field was absent. Both now go through lineItemUtils, which reports the row number and the field label a flow builder actually sees: Line Items row 3, "Quantity" must be a number, got "abc". Blank and whitespace-only strings count as missing rather than as 0, booleans are rejected rather than becoming 1, a wrong type is reported as a wrong type rather than as a missing value, and text fields are trimmed so an id pasted with whitespace still resolves. Optional fields holding only whitespace are treated as absent, so a stray space does not fail the run. Invoice creation additionally reconciles Total Amount against the sum of the row totals. The comparison is in minor units so 0.1 + 0.2 against 0.3 does not fail, with one cent of tolerance for rounding. The prop description had claimed the total "should equal the sum of all line item row totals" without anything enforcing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
update-customer, update-user and update-customer-user assembled attributes and
relationships from whatever the user filled in. Filling in nothing still sent
{data:{type,id,attributes:{},relationships:{}}}; sanitizeJsonApiBody stripped the
empty containers, Oro changed nothing, and the step reported success. A flow
built on that reads as working while doing nothing at all.
The three actions now refuse the call when both maps come back empty, naming
themselves so the message is useful in a flow with several update steps.
The accompanying tests also cover the invoice line-item validation added in the
previous commit, because both are failures that must surface before any request
reaches Oro — the tests would hit the network if either guard let them through.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
invoicePdfContent was a LongText holding a base64 PDF. A multi-megabyte string in a text prop travels in the request body, sits in worker memory, and lands in the run log; it also collides with AP_MAX_FILE_SIZE_MB and AP_MAX_FLOW_RUN_LOG_SIZE_MB. It is now a File prop, so the runtime handles the transfer and the action reads .base64 at the point it builds the JSON:API resource. The filename default moves to the uploaded file's own name, which also removes the duplicated 'invoice.pdf' literal that was set in two places. refCustomerId and refCustomerUserId were Number props whose descriptions call them arbitrary external identifiers. External ids are routinely strings, and leading zeros do not survive a Number — both are now ShortText. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Regenerating the piece's translation files was a manual step, so it stopped happening. The locale files carried 175 keys against the 255 the piece actually exposes: 46 belonged to props that no longer exist, and 126 current strings were missing entirely. The translations themselves are good and are left untouched. tools/check-i18n.mjs closes the loop. It imports the built piece, walks the same 19 metadata paths as pieceTranslation.pathsToValuesToTranslate and truncates keys at the same 512 characters, so it derives exactly the key set the official generator does — verified by diffing its output against "cli pieces generate-translation-file orocommerce", which matches key for key and value for value. It fails on keys missing from or stale in translation.json, on any locale file whose key set differs from it, and on empty values, and it names the offending keys rather than only counting them. A value identical to the English source is a warning, since GET and Website are the same word in several of these languages; --strict-untranslated promotes it. i18n:write reconciles without the monorepo CLI: stale keys go, missing keys are seeded with the English text, existing translations are kept. --bundle= and --i18n-dir= override the paths for use outside this layout. The check runs in CI alongside the piece's tests, which were not running at all before — ci.yml's test step is a hardcoded filter list that did not name this piece, so both jsonapi-roundtrip and the new line-item tests were local-only. Two warnings are expected and left standing: pl.json and uk.json are never loaded by the runtime, because LocalesEnum has no Polish or Ukrainian. They stay in sync so they are ready if those locales are ever added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
minimumSupportedRelease claimed 0.36.1. The package declares workspace dependencies on @activepieces/core-utils and @activepieces/core-piece-types and follows the bundle model, none of which exist before 0.86.0 — an 0.36.1 server cannot load this piece at all. The floor is now the real one. bun.lock had no entry for this workspace, so a clean checkout resolved it by accident rather than by the lockfile. The logo URL keeps its host but escapes the parentheses in O(logo).svg, which several markdown and URL parsers treat as delimiters. Renaming the asset needs someone with write access to static.oroinc.com; the encoded form is verified to serve the same image. package.json gains description, keywords, license, homepage and repository. The piece sits under packages/pieces/community, so the repository's root MIT terms apply; no separate LICENSE file is added, since no other piece ships one and the only thing that would need it is standalone npm publishing, which is not settled. The README leads with what a user needs — connection setup, the action and trigger list, where to file issues — and keeps the existing engineering notes below for contributors, with new sections on the i18n gate and on why the password props cannot currently be secrets. Its claim that the repository "bans code comments" is softened to match the seventy-odd section markers actually in the source, and the claim that CI does not run these tests is now false and removed. Also drops plan-oroCommerceActions.prompt.md_, a leftover planning artifact, and restores .nvmrc to its upstream byte-for-byte form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A review of the five preceding commits turned up a handful of things that
pass the suite but would misbehave in front of a user. This fixes the ones
that do not need a product decision.
Line items:
- parseNumber still handed the string to Number(), so '0x10' became a
quantity of 16 and '0b101' became 5. It now requires a decimal shape
first. '+5', '-5', '.5', '5.' and '1e3' still parse.
- readRows reported "add at least one row" for anything that was not an
array, so a lineItems field mapped to a string told the user to add a
row they had already added. Not-a-list and empty-list are now separate
messages; the empty cases keep the old wording.
- toMinorUnits used Math.round(value * 100), which is not decimal-exact:
Math.round(1.005 * 100) is 100, not 101, so a total that was correct to
the cent was rejected whenever toleranceMinorUnits was 0. create-invoice
passes 1 and absorbed it, but 0 is the default and the next caller would
not have. Checked across a matrix of amounts: the only behaviour that
changes is at tolerance 0, and only half-cent pairings that were always
correct now pass.
Casts:
- `p.additionalHeaders as Record<string, string>` claimed every value in
an Object property is a string. It is Record<string, unknown> and the
engine does not narrow it, so {"X-Foo": 123} reached the HTTP client as
a number. All eight call sites now use toHeaderRecord, which the piece
already applies to the connection's own headers.
- `context.auth as OroAuth` was doing nothing in six actions —
create-invoice, create-order and the trigger pass context.auth
unchanged and compile fine. Removed, along with the imports that only
existed to serve them.
Also:
- create-order read a `warehouseId` line-item field that no prop defines,
so the first half of the `??` could never be satisfied.
- create-invoice carried a comment copied from create-order describing a
StaticDropdown of product units loaded once. This file loads nothing
and its Product Unit is a ShortText. Removed; the DynamicProperties
wrapper is left alone because unwrapping it would move the line-item
labels into piece metadata and change the i18n key set.
- The webhook trigger's onDisable deleted the webhook in Oro but left
webhookInfo in the store. Harmless while it only holds an id; not
harmless once it holds a secret.
- check-i18n reported a malformed locale file as a bare "Unexpected end
of JSON input" with six candidate files and no hint which. It now names
the path. --write reported dropped keys as a count, which is no help
when the count is a dozen human translations; it now lists them.
- The i18n gate's own tool shipped unlinted, because eslint could not
parse .mjs here at all. Added the override and widened the lint glob.
- Dropped the @activepieces/shared alias from vitest.config.ts. Nothing
imports it, the framework does not need it transitively, and the
piece's own eslintrc bans that import.
- New test/body-utils.test.ts: the helpers that decide what actually goes
on the wire had no tests, including every throw branch of the two
Additional* parsers.
The README's claim that this piece keeps explanatory prose out of the
source was not true (60 section markers and 15 prose comments), and its
claim that i18n:write is "the same thing" as the CLI generator was not
either — the CLI writes one file, i18n:write writes six, and they
disagree on the trailing newline. Both corrected, and the checkbox
gotcha below is recorded there.
Left alone deliberately: an untouched Property.Checkbox is persisted as
false, not undefined, so update-user and update-customer-user send
enabled: false on every call and assertUpdateNotEmpty can never fire for
them. The fix is a three-state prop, which changes the props and forces
an i18n regeneration, so it wants its own change. Written up under
Gotchas in the README with the mechanism and why a defaultValue cannot
fix it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The i18n gate was added as its own background `turbo run` next to the test one. Both tasks declare dependsOn: ["build"], and turbo only serialises tasks within a single invocation — across two processes nothing stops piece-orocommerce#build running twice at once against the same dist/, which is the directory check-i18n.mjs imports the built piece from. In practice the earlier "Build changed pieces projects" step warms the cache and both runs hit it, so this has not bitten. But that step is conditional on the piece appearing in the changed-pieces filter, and a PR that touches only tools/check-i18n.mjs, turbo.json or ci.yml does not put it there — so the race is reachable exactly on the PRs that change this tooling. Running both tasks in one invocation lets turbo own the graph. Confirmed with --dry=json: piece-orocommerce#build is scheduled once, and packages that do not define i18n:check resolve to <NONEXISTENT> rather than failing. Also gave i18n:write a turbo task. i18n:check gets a fresh dist/ because turbo builds first; i18n:write was only an npm script, so editing a prop description and running it reconciled all six files against the previous build and then failed CI on drift. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The trigger returned [context.payload.body] with no verification at all,
so anyone who learned the webhook URL could inject fabricated events into
a flow. Oro can sign deliveries; the connector simply never asked it to.
onEnable now generates a 32-byte secret, sends it as the webhook's
`secret` attribute at create, and keeps the only usable copy in the
flow-scoped store. run() recomputes hash_hmac('sha256', rawBody, secret)
and compares it to the `Webhook-Signature` header before returning
anything.
Verification is keyed off "this trigger stored a secret", not off the
header being present: Oro's addSignature returns early and silently when
a webhook has no secret, so trusting header presence would let an
attacker skip the check by omitting the header. Entries written before
this change carry no secret and keep running unverified — that absence is
the whole migration.
It verifies payload.rawBody, never a re-serialized payload.body. The
server captures rawBody as a UTF-8 string for the string-parsed content
types, which is what Oro sends; JSON round-tripping reorders keys and the
digest would never match. A missing rawBody with a secret stored is a
rejection, not a fallback.
A rejected delivery is logged with console.warn and returns [] — no run
is created, the caller still gets its 200, and no secret material reaches
the log. The success path still returns [context.payload.body]; returning
the whole payload would change the output schema of every existing flow.
The comparison is the verifyHmacAuth shape from the webhook core piece —
explicit length check, then timingSafeEqual — copied rather than imported
because pieces may not depend on each other. Not the attio shape, which
has no length check and throws on a malformed header.
Two compensating deletes:
- onEnable creates the webhook before it can store the secret. A failing
store.put would leave a live webhook whose secret is unrecoverable and
unchangeable, so every delivery would be discarded forever and the only
recovery would be disable/enable. The put is wrapped; on failure the
webhook is deleted and the original error rethrown.
- Republishing a flow calls triggerSourceService.enable without ever
calling disable (flow.service.ts -> flow-service-side-effects.ts ->
trigger-source-service.ts), so onEnable can run with no matching
onDisable. It now deletes a leftover registration found in the store
before creating a replacement; the store is keyed by flowId, not flow
version, so that entry is still visible.
The `secret` attribute needs OroCommerce 6.1, so signing is behind a
"Sign webhook deliveries" checkbox defaulting to true. This is a create
call, not an update, so there is no "leave unchanged" semantic and the
default is safe. Turning it off generates and sends nothing and skips
verification. If Oro rejects the create because of the attribute the
error surfaces as-is rather than silently retrying unsigned.
node:crypto only, no new dependency.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
….71.0 Review flagged the console.warn in run() as the riskiest line in the file: it read context.flows.current.id and context.step.name, which the framework types guarantee but the runtime only partially does. flows has been on the trigger run context since at least release 0.46.0, but step was only added in d64d7bf (2025-10-12), first shipped in 0.71.0. On any older engine the plain property access throws TypeError on the rejection path — turning "discard the forged delivery" into "error the delivery", on the security path of all places. Both reads now go through optional chaining with an 'unknown' fallback, and a test runs the rejection path against a context shaped like a pre-0.71.0 engine (no flows, no step) to prove it returns [] instead of throwing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ng change The reviewer overruled the PR's "not breaking" self-assessment: the template defines functional as "default/limit/behaviour change", and a default-on behaviour whose first failure appears at the first re-enable after upgrade reads, from the user's chair, as "the upgrade broke my trigger". The entry sits under 0.87.0 because that is this tree's version; move it if the fork's release numbering says otherwise. Placed with the label so breaking-change-check's invariant (label and docs entry travel together) holds if this is ever retargeted upstream — the job itself is gated to github.repository == activepieces/activepieces and does not run on the fork. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hardening # Conflicts: # .github/workflows/ci.yml
# Conflicts: # docs/install/reference/breaking-changes.mdx
Every job in ci.yml is guarded by
if: github.repository == 'activepieces/activepieces'
so on oroinc/activepieces lint, main and tool-search-postgres skip
unconditionally. The same guard is on breaking-change-check.yml,
pr-size.yml and setup-environment.yml. Validate PR title is the only
check that has ever run on a fork PR — every other check on #4 and #5
reports SKIPPED, including the i18n gate and the piece tests added in
7ca5361 and 36372f4. Neither has executed once.
Relaxing the upstream guard was the other option and is worse: it edits
a file that conflicts on every sync, and the jobs behind it want
TURBO_CACHE_S3_* secrets the fork does not have (gh secret list is
empty), plus a full-monorepo build the fork has no reason to pay for.
So this is a fork-owned workflow instead, guarded the other way round
(!= upstream) so it stays inert if the piece is ever contributed
upstream, and scoped to the one package the fork owns. No paths filter:
the piece breaks from upstream merges that touch the framework, not
only from edits under its own directory, and those merges do not touch
it.
Verified the gate fails as intended, not just passes — a stale key in
translation.json, a key dropped from de.json, and a changed piece
description each exit 1 with the offending keys named. Also confirmed
the 113-commit merge did not move anything check-i18n.mjs hardcodes:
pathsToValuesToTranslate is the same 19 paths, MAX_KEY_LENGTH_FOR_CORWDIN
is still 512, LocalesEnum is still the same 10, and the CLI generator's
getPropertyValue is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hey patch Update User and Update Customer User sent enabled: false on every run, so "Update User, change the last name" also locked the account out. Verified against a live Oro 7.1 instance: oro_user 6 went Pace|t -> SmokeTest-V1|f and oro_customer_user 1 went Cole|t -> SmokeTest-V2|f, each from a step whose Enabled box was never touched. The second one is a storefront login. The cause is not in this piece. The builder seeds an unset checkbox with property.defaultValue ?? false (web/src/features/pieces/utils/form-utils.tsx) and persists it into the step input, and checkboxProcessor passes false through - Checkbox is the one property type whose empty form value is not normalised to undefined the way textProcessor and numberProcessor normalise theirs. So `p.enabled ?? undefined` was false, pickDefined kept it, and assertUpdateNotEmpty could never fire for either action: there was always one attribute to send. Both flags, and Confirmed on the customer user, are now a three-state StaticDropdown defaulting to "Leave unchanged", the shape campaign-monitor's update-subscriber-details already uses. A defaultValue on the checkbox was the other option and is worse: true would unconditionally enable instead. readBooleanUpdate ignores a boolean false, which is what a step saved by the checkbox version of the prop still holds. There it means "untouched" at least as often as it means "disable", and the two are indistinguishable, so the safe reading is to leave the record alone - an existing flow stops disabling its target rather than carrying the defect forward. Set the dropdown to "No" to disable deliberately. Re-verified on the same rig, three runs through the flow-operations API: untouched Enabled plus a new last name leaves enabled = t; a step input holding the old boolean false also leaves enabled = t; "No" still sets enabled = f. And an update whose only field is an untouched flag now reports "nothing to update" instead of quietly disabling the account. i18n regenerated: +3 keys for the option labels, none dropped, so no existing translation is lost. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All three were found by running the actions against a live Oro 7.1 instance;
each one reached Oro and came back 400, or worse, succeeded wrongly.
Product Unit on an invoice line item was optional in the piece and is not
optional in Oro: leaving it empty answered
400 not blank constraint at /included/0/attributes/unitOfQuantity
which names neither the row nor the field the user filled in. It is now
required and validated through lineItemUtils.requiredString like Quantity,
so the message says "Line Items row 1, "Product Unit" is required". No
working flow changes behaviour — a flow that left it empty could not
succeed before either.
The invoice attachment was sent with a hardcoded mimeType of
application/pdf whatever the file was, and Oro does not sniff the content:
a PNG attached to Invoice PDF was accepted and stored as file 705,
original_filename unknown.png, extension png, mime_type application/pdf.
Nothing failed, so nothing said the invoice's default PDF is now a file that
no PDF reader will open. Deriving the type from the extension was the other
option; it is worse here, because the relationship this file lands on is
invoiceDefaultPdfFile and Oro renders it as the invoice PDF, so a non-PDF is
a mistake to report rather than a type to record. readPdfContent checks for
the %PDF- signature and names the offending file. Re-verified live: the PNG
is now refused with no invoice created, and a real PDF still lands - invoice
60, file 706, pr4-fix.pdf / pdf / application/pdf, all three agreeing.
A null value in Additional Relations was passed through to Oro, which
answers
400 request data constraint - The relationship should have 'data' property
so the request could never have succeeded. It is rejected up front with the
linkage forms spelled out. {"data": null} stays accepted and does clear a
to-one relationship - confirmed against the rig, not assumed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hour on Neither is discoverable from the piece, and both look like something else. The OAuth application's organization scopes every record the connection can reach. On the rig, customers 1-4 belong to ACME, Inc and the client's user to Oro, and every one of them answers 403 No access to the entity - the same status a missing permission gives, so the first hour went into roles that were already correct. Customer 5 worked immediately. The webhook trigger cannot be published until the entity is marked webhook accessible in Oro. Until then the Topic dropdown is empty for that entity and publishing fails with valid webhook topic constraint, which reads as a bug in the trigger. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The only change this branch makes to .nvmrc is stripping the newline at end of file - same version, v24.14.0, before and after. An editor artifact, and it sits in a root file upstream owns, so it would conflict on a future sync for no benefit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(orocommerce): harden the piece and fix what the review turned up
Six commits hardening the piece, two fixing what reviewing them turned up, three fixing what running them against a live Oro 7.1 turned up, plus CI and an upstream merge.
The one worth knowing about: Update User and Update Customer User sent enabled: false on every run, so "change the last name" also locked the account out. Confirmed on a live instance, not inferred — a back-office user and a storefront login each went from enabled to disabled from a step whose Enabled box was never touched. The cause is in Activepieces rather than the piece: the builder seeds an unset checkbox with defaultValue ?? false and checkboxProcessor passes it through, so ?? undefined never fired, and the no-op guard could never trigger because there was always one attribute to send. Enabled and Confirmed are now a three-state dropdown defaulting to "Leave unchanged", and a step input saved by the old checkbox version stops disabling its target.
Three more that only a live instance could find. Product Unit was optional in the piece and mandatory in Oro, so an empty one came back 400 naming neither the row nor the field. A PNG attached to Invoice PDF was accepted and stored as application/pdf, leaving an invoice whose default PDF no reader opens — nothing failed, so nothing said so. A null Additional Relations value could only ever be rejected; {"data": null} stays accepted and still clears a to-one relationship.
The rest: line items are validated instead of coerced with Number(), so a missing quantity no longer reaches Oro as null via JSON.stringify(NaN); an update with nothing filled in is rejected instead of PATCHing an empty body and reporting success; the invoice PDF is a Property.File rather than a ShortText you paste base64 into; minimumSupportedRelease said 0.36.1 when the piece cannot load below 0.86.0; '0x10' is no longer a quantity of 16; and the false additionalHeaders as Record<string, string> cast no longer lets a number reach the HTTP client.
CI gates the six translation files against the piece's exposed strings, using an algorithm proven byte-identical to pieces generate-translation-file. That gate had never actually run: every job in ci.yml is guarded by github.repository == 'activepieces/activepieces', so on this fork they all skip and Validate PR title was the only check that had ever executed. oro-ci.yml is guarded the other way round and runs build/lint/test/i18n:check for this package on every PR.
78 tests across 5 files, 258 i18n keys, green. Every action was also exercised against a live Oro 7.1 instance.
Behaviour changes are confined to the piece. Inputs that were silently coerced now error, Product Unit is required where it was optional, and a non-PDF attachment is refused — none of these had a working flow behind them. No env vars, API surface or migrations.
Left for follow-ups: about half of each locale file is still English, which the gate reports as a warning; pl.json and uk.json are dead files that 0.88.1 drops at install; and .agents/skills/orocommerce-action-builder/SKILL.md still maps every boolean flag to Property.Checkbox, which is how the disabling bug got written — it lives on another branch, so it's queued as skill-boolean-update.patch.
…hook-signature # Conflicts: # packages/pieces/community/orocommerce/README.md
mikhail-yahorau
changed the title
poc/orocommerce is the branch proposed upstream, so it carries the piece and nothing else. Reverted: the i18n:check task and the piece filter added to .github/workflows/ci.yml, the i18n:check and i18n:write tasks in turbo.json, the whole of .github/workflows/oro-ci.yml, and an accidental trailing newline in .nvmrc. bun.lock keeps the workspace entry. All 727 community pieces on main have one, and without it the next bun install re-adds it. The i18n gate moves inside the piece: test/i18n.test.ts runs tools/check-i18n.mjs, so the root test task covers it with no task of its own in turbo.json. i18n:write stays a package script and never needed one. tools/check-scope.mjs is what stops this recurring — it diffs origin/poc/orocommerce...HEAD and fails on any file outside the package. Nothing in .github/workflows/ now runs the piece's checks on the fork. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
refCustomerId and refCustomerUserId are integers in Oro, so they go back to Property.Number. Making them ShortText was wrong: the Oro field is typed, and the arbitrary-external-id reading of the description does not survive that. invoicePdfContent goes back to a base64 LongText. Property.File cannot be fed from a transformation: the engine's file processor takes a data: URI or a URL and nothing else, so a bare base64 string from an earlier step falls through to new URL(), throws, and is swallowed into null — the action would then file the invoice with no PDF and no error. It now accepts base64 with or without the data: prefix, keeps the %PDF- signature check, and falls back to invoice.pdf for the filename. Long prop descriptions break the builder layout. The four password descriptions, the update-user action description, the confirmed checkbox and the connection's custom-headers description are cut to one line each; what they explained is in the README. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Drop the breaking-changes.mdx entry: poc/orocommerce is the branch that becomes the upstream PR and must stay a piece-only diff. The OroCommerce 6.1 floor and the "untick before republishing on older instances" note now live in the piece README next to the checkbox they describe. Cut the Sign webhook deliveries description to one sentence under the 200-character prop limit and reseed its i18n key in every locale file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- BAP-23451: Frontend review and fixes of OroCommerce piece
Drop every OroCommerce version mention from the signing feature. Webhooks and their secret shipped as one unit, so a build that has webhooks but rejects the secret does not exist — the "needs 6.1, turn off for older versions" framing described nothing. The checkbox stays, re-justified as the escape hatch for a proxy between Oro and Activepieces that re-encodes the body: the signature covers the exact bytes delivered, so such a proxy makes every delivery fail verification. The prop description is one sentence; the off-switch guidance lives in the README. Comment the stale-registration cleanup in onEnable with why it exists and what it prevents, as requested on review. Rewrite the README webhook section down from ~40 lines to the facts a maintainer acts on: what Oro signs, when verification runs, the two compensating deletes, and that a rejected delivery is silent — no run, Oro still gets its 200 — so a quiet signed trigger means checking the worker logs for "webhook delivery discarded". The engine-version archaeology and call-chain traces stay in the commit history. The description is an i18n key, so all six locale files are reseeded; the dropped translations were English seeds, nothing hand-translated is lost. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
0.2.0 is already installed out there, and POST /v1/pieces rejects a second upload of the same name+version with 409 piece_metadata_already_exists, so this build could not be installed over it at all. Verified on a CE 0.88.1 rig. Minor rather than patch because the webhook trigger gains a signDeliveries prop. Also fixes the why-comment on the onEnable stale-registration cleanup. It said republishing runs onEnable without a matching onDisable; on 0.88.1 LOCK_AND_PUBLISH calls onDisable first, and republishing an enabled flow leaves exactly one webhook row in Oro with or without the cleanup. Keeping the cleanup as a guard for enable-without-disable paths, and saying so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Adds signature verification to the Oro Webhook Event trigger, so a flow starts only from deliveries that the connected OroCommerce instance signed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.