-
Notifications
You must be signed in to change notification settings - Fork 103
feat(default-theme): promotion code functionality #1155
feat(default-theme): promotion code functionality #1155
Conversation
Update Master
Update Master
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/shopware-pwa/shopware-pwa-docs/7m8icg3a6 |
in regards to point 2. from help wanted: 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
@mkucmus thanks, I added toast notifications and resolved the conflicts. :) |
here is a design from storefront-ui figma: |
@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. |
There was a problem hiding this 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, |
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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"
There was a problem hiding this comment.
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.
// 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()); | ||
} |
There was a problem hiding this comment.
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>
@patzick I think I have tackled all your notes. |
@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. |
There was a problem hiding this 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! 💪
Changes
closes #1154
Help wanted
Testing
To test this, a promotion code has to be added to the backend.
Checklist