[Vouchers] Flat rate of any amount #10761

rioug · 2023-04-28T03:24:50Z

What? Why?

First part of [Vouchers] Include unfixed amounts and percentage rates #10727

Add the ability to have a flat rate voucher of any amount

What should we test?

Enable vouchers feature
Visit admin/feature-toggle/features and enable or add vouchers feature

In the backoffice logged as an enterprise user:

Visit the Enterprise tab and click on "Settings" for a shop of your choosing, and click on the "vouchers" in left hand side menu
Click on "Add New button" --> New Voucher page should now have an "Amount" input
Enter a voucher code and an amount and click "Save"
--> It should redirect you to the vouchers page on the enterprise edit page
--> You should now see a list of vouchers with the one you just created, included the entered amount

On the website logged as a customer :

Add one or more product in your cart from the shop above
Proceed to checkout
Enter details and shipping info, click on "Next - Payment method"
Enter the voucher code and click "Apply"
--> You should see a "$xx Voucher" box where xx is the amount you entered when creating a voucher
choose a payment method and click on "Next - Order summary"
--> you should see the voucher applied to the order in the order summary column, with the correct amount
Click on "Complete order"
--> you should see the voucher applied to the order in the order summary with the correct amount

Release notes

Changelog Category: User facing changes

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

app/models/spree/adjustment.rb

mkllnk

Nice work!

mkllnk · 2023-05-01T23:39:10Z

db/migrate/20230418030646_add_amount_to_vouchers.rb

@@ -0,0 +1,5 @@
+class AddAmountToVouchers < ActiveRecord::Migration[7.0]
+  def change
+    add_column :vouchers, :amount, :decimal, precision: 10, scale: 2, null: false, default: 0


Hm, the default is 0 but the Rails validation requires it to be greater than that. It's contradictory but I don't have any better ideas either. 🤷

Is a default necessary? If an amount isn't provided, then we want an error to be thrown don't we?

I think you are right, my thinking was a voucher with an amount 0 is probably not a big problem, the worse than can happen is you apply a voucher that won't change the order amount..

db/schema.rb

app/controllers/admin/vouchers_controller.rb

app/views/admin/vouchers/new.html.haml

app/views/split_checkout/_voucher_section.cable_ready.haml

spec/factories/voucher_factory.rb

rioug · 2023-05-05T11:54:38Z

New commit start here : 2da76b2

mkllnk

Great. Thank you for addressing it all.

I noticed that I get weird results in db/schema.rb on master as well. I think that something must have gotten wrong when merging. I'll open a PR to correct master.

dacook

I think this is fine as-is, but have a few comments to potentially improve.

dacook · 2023-05-10T02:14:47Z

app/views/split_checkout/_voucher_section.cable_ready.haml

            .checkout-input
              %span.formError.standalone
                = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount")
        - else
          = text_field_tag "[order][voucher_code]", params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged", }, placeholder: t("split_checkout.step2.voucher.placeholder") , class: "voucher"
-          = submit_tag t("split_checkout.step2.voucher.apply"), name: "apply_voucher", disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" }
+          = submit_tag t("split_checkout.step2.voucher.apply"), name: "apply_voucher", disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" }  


Oops, a couple of spaces were added at end of line here.

dacook · 2023-05-10T02:16:55Z

db/migrate/20230418030646_add_amount_to_vouchers.rb

@@ -0,0 +1,5 @@
+class AddAmountToVouchers < ActiveRecord::Migration[7.0]
+  def change
+    add_column :vouchers, :amount, :decimal, precision: 10, scale: 2, null: false, default: 0


Is a default necessary? If an amount isn't provided, then we want an error to be thrown don't we?

app/controllers/admin/vouchers_controller.rb

spec/system/admin/vouchers_spec.rb

rioug · 2023-05-12T06:45:12Z

This will need to be rebased on https://github.com/rioug/openfoodnetwork/tree/10432-vouchers-bare-minimum-checkout (or master if 10432-vouchers-bare-minimum-checkout has been merged)

Change value to amount to be more consistent

As vouchers are getting more complicated, it makes sense to use a factory to simplify writing test.

It should have already been the case...

Speed up testing by removing unnecessarily created record

dacook

I think we should still change the random value in the factory.

spec/factories/voucher_factory.rb

mkllnk · 2023-05-19T01:37:18Z

Yes, I would be happy with a fixed value.

drummer83 · 2023-05-19T15:39:08Z

Hi @rioug and @mkllnk,
So exciting to play with this new feature! Since it was my first time I have also noted some things which we may want to look into at some point, but they are out of scope of this PR.

General remark after staging

Previously generated vouchers are set to Amount = 0 (instead of 10 $ when created). I guess we can live with this, since it is not in production yet.

Positive test cases

only numbers > 0 are accepted ✔️
the new voucher is added to the vouchers list ✔️
checkout using the voucher works as expected ✔️
order confirmation page shows the voucher ✔️
confirmation email shows the voucher ✔️
vouchers can be applied unlimited times ✔️
vouchers can be applied by any customer ✔️
vouchers are not displayed when checking out at an enterprise which has no voucher codes ✔️

Proposed adjustments of backoffice design

Main page
- Shouldn't be available for hub profile and shop profile enterprises (sells = none) ➡️ [Vouchers] Vouchers page is shown for profile enterprises #10867
- 'Add new' button is currently above the table --> Comparing to Payment/Shipping methods: Name it 'Create new voucher +' and place it below the table ➡️ [Vouchers] UX and design improvements of page 'Vouchers' #10871
Create new voucher page
- Use header design from Payment/Shipping methods: 'New Voucher' & button 'Back to voucher list' ➡️ [Vouchers] UX and design improvements of page 'New voucher' #10870
- Include the 'Cancel' and 'Create' buttons in the footer of the page (see Payment/Shipping methods) ➡️ [Vouchers] UX and design improvements of page 'New voucher' #10870
- Is a multi-line text field required for the voucher code? --> It's not possible to use a voucher with two or more lines at checkout (Error: voucher not found). ➡️ [Vouchers] Multi-line voucher code can be created but can't be applied #10868
Default value should be 0.00 (instead of 0.0 which is untypical for money amounts). ➡️ [Vouchers] UX and design improvements of page 'New voucher' #10870
Entering a comma as decimal separator shows flash error - we have just recently decided to go for more browser validation
Entering letters shows the flash error - we have just recently decided to go for more browser validation

Proposed adjustments for backoffice functionality

Edit vouchers
Delete vouchers

Proposed adjustments for checkout

Irritating error message when voucher can't be found and no red highlighting of the voucher field. ➡️ [Vouchers] No red highlighting of voucher code when code can't be found #10869

Negative test cases

if after applying the voucher you go back to edit your basket and the order total is 0 $ after updating the cart, you then click checkout, enter your details and then click 'next - payment details', then you are redirected to the main page, but the order is placed (email confirmation is sent) (:arrow_right: [Vouchers] Redirection to HOME after updating the cart including a voucher #10865):

Peek.2023-05-19.15-48.mp4

the voucher is displayed on the /cart page after applying it on step 2 of checkout. However it's value is not being calculated into the order total (:arrow_right: [Vouchers] Order total in /cart page not updated #10855):

Peek.2023-05-19.15-42.mp4

if a voucher code is entered but not yet applied and no payment method is selected when clicking 'next', then the 'apply' button remains disabled. You have to change the value in the voucher code field before it can be applied ([Vouchers] Disabled 'Apply' button is blocking the voucher usage #10866):

Peek.2023-05-19.16-40.mp4

Result

The requested feature has been implemented according to specification. We can merge this one here. I will open issues for the negative test cases and some of the improvements.

Thanks! 🙏

mkllnk · 2023-05-22T00:56:59Z

Thank you, excellent testing. Definitely some edge cases to work out before it can be released.

rioug self-assigned this Apr 28, 2023

rioug force-pushed the vouchers-any-amount branch 2 times, most recently from e6b77fb to b8d25de Compare April 28, 2023 05:02

rioug changed the title ~~Vouchers any amount~~ Vouchers, flat rate of any amount Apr 28, 2023

rioug marked this pull request as ready for review April 28, 2023 05:03

Matt-Yorkley reviewed May 1, 2023

View reviewed changes

app/models/spree/adjustment.rb Outdated Show resolved Hide resolved

mkllnk requested changes May 2, 2023

View reviewed changes

mkllnk mentioned this pull request May 2, 2023

Require belongs_to associations by default #10778

Closed

rioug force-pushed the vouchers-any-amount branch 2 times, most recently from 4e0e0c6 to 08927e6 Compare May 5, 2023 06:35

rioug requested a review from mkllnk May 5, 2023 11:53

rioug force-pushed the vouchers-any-amount branch from 08927e6 to 74fee33 Compare May 7, 2023 23:37

mkllnk approved these changes May 8, 2023

View reviewed changes

mkllnk mentioned this pull request May 8, 2023

Fixup order of statements in db schema #10818

Merged

lin-d-hop added the priority We focus on this issue right now label May 9, 2023

dacook reviewed May 10, 2023

View reviewed changes

mkllnk assigned mkllnk and unassigned rioug May 16, 2023

rioug added 6 commits May 16, 2023 16:19

Add amount to voucher

15eee81

Change value to amount to be more consistent

Update admin pages to allow entering an amount

b427e42

Add a voucher factory

2d8fa24

As vouchers are getting more complicated, it makes sense to use a factory to simplify writing test.

Make enterprise mandatory for a voucher

b585f77

It should have already been the case...

Per review comment, build enterprise instead of create

355af9e

Speed up testing by removing unnecessarily created record

Admin::VoucherController check enterprise permission

abf2105

mkllnk force-pushed the vouchers-any-amount branch from 74fee33 to abf2105 Compare May 16, 2023 06:20

mkllnk approved these changes May 16, 2023

View reviewed changes

sigmundpetersen changed the title ~~Vouchers, flat rate of any amount~~ [Vouchers] Flat rate of any amount May 16, 2023

dacook requested changes May 17, 2023

View reviewed changes

spec/factories/voucher_factory.rb Outdated Show resolved Hide resolved

This comment was marked as duplicate.

Sign in to view

dacook self-assigned this May 19, 2023

Use a fixed value for voucher factory

9d7316c

dacook approved these changes May 19, 2023

View reviewed changes

drummer83 self-assigned this May 19, 2023

drummer83 added the pr-staged-au staging.openfoodnetwork.org.au label May 19, 2023

drummer83 merged commit fed7c3d into openfoodfoundation:master May 19, 2023
50 checks passed

drummer83 removed the pr-staged-au staging.openfoodnetwork.org.au label May 19, 2023

This was referenced May 19, 2023

[Vouchers] Percentage rate #10821

Merged

[Vouchers] UX and design improvements of page 'New voucher' #10870

Closed

[Vouchers] UX and design improvements of page 'Vouchers' #10871

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Vouchers] Flat rate of any amount #10761

[Vouchers] Flat rate of any amount #10761

rioug commented Apr 28, 2023 •

edited by mkllnk

Loading

mkllnk left a comment

mkllnk May 1, 2023

dacook May 10, 2023

rioug May 12, 2023

rioug commented May 5, 2023

mkllnk left a comment

dacook left a comment

dacook May 10, 2023

dacook May 10, 2023

rioug commented May 12, 2023

dacook left a comment

This comment was marked as duplicate.

This comment was marked as duplicate.

mkllnk commented May 19, 2023

drummer83 commented May 19, 2023 •

edited

Loading

mkllnk commented May 22, 2023

[Vouchers] Flat rate of any amount #10761

[Vouchers] Flat rate of any amount #10761

Conversation

rioug commented Apr 28, 2023 • edited by mkllnk Loading

What? Why?

What should we test?

Release notes

Dependencies

Documentation updates

mkllnk left a comment

Choose a reason for hiding this comment

mkllnk May 1, 2023

Choose a reason for hiding this comment

dacook May 10, 2023

Choose a reason for hiding this comment

rioug May 12, 2023

Choose a reason for hiding this comment

rioug commented May 5, 2023

mkllnk left a comment

Choose a reason for hiding this comment

dacook left a comment

Choose a reason for hiding this comment

dacook May 10, 2023

Choose a reason for hiding this comment

dacook May 10, 2023

Choose a reason for hiding this comment

rioug commented May 12, 2023

dacook left a comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

This comment was marked as duplicate.

mkllnk commented May 19, 2023

drummer83 commented May 19, 2023 • edited Loading

General remark after staging

Positive test cases

Proposed adjustments of backoffice design

Proposed adjustments for backoffice functionality

Proposed adjustments for checkout

Negative test cases

Result

mkllnk commented May 22, 2023

rioug commented Apr 28, 2023 •

edited by mkllnk

Loading

drummer83 commented May 19, 2023 •

edited

Loading