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

feat(default-theme): promotion code functionality #1155

Merged

Conversation

niklaswolf
Copy link
Collaborator

Changes

closes #1154

  • User can add/remove promotion codes
  • List of applied promotion codes (labels of promotion are displayed)
  • Cart totals update accordingly

Help wanted

  • showing error message (toast) when there is an error returned from API (e.g. promo code does not exist)
  • Mobile: where should the promo code input be placed as there is no such thing as the sidebar on desktop

Testing

To test this, a promotion code has to be added to the backend.

Checklist

@vercel
Copy link

vercel bot commented Oct 5, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/shopware-pwa/shopware-pwa-docs/7m8icg3a6
✅ Preview: https://shopware-pwa-docs-git-1154-promotion-code-functionality.shopware-pwa.vercel.app

@vercel vercel bot temporarily deployed to preview October 5, 2020 14:25 Inactive
@niklaswolf niklaswolf marked this pull request as draft October 5, 2020 14:25
@vercel vercel bot temporarily deployed to preview October 5, 2020 14:38 Inactive
@vercel vercel bot temporarily deployed to preview October 6, 2020 09:42 Inactive
@mkucmus
Copy link
Collaborator

mkucmus commented Oct 7, 2020

in regards to point 2. from help wanted:
you can follow that design: https://storybook.storefrontui.io/?path=/story/pages-checkout--common
image

and on the occasion please resolve the conflicts.

…de-functionality

# Conflicts:
#	packages/default-theme/components/checkout/sidebar/SidebarOrderSummary.vue
#	packages/default-theme/locales/de-DE.json
#	packages/default-theme/locales/en-GB.json
@vercel vercel bot temporarily deployed to preview October 8, 2020 08:19 Inactive
@niklaswolf
Copy link
Collaborator Author

@mkucmus thanks, I added toast notifications and resolved the conflicts. :)
Regarding my second point: in the UI you linked https://storybook.storefrontui.io/?path=/story/pages-checkout--common there is no coupon code input for mobile devices. That's what my question was about: where should the coupon input field be placed for mobile devices?

@mkucmus
Copy link
Collaborator

mkucmus commented Oct 8, 2020

@mkucmus thanks, I added toast notifications and resolved the conflicts. :)
Regarding my second point: in the UI you linked https://storybook.storefrontui.io/?path=/story/pages-checkout--common there is no coupon code input for mobile devices. That's what my question was about: where should the coupon input field be placed for mobile devices?

here is a design from storefront-ui figma:

image

@niklaswolf
Copy link
Collaborator Author

@mkucmus okay that looks good. Right now the whole totals section is missing in mobile checkout. So to keep this PR in scope I'd not add the whole totals section in this PR, but create a new issue for it. The scope of the new issue should then also include adding the coupon code component from this PR, which should be usable by just dropping it in.
Would this be okay?

@vercel vercel bot temporarily deployed to preview October 9, 2020 11:55 Inactive
Copy link
Collaborator

@patzick patzick left a comment

Choose a reason for hiding this comment

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

Thanks, few notes from me.
Also a question - why there is so big difference in generated api/composables.api.md? Have you linted it? If so please generate new one without linting, so it should only show changes made by this PR in public API.
Cheers!

import { getApplicationContext } from "@shopware-pwa/composables";
import {
getApplicationContext,
useNotifications,
Copy link
Collaborator

Choose a reason for hiding this comment

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

notifications Should not be used inside other composables. Users need to have a choice of how they want to react on events across the system. Use interceptors instead: https://shopware-pwa-docs.vuestorefront.io/landing/concepts/interceptor.html#events-interceptor

import { ApplicationVueContext } from "../../appContext";

const TYPE_PROMOTION = "promotion";
Copy link
Collaborator

Choose a reason for hiding this comment

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

this could be a type in commons package
example

export type CartItemType = "promotion" | "product"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right and there even is a CartItemType interface already with the needed types... I'll use that one.

packages/composables/src/hooks/useCart/index.ts Outdated Show resolved Hide resolved
packages/composables/src/hooks/useCart/index.ts Outdated Show resolved Hide resolved
Comment on lines 105 to 116
// It's strange that success also ends up as an error in the API response
const err = <any>Object.values(result.errors)[0];
switch (err.messageKey) {
case "promotion-discount-added":
pushSuccess(rootContext.$t("Promotion code added successfully"));
break;
case "promotion-not-found":
pushError(rootContext.$t("Promotion code does not exist"));
break;
default:
pushError(err.message.toString());
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

this logic should go into notifications.js

Co-authored-by: Patryk Tomczyk <13100280+patzick@users.noreply.github.com>
Co-authored-by: Patryk Tomczyk <13100280+patzick@users.noreply.github.com>
@vercel vercel bot temporarily deployed to preview October 9, 2020 15:08 Inactive
@vercel vercel bot temporarily deployed to preview October 12, 2020 09:53 Inactive
@niklaswolf niklaswolf marked this pull request as ready for review October 12, 2020 09:53
@niklaswolf
Copy link
Collaborator Author

@patzick I think I have tackled all your notes.
Regarding composables.api.md: I don't know why it's displaying such a big diff, I did not lint it and with my last commit I even regenerated it, but still no difference. My Diff-Viewer in my IDE only shows the small changes I really made...

@patzick
Copy link
Collaborator

patzick commented Oct 12, 2020

@niklas-wolf please change PR settings and allow edition from maintainers. I'll check out your code and see what's wrong with API contract.

@vercel vercel bot temporarily deployed to preview October 12, 2020 12:56 Inactive
@vercel vercel bot temporarily deployed to preview October 12, 2020 14:51 Inactive
@vercel vercel bot temporarily deployed to preview October 12, 2020 16:03 Inactive
@vercel vercel bot temporarily deployed to preview October 13, 2020 08:20 Inactive
Copy link
Collaborator

@patzick patzick left a comment

Choose a reason for hiding this comment

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

Made some small changes, it's ready for merge now. Thanks for your contribution! 💪

@patzick patzick merged commit 66661f7 into vuestorefront:master Oct 13, 2020
@niklaswolf niklaswolf deleted the 1154-promotion-code-functionality branch October 13, 2020 08:52
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.

[FEATURE] Implement promotion code functionality
3 participants