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.

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

Support channel setting: AutomaticallyCompleteFullyPaidCheckouts #5199

Merged
merged 10 commits into from
Nov 4, 2024

Conversation

lkostrowski
Copy link
Member

@lkostrowski lkostrowski commented Oct 4, 2024

Waiting for API to be released everywhere in Saleor Cloud

What type of PR is this?

  • 💅 Refactor
  • 🌟 Feature
  • 🔥 Bug Fix
  • 🔩 Maintenance
  • 🛠 Workflow CI/CD changes

Related Issues or Documents

saleor/saleor-docs#1334
saleor/saleor#16782

  • closes #

Usage Instructions, Screenshots, Recordings

CleanShot 2024-10-04 at 09 41 42

Have you written tests?

  • Yes!
  • No... here is why: I don't see what to test here

[Optional] Description

Copy link

changeset-bot bot commented Oct 4, 2024

🦋 Changeset detected

Latest commit: 52cdbd8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
saleor-dashboard Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@lkostrowski lkostrowski requested a review from a team October 4, 2024 07:51
@github-actions github-actions bot temporarily deployed to pr-5199 October 4, 2024 07:53 Destroyed
poulch
poulch previously approved these changes Oct 4, 2024
values={{
link: (
<Link
href="https://docs.saleor.io/developer/payments/transactions#automatic-checkout-completion"
Copy link
Member

Choose a reason for hiding this comment

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

question: what about putting this link in src/links.ts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. I suggest you add ESLint rule that will suggest such change

@@ -212,6 +216,13 @@ export const ChannelForm: React.FC<ChannelFormProps> = ({
hasError={!!formErrors.defaultTransactionFlowStrategy}
disabled={disabled}
/>
<Box />
Copy link
Member

Choose a reason for hiding this comment

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

questtion: do we need this empty box?

Copy link
Member Author

Choose a reason for hiding this comment

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

CleanShot 2024-10-04 at 11 03 49

container is written incorrectly, so Box seems to be used to push the component to the new line. This should be refactored but its out of the scope of my changes


const handleAutomaticallyCompleteCheckoutsChange = () => {
set({
automaticallyCompleteCheckouts: !data.automaticallyCompleteCheckouts,
Copy link
Member

Choose a reason for hiding this comment

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

question: why do you use !?

Copy link
Member Author

Choose a reason for hiding this comment

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

Becasue its a checkbox, so it inverts data (false → true, true → false)
The handler doesnt receive the change, it uses current data to flip the state

@szczecha
Copy link
Member

szczecha commented Oct 4, 2024

  1. Empty order history for the event PLACED_AUTOMATICALLY_FROM_PAID_CHECKOUT Screenshot 2024-10-04 at 09 56 13
  2. When creating a new channel, we don't send checkout settings in channelCreate input
Screen.Recording.2024-10-04.at.10.13.52.mov

@lkostrowski
Copy link
Member Author

CleanShot 2024-10-04 at 11 17 47

@lkostrowski
Copy link
Member Author

CleanShot 2024-10-04 at 11 26 44

@lkostrowski
Copy link
Member Author

@szczecha @poulch applied fixes for your comments

@github-actions github-actions bot temporarily deployed to pr-5199 October 4, 2024 09:31 Destroyed
Copy link

This pull request is stale because it has been open 14 days with no activity.

@github-actions github-actions bot added the stale label Oct 30, 2024
Copy link

github-actions bot commented Nov 1, 2024

This PR request has been closed because it has been stalled for 2 days with no activity. You are still welcome to reopen it and continue from where you finished. Best regards Saleor team

@github-actions github-actions bot closed this Nov 1, 2024
@lkostrowski lkostrowski reopened this Nov 4, 2024
@lkostrowski lkostrowski marked this pull request as ready for review November 4, 2024 07:24
@lkostrowski lkostrowski requested a review from a team as a code owner November 4, 2024 07:24
@andrzejewsky andrzejewsky merged commit 5b6ed8c into main Nov 4, 2024
19 of 20 checks passed
@andrzejewsky andrzejewsky deleted the SHOPX-1418 branch November 4, 2024 15:54
Cloud11PL pushed a commit that referenced this pull request Nov 7, 2024
* Regenerate graphql types

* Fetch new field from api, stub display

* add setting to mutation

* ui

* better messages

* changeset

* fix fixtures

* Include new field in channel creation

* add new event mapping and extract messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants