Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds optional channel prop to ShopPayButton #1447

Merged
merged 4 commits into from
Nov 6, 2023
Merged

Adds optional channel prop to ShopPayButton #1447

merged 4 commits into from
Nov 6, 2023

Conversation

QuintonC
Copy link
Contributor

@QuintonC QuintonC commented Oct 25, 2023 •
edited
Loading

WHY are these changes introduced?

This adds an optional channel prop to the ShopPayButton which will be used to add attribution to an order.

Note
The channel being specified (either Headless or Hydrogen) must be installed otherwise validation will fail at checkout and the buyer will be redirected to the online store domain.

WHAT is this pull request doing?

This change leverages a change made in Shop JS that allows channel to be forwarded to the checkout for order attribution. Now, when developers add either headless or hydrogen to their ShopPayButton props, they will be able to ensure orders are attributed to the correct channel.

HOW to test your changes?

Important
You will need admin access for a storefront to test these changes.

It is recommended to test this change using a store of your own using a free product.

  1. Navigate to the free product's product details page.
  2. Ensure you've included either headless or hydrogen as the prop on your ShopPayButton
    a. Use the channel that aligns with the channel you've added to your storefront.
  3. Pressing the ShopPayButton should direct you to your checkout without any errors.
  4. Confirm payment (again, it's recommended you use a free item)
  5. Check your Orders page in admin and check the order attribution for your test order. It should reflect the correct channel.

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or noteworthy changes
  • I've added tests to cover my changes
  • I've added or updated the documentation

@QuintonC QuintonC force-pushed the shop-pay/channel-attribution branch from 84a05bd to 65497b1 Compare October 25, 2023 00:24
@QuintonC QuintonC force-pushed the shop-pay/channel-attribution branch from 65497b1 to a69cd54 Compare October 25, 2023 15:05
@github-actions github-actions bot had a problem deploying to preview October 25, 2023 15:10 Failure
@QuintonC QuintonC changed the title [WIP] Adds optional channel prop to ShopPayButton Adds optional channel prop to ShopPayButton Oct 25, 2023
@QuintonC QuintonC force-pushed the shop-pay/channel-attribution branch 2 times, most recently from 307c553 to 8d6a4a9 Compare October 25, 2023 21:56
@QuintonC QuintonC force-pushed the shop-pay/channel-attribution branch from 8d6a4a9 to a371b3a Compare October 25, 2023 21:58
@QuintonC QuintonC force-pushed the shop-pay/channel-attribution branch from a371b3a to 6750dc1 Compare October 26, 2023 19:43
@QuintonC QuintonC self-assigned this Oct 30, 2023
@QuintonC QuintonC force-pushed the shop-pay/channel-attribution branch from 6750dc1 to e20cda8 Compare October 30, 2023 23:00
@QuintonC QuintonC marked this pull request as ready for review October 30, 2023 23:11
@QuintonC QuintonC requested review from benjaminsehl and a team October 30, 2023 23:12
benjaminsehl
benjaminsehl reviewed Nov 3, 2023
View reviewed changes
Copy link
Member

@benjaminsehl benjaminsehl left a comment

Choose a reason for hiding this comment

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

I think (but need a gut check) that the changeset should be a patch. (Weird, I know)

I also think we should be modifying the reexported package from @shopify/hydrogen to just set channel for you as hydrogen. That way anyone that updates gets a non breaking change that automatically fixes the problem, and it keeps the DX cleaner.

I'd leave it unset in @shopify/hydrogen-react since folks can use that in a number of ways.

.changeset/small-zoos-grab.md Outdated
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be a patch I think... because of our kinda weird versioning strategy

@QuintonC
Copy link
Contributor Author

QuintonC commented Nov 3, 2023

I also think we should be modifying the reexported package from @shopify/hydrogen to just set channel for you as hydrogen. That way anyone that updates gets a non breaking change that automatically fixes the problem, and it keeps the DX cleaner.

I like this idea, good thinking! I'll make this change Monday morning 🙌

wizardlyhel
wizardlyhel approved these changes Nov 6, 2023
View reviewed changes
Copy link
Contributor

@wizardlyhel wizardlyhel left a comment

Choose a reason for hiding this comment

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

Overall this looks good. Please update changeset to patch

packages/hydrogen-react/src/ShopPayButton.tsx
@@ -36,11 +37,17 @@ type ShopPayVariantAndQuantities = {
}>;
};

type ShopPayChannelAttribution = {
/** A string that adds channel attribution to the order. Can be either `headless` or `hydrogen` */
channel?: 'headless' | 'hydrogen';
Copy link
Contributor

@wizardlyhel wizardlyhel Nov 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Is the channel limited to just headless or hydrogen because that is the only possible values that <shop-pay-button> accepts ... or are the values something that scopes to hydrogen-react and hydrogen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, valid values for cart permalink attribution are:

  • buy_button
  • online_store
  • headless
  • hydrogen
  • headless-storefronts
  • and hydrogen-5

On the backend, hydrogen and headless are mapped to the attributed clients (hydrogen-5 and headless-storefronts). The limitations aren't with shop-pay-button.

Copy link
Contributor

Choose a reason for hiding this comment

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

=_=

Okay. Let's stay with headless and hydrogen and not get everyone confused with which one they should using.

@QuintonC QuintonC force-pushed the shop-pay/channel-attribution branch from b4c3054 to f29d27a Compare November 6, 2023 20:23
@wizardlyhel wizardlyhel merged commit e8cc49f into main Nov 6, 2023
@wizardlyhel wizardlyhel deleted the shop-pay/channel-attribution branch November 6, 2023 20:29
@shopify-github-actions-access shopify-github-actions-access bot mentioned this pull request Nov 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjaminsehl benjaminsehl benjaminsehl left review comments

@wizardlyhel wizardlyhel wizardlyhel approved these changes

Assignees

@QuintonC QuintonC

Labels
Feature @shopify/hydrogen-react
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@QuintonC @benjaminsehl @wizardlyhel