This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Add support for Order Source Attribution #11506
Closed
Conversation
This file contains 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
15 tasks
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
assets/js/extensions/order-source-attribution/index.ts
|
|
Size Change: +265 B (0%) Total Size: 1.58 MB
ℹ️ View Unchanged
|
Save the data to the order here.Save the data to the order here.
woocommerce-blocks/src/Domain/Services/OrderSourceAttribution.php Lines 142 to 153 in f71ba32 🚀 This comment was generated by the automations bot based on a
|
JPry
force-pushed
the
feature/order_source_attribution
branch
from
8d69dc5 to
a8cb8ea
Compare
Remove this try/catch once the PR is merged.Remove this try/catch once the PR is merged.
woocommerce-blocks/src/Domain/Bootstrap.php Lines 150 to 156 in 3d14f8c 🚀 This comment was generated by the automations bot based on a
|
JPry
force-pushed
the
feature/order_source_attribution
branch
from
3d14f8c to
8d876ee
Compare
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
This depends on woocommerce/woocommerce#39701 being merged. This can be tested locally, but running automated testing will fail due to the files in WooCommerce core being missing.
budzanowski
force-pushed
the
feature/order_source_attribution
branch
2 times, most recently
from
1bf5be1 to
a880396
Compare
|
It would be very useful to add the scope of the project and more context to the PR description to aid in review, including:
|
mikejolley
reviewed
Nov 15, 2023
| @@ -134,6 +139,18 @@ function() { | |||
| $this->container->get( TasksController::class )->init(); | |||
| $this->container->get( JetpackWooCommerceAnalytics::class )->init(); | |||
|
|
|||
| try { | |||
There was a problem hiding this comment.
I understand this is temporary but wouldn't it be better to gate this based on class existence either here or within the init method?
mikejolley
reviewed
Nov 15, 2023
| return true; | ||
| }; | ||
|
|
||
| $sanitize_callback = function( $value ) { |
There was a problem hiding this comment.
If this is unlikely to change you can just pass sanitize_text_field as the sanitize_callback instead of using an inline function.
|
I believe that rather than introducing this feature via two repositories (blocks and core) this integration should be made directly into core, in the same way this feature supports WC.Com, via this integration: The reasons being:
I don't see anything in this PR that specifically requires it to be bundled with blocks plugin. Everything seems to be using extensibility interfaces that are available to 3rd parties. it does currently make use of the build process in blocks but this is not required. JavaScript
PHP
For the main JS module which is currently: You can avoid the use of imports/the blocks build process by accessing the window properties. e.g. I think thats the correct method but @opr or @senadir can chime in here. I tested in console successfully. cc @ralucaStan |
Yep, that's it. We really need to remove the |
github-actions
bot
removed
the
status: stale
|
Discussion about why this would live here: p1698288117402579-slack-C8X6Q7XQU The reasoning would be that it's easier to integrate here directly than to do it in core. My reasoning given this is a core feature, it doesn't need any third party code, it should be just part of Checkout Block source code than live externally. |
|
@senadir I saw your reasoning. If blocks were in the monorepo I'd agree, but as the feature is being introduced in core to support both block and legacy checkouts it should be done there and moved after the core merge. This cannot be tested without the core feature branch and the code would be near identical, just in a different repository. |
tomalec
added a commit
to woocommerce/woocommerce
that referenced
this pull request
Nov 20, 2023
woocommerce/woocommerce-blocks#11506 Co-authored-by: Jeremy Pry <jeremy.pry@gmail.com>
14 tasks
tomalec
added a commit
to woocommerce/woocommerce
that referenced
this pull request
Nov 20, 2023
woocommerce/woocommerce-blocks#11506 Co-authored-by: Jeremy Pry <jeremy.pry@gmail.com>
|
I started migrating this PR to the monorepo. My WIP is at https://github.com/woocommerce/woocommerce/pull/41592/files |
tomalec
added a commit
to woocommerce/woocommerce
that referenced
this pull request
Nov 21, 2023
woocommerce/woocommerce-blocks#11506 Co-authored-by: Jeremy Pry <jeremy.pry@gmail.com>
tomalec
added a commit
to woocommerce/woocommerce
that referenced
this pull request
Nov 21, 2023
woocommerce/woocommerce-blocks#11506 Co-authored-by: Jeremy Pry <jeremy.pry@gmail.com>
|
Closing in favour of woocommerce/woocommerce#41592 |
layoutd
pushed a commit
to woocommerce/woocommerce
that referenced
this pull request
Nov 24, 2023
woocommerce/woocommerce-blocks#11506 Co-authored-by: Jeremy Pry <jeremy.pry@gmail.com>
layoutd
pushed a commit
to woocommerce/woocommerce
that referenced
this pull request
Nov 24, 2023
woocommerce/woocommerce-blocks#11506 Co-authored-by: Jeremy Pry <jeremy.pry@gmail.com>
layoutd
pushed a commit
to woocommerce/woocommerce
that referenced
this pull request
Nov 27, 2023
woocommerce/woocommerce-blocks#11506 Co-authored-by: Jeremy Pry <jeremy.pry@gmail.com>
layoutd
pushed a commit
to woocommerce/woocommerce
that referenced
this pull request
Nov 27, 2023
woocommerce/woocommerce-blocks#11506 Co-authored-by: Jeremy Pry <jeremy.pry@gmail.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR is an effort to bring Order Source Attribution support to the Cart and checkout blocks.
For reference: pe2C5g-JD-p2
Order Source Attribution is a new core feature that will show merchants additional information about the order ( Order Information ):
The core implementation of the Order Source Attribution will be delivered using the following PR woocommerce/woocommerce#39701
Why
Since Blocks-based checkout is now the default we need to provide compatibility with the Order Source Attribution feature.
Testing Instructions
To fully test one needs the WooCommerce core woocommerce/woocommerce#39701. Please use the testing instructions there with the combination of Cart & Checkout blocks instead of the classic flow.
After an order is made:
Screenshots or screencast
WooCommerce Visibility
Required:
Checklist
Required:
[type]label or a[skip-changelog]label.Conditional:
[skip-changelog]label is not present).Changelog