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

feat: order recurrence (subscription support) #1690

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

feat: order recurrence (subscription support) #1690

wants to merge 7 commits into from

Conversation

shauke
Copy link
Collaborator

@shauke shauke commented Jun 24, 2024 •
edited
Loading

PR Type

[x] Feature

What Is the Current Behavior?

The PWA does not support recurring orders/subscriptions.

What Is the New Behavior?

The PWA supports recurring orders/subscriptions.

Does this PR Introduce a Breaking Change?

[x] No

TODOs

see IAD https://www.figma.com/design/dyViJpIjdQbAHbdhr8z6iw/Recurring-orders?node-id=3-503&t=bTzn9muQiMK1AiKi-0

  • styling for the order recurrence form
  • error handling and validation
  • specific form field to implement the "until after 50 orders" form group
  • localization (especially the german and french translations)
  • the complete recurring orders/requisitions support in my account is missing

Other Information

AB#97622

@shauke shauke self-assigned this Jun 24, 2024
@shauke shauke force-pushed the feature/recurring_orders branch 2 times, most recently from 48251ed to 8404556 Compare June 24, 2024 20:41
@shauke shauke force-pushed the feature/recurring_orders branch from 8404556 to 17e950c Compare August 13, 2024 08:58
@shauke shauke added this to the 5.3 milestone Aug 14, 2024
@shauke shauke force-pushed the feature/recurring_orders branch from 1d1fc88 to 4be7a4a Compare August 20, 2024 12:33
@shauke shauke force-pushed the feature/recurring_orders branch from 3358318 to d4125b1 Compare August 23, 2024 16:16
@suschneider suschneider force-pushed the feature/recurring_orders branch from 7d8e9c0 to cec66f0 Compare August 29, 2024 09:48
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 3, 2024

Azure Demo Servers are available:

@shauke shauke force-pushed the feature/recurring_orders branch from 5b90b6f to 9ee5d14 Compare September 19, 2024 12:25
@github-actions GitHub Actions
Copy link

Azure Demo Servers are available:

@M-Behr
Copy link

M-Behr commented Sep 23, 2024

  • remove desired delivery date or show error message - doesn't make sense for recurring orders, does it?
    image
  • adapt text in approval details modal in checkout - review step
  • last 5 orders are missing in recurring order details
  • padding bottom is missing in infobox in mobile view
    image

@suschneider
Copy link
Collaborator

  • remove desired delivery date - done
  • adapt text in approval details modal in checkout - review step
    In our opinion we adapted the modal as defined in the IAD

image

  • last 5 orders are missing in recurring order details - The REST API cannot provide this at the moment. But everything is prepared for this so that the last 5 orders are displayed when the REST API delivers them.
  • padding bottom is missing in infobox in mobile view - done

@mglatter mglatter requested review from mglatter and removed request for mglatter September 24, 2024 08:36
mglatter
mglatter reviewed Sep 24, 2024
View reviewed changes
src/assets/i18n/en_US.json Outdated
"account.recurring_order.details.inactive-by-system.message": "The last planned order could not be placed and the recurring order has been deactivated.",
"account.recurring_order.details.inactive.message": "Your recurring order is currently inactive. Activate it now to continue receiving regular deliveries and exclusive discounts.",
"account.recurring_order.details.inactive.text": "Inactive",
"account.recurring_order.details.info_message": "Please note: Orders are created even if prices or the availability of subscribed items have changed. See the order confirmation for details.",
Copy link
Contributor

Choose a reason for hiding this comment

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

As we want to get rid of the word "subscription": I'd suggest changing "of subscribed items" to "of included items" here.

mglatter
mglatter reviewed Sep 24, 2024
View reviewed changes
src/assets/i18n/en_US.json Outdated
"account.recurring_order.details.inactive.message": "Your recurring order is currently inactive. Activate it now to continue receiving regular deliveries and exclusive discounts.",
"account.recurring_order.details.inactive.text": "Inactive",
"account.recurring_order.details.info_message": "Please note: Orders are created even if prices or the availability of subscribed items have changed. See the order confirmation for details.",
"account.recurring_order.details.last_order_date.label": "Last order date",
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be only "Last order"?

mglatter
mglatter reviewed Sep 24, 2024
View reviewed changes
src/assets/i18n/en_US.json Outdated
"account.recurring_order.details.last_order_date.label": "Last order date",
"account.recurring_order.details.last_placed_orders.label": "Last five orders",
"account.recurring_order.details.links.return_to_orders": "Back to recurring orders",
"account.recurring_order.details.next_order_date.label": "Next order date",
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be only "Next order"?

@github-actions GitHub Actions
Copy link

Azure Demo Servers are available:

@mglatter
Copy link
Contributor

I will ask Ami to double-check French localization. Please do not consider localizations done before approval.

shauke and others added 3 commits September 26, 2024 20:13
@shauke
feat: introduce Formly information field for rendering any freestyle …
cbc0834 
…text within generated forms
@suschneider @shauke
feat: introduce Switch component (UI element for imidiate status chan…
54b2fe5 
…ges)

* implemented as standalone component
@shauke
feat: introduce Server Setting route guard
0ad698b 
* checks for an enabled server setting before routing
@shauke shauke force-pushed the feature/recurring_orders branch from 6745253 to b82b990 Compare September 26, 2024 18:45
@shauke shauke marked this pull request as ready for review September 27, 2024 07:59
@shauke shauke changed the title feat: Order recurrence (subscription support) feat: order recurrence (subscription support) Sep 27, 2024
@shauke shauke added the feature New feature or request label Sep 27, 2024
shauke and others added 2 commits September 27, 2024 10:05
@shauke @suschneider
feat: order recurrence (subscription support)
994bf9f 
* create recurring order during checkout
* handle recurring orders in requisition/approval listings
* recurring orders listing in my account
* recurring order details

Co-authored-by: Susanne Schneider <s.schneider@intershop.de>
@shauke @suschneider
temp: deploy with develop.icm that already has the recurring order API
6948724
@github-actions GitHub Actions
Copy link

Azure Demo Servers are available:

@suschneider suschneider force-pushed the feature/recurring_orders branch from b82b990 to 6948724 Compare September 27, 2024 08:08
@mglatter
Copy link
Contributor

mglatter commented Oct 1, 2024

It turns out that the wording "taxation ID" is not ideal as it doesn't clearly refer to the intended number:
EN: VAT registration number (or: sales tax identification number)
DE: Umsatzsteueridentifikationsnummer (or: USt-ID)
FR: Numéro d’identification TVA

We could adjust it for PWA in the course of this pull request, but it may probably be better to do it in a separate branch, as we would not only touch your new localization keys by that change but all occurrences in the localization file.

@M-Behr
Copy link

M-Behr commented Oct 2, 2024

It turns out that the wording "taxation ID" is not ideal as it doesn't clearly refer to the intended number: EN: VAT registration number (or: sales tax identification number) DE: Umsatzsteueridentifikationsnummer (or: USt-ID) FR: Numéro d’identification TVA

We could adjust it for PWA in the course of this pull request, but it may probably be better to do it in a separate branch, as we would not only touch your new localization keys by that change but all occurrences in the localization file.

@mglatter Please make sure that the changes are also made in the ICM and CEC. Thanks!

@mglatter
Copy link
Contributor

mglatter commented Oct 2, 2024 •
edited
Loading

It turns out that the wording "taxation ID" is not ideal as it doesn't clearly refer to the intended number: EN: VAT registration number (or: sales tax identification number) DE: Umsatzsteueridentifikationsnummer (or: USt-ID) FR: Numéro d’identification TVA
We could adjust it for PWA in the course of this pull request, but it may probably be better to do it in a separate branch, as we would not only touch your new localization keys by that change but all occurrences in the localization file.

@mglatter Please make sure that the changes are also made in the ICM and CEC. Thanks!

We will handle this in a separate ticket covering all these.

mglatter
mglatter reviewed Oct 2, 2024
View reviewed changes
Copy link
Contributor

@mglatter mglatter left a comment

Choose a reason for hiding this comment

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

Please see my comments at formly.md
Localization can be considered done and approved.

src/assets/i18n/de_DE.json Outdated
"account.recurring_order.details.shipping_method.heading": "Shipping method",
"account.recurring_order.details.status.label": "Status",
"account.recurring_order.details.taxationId.label": "Taxation ID:",
"account.recurring_order.heading": "Recurring Order Details",
Copy link
Contributor

Choose a reason for hiding this comment

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

Sentence case: Recurring order details

Copy link
Contributor

Choose a reason for hiding this comment

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

Adjusted in EN where this remark should have been written.

src/assets/i18n/de_DE.json Outdated
"account.recurring_order.details.taxationId.label": "Taxation ID:",
"account.recurring_order.heading": "Recurring Order Details",
"account.recurring_order.subtitle": "Below are details about the item(s) in your recurring order. If you ordered more than one item, please note that some items may display a different shipping method and/or status because they are shipped in a separate package.",
"account.recurring_orders.breadcrumb": "Recurring Order",
Copy link
Contributor

Choose a reason for hiding this comment

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

Sentence case: Recurring order

Copy link
Contributor

Choose a reason for hiding this comment

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

Adjusted in EN where this remark should have been written.

src/assets/i18n/de_DE.json Outdated
"account.recurring_order.details.status.label": "Status",
"account.recurring_order.details.taxationId.label": "Taxation ID:",
"account.recurring_order.heading": "Recurring Order Details",
"account.recurring_order.subtitle": "Below are details about the item(s) in your recurring order. If you ordered more than one item, please note that some items may display a different shipping method and/or status because they are shipped in a separate package.",
Copy link
Contributor

Choose a reason for hiding this comment

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

"Please find details about the item(s) in your recurring order below. If you ordered more than one item, please note that some items may display a different shipping method and/or status because they are shipped in a separate package."

Copy link
Contributor

Choose a reason for hiding this comment

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

Adjusted in EN where this remark should have been written.

src/assets/i18n/de_DE.json Outdated
"account.recurring_orders.link.title.remove": "Delete recurring order \"{{0}}\"",
"account.recurring_orders.navigation.link": "Recurring orders",
"account.recurring_orders.no_placed_orders_message": "You have not placed a recurring order yet.",
"account.recurring_orders.subtitle": "Your most recent recurring order appears first. Please allow up to 5 minutes for new orders to appear below.",
Copy link
Contributor

Choose a reason for hiding this comment

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

"Your most recent recurring order appears first. It may take up to 5 minutes for new orders to appear."

Copy link
Contributor

Choose a reason for hiding this comment

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

Adjusted in EN where this remark should have been written.

src/assets/i18n/de_DE.json Outdated
"account.recurring_order.details.inactive.text": "Inactive",
"account.recurring_order.details.info_message": "Please note: Orders are created even if prices or the availability of subscribed items have changed. See the order confirmation for details.",
"account.recurring_order.details.last_order_date.label": "Last order date",
"account.recurring_order.details.last_placed_orders.label": "Last placed orders",
Copy link
Contributor

Choose a reason for hiding this comment

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

"Last 5 orders"

Copy link
Contributor

Choose a reason for hiding this comment

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

Adjusted in EN where this remark should have been written.

src/assets/i18n/en_US.json Show resolved Hide resolved
docs/guides/formly.md Outdated
| ish-html-text-field | Only display the form value as html | ---- |
| ish-date-picker-field | Basic datepicker | `minDays`: Computes the minDate by adding the minimum allowed days to today. `maxDays`: Computes the maxDate by adding the maximum allowed days to today. `isSatExcluded`: Specifies if saturdays can be disabled. `isSunExcluded`: Specifies if sundays can be disabled. |
| ish-date-range-picker-field | Datepicker with range | `minDays`: Computes the minDate by adding the minimum allowed days to today. `maxDays`: Computes the maxDate by adding the maximum allowed days to today. `startDate`: The start date. `placeholder`: Placeholder that displays the date format in the input field. |
| Name | Description | Relevant props |
Copy link
Contributor

Choose a reason for hiding this comment

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

Relevant props → Better to write "Relevant properties" because props has many different meanings.

docs/guides/formly.md
| ish-date-range-picker-field | Datepicker with range | `minDays`: Computes the minDate by adding the minimum allowed days to today. `maxDays`: Computes the maxDate by adding the maximum allowed days to today. `startDate`: The start date. `placeholder`: Placeholder that displays the date format in the input field. |
| Name | Description | Relevant properties |
| --------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| ish-text-input-field | Basic input field, supports all text types | `type`: 'text (default),'email','tel','password'. `mask`: input mask for a needed pattern (see [ngx-mask](https://www.npmjs.com/package/ngx-mask) for more information) |
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that there is an apostrophe missing here: 'text (default)
Should it be added after "text" or after "(default)"?

docs/guides/formly.md
| ish-date-range-picker-field | Datepicker with range | `minDays`: Computes the minDate by adding the minimum allowed days to today. `maxDays`: Computes the maxDate by adding the maximum allowed days to today. `startDate`: The start date. `placeholder`: Placeholder that displays the date format in the input field. |
| Name | Description | Relevant properties |
| --------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| ish-text-input-field | Basic input field, supports all text types | `type`: 'text (default),'email','tel','password'. `mask`: input mask for a needed pattern (see [ngx-mask](https://www.npmjs.com/package/ngx-mask) for more information) |
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the 'email' property (in 3rd column) be spelled 'e-mail', or is it written in the code like this and spelling needs to remain as is?

@MarioVesper MarioVesper linked an issue Oct 22, 2024 that may be closed by this pull request
Order subscription missing #692
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mglatter mglatter mglatter left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@shauke shauke

Labels
feature New feature or request
Projects
None yet
Milestone
5.3
Development

Successfully merging this pull request may close these issues.

Order subscription missing
4 participants
@shauke @M-Behr @suschneider @mglatter