Support for Colorado Delivery Fee (flat fee and order-level taxes) #4491

adammathys · 2022-08-05T21:19:34Z

❗ TODO

Add radio buttons for TaxRate#level enum to admin
Tweak descriptions for TaxRate#rate and TaxRate#calculator in admin
Add specs for order-level taxes
Rake task to automatically generate Colorado Delivery Fee stuff

Summary

Add support for the new Colorado Delivery Fee. This is achieved by making two small changes to the existing tax system:

New Spree::Calculator::FlatFee calculator. Can be used to return a fixed amount that comes from an associated Spree::TaxRate.
Spree::TaxRate#level enum to distinguish between item- and order-level tax rates. Item-level taxes remain the default, since we very rarely if ever want to use order-level tax rates.

These changes in additional to the recent work to support order-level taxes in Spree::OrderTaxation (#4477), makes it possible to configure a tax rate for the Colorado Delivery Fee.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

I have written a thorough PR description.
I have kept my commits small and atomic.
I have used clear, explanatory commit messages.

The following are not always needed (~~cross them out~~ if they are not):

I have added automated tests to cover my changes.
~~I have attached screenshots to demo visual changes.~~
I have opened a PR to update the guides.
~~I have updated the readme to account for my changes.~~

waiting-for-dev · 2022-08-25T08:49:30Z

Hey @adammathys, it looks nice. How is it going? Do you need any help?

adammathys · 2022-08-26T22:27:02Z

@waiting-for-dev I haven't had much time to work on this as of late, but I should be able to wrap up the remaining TODO items next week.

waiting-for-dev · 2022-09-08T08:38:32Z

core/app/models/spree/calculator/flat_fee.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+require_dependency 'spree/calculator'


require_dependency has been deprecated. Anyway, why do we need it? 🤔

Oh, I see it's also used in the other calculators. I wonder if it can be removed safely. However, feel free to ignore the comment as it's outside this PR's scope.

waiting-for-dev

That's impressive work, @adammathys!

I've tried it locally with the data generated from the new task, and it looks like it applies the Colorado tax twice:

Do you have an idea what might be going on?

Also, do you think it'd be challenging to update the label rendered if somebody sets "show_rate_in_label" to true?

adammathys · 2022-09-08T14:52:04Z

@waiting-for-dev The double render is fixed by my two other PRs in the frontend repos. (solidusio/solidus_starter_frontend#242 and solidusio/solidus_frontend#11) It's a longstanding issue with the code where it'll double render any order-level tax adjustments. I believe it had gone unnoticed until now because we typically didn't have any.

I don't think it'd be too difficult to update the label if show_rate_in_label was set to true. We should be able to add some logic that checked the type of calculator since that would be the determining factor.

waiting-for-dev · 2022-09-09T07:42:45Z

The double render is fixed by my two other PRs in the frontend repos.

Oh, sorry, I missed that. Thanks for pointing this out.

I don't think it'd be too difficult to update the label if show_rate_in_label was set to true. We should be able to add some logic that checked the type of calculator since that would be the determining factor.

What do you think of doing it as part of this PR to make the work complete here? Otherwise, we can follow up.

BTW, it'd be nice to add information about this change in the CHANGELOG, including the available task.

spaghetticode

👏 great work Adam! I left a question about a possible small change.

backend/app/views/spree/admin/tax_rates/_form.html.erb

kennyadsl

Thanks for the excellent work @adammathys! I left a question but am super in favor of merging this.

core/app/models/spree/tax/tax_helpers.rb

kennyadsl

Thanks @adammathys! Approving, I'm fine even without that last comment, but not merging yet.

Very simple calculator that will always return the same amount. Will hopefully be useful for the Colorado Delivery Fee, but might also be usable for different fees that come up.

We unfortunately need to introduce the concept of order-level tax rates to handle the Colorado Delivery fee. (And maybe other similar things?) The simplest solution is to add an enum to our TaxRate model to help distinguish between rates that should be applied on an order-level and our regular item-level taxes. Co-Authored-By: Andrew Stewart <andrew@super.gd>

Updates TaxHelpers#rates_for_item to use our new enum to filter for rates that only apply to items. Co-Authored-By: Andrew Stewart <andrew@super.gd>

Looks for all order-level tax rates and computes order taxes based on the ones matching the order's ship address and tax categories.

Just a simple radio input to help set the value in the admin.

Adds the translation entry for the class and makes a small tweak to the tax rate calculator hint to explain the difference between the two calculators.

Now that this field can be used in two different ways, as a percentage or as a flat amount, we want to ensure the explanation is still helpful for users.

Simple task that can be used to help setup all of the records necessary for calculating the new Colorado Delivery Fee in stores.

Skips the logic for deciding if the "rate" should be shown in the label, since that never makes sense for flat fee tax rates.

codecov · 2022-09-14T21:05:52Z

Codecov Report

Merging #4491 (d762e2e) into master (8052691) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4491      +/-   ##
==========================================
+ Coverage   85.55%   85.58%   +0.02%     
==========================================
  Files         569      570       +1     
  Lines       14653    14675      +22     
==========================================
+ Hits        12537    12559      +22     
  Misses       2116     2116

Impacted Files	Coverage Δ
core/lib/spree/app_configuration.rb	`99.29% <ø> (ø)`
core/app/models/spree/calculator/flat_fee.rb	`100.00% <100.00%> (ø)`
core/app/models/spree/tax/tax_helpers.rb	`100.00% <100.00%> (ø)`
core/app/models/spree/tax_calculator/default.rb	`100.00% <100.00%> (ø)`
core/app/models/spree/tax_rate.rb	`100.00% <100.00%> (ø)`
...ee/testing_support/factories/calculator_factory.rb	`93.75% <100.00%> (+0.41%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

waiting-for-dev · 2022-09-16T03:32:50Z

Thanks, @adammathys, that's great. It would be great to update the Changelog if you have the time, even if in a separate PR. I think this change is worth it.

waiting-for-dev added type:enhancement Proposed or newly added feature changelog:solidus_core Changes to the solidus_core gem Important Change labels Aug 25, 2022

adammathys force-pushed the adammathys/flat-fee-taxes branch from 1c50314 to ac1faf2 Compare August 26, 2022 22:24

github-actions bot added the changelog:solidus_backend Changes to the solidus_backend gem label Sep 2, 2022

adammathys force-pushed the adammathys/flat-fee-taxes branch from 927b48c to f160208 Compare September 2, 2022 21:10

adammathys marked this pull request as ready for review September 2, 2022 21:35

adammathys force-pushed the adammathys/flat-fee-taxes branch 2 times, most recently from 055d51a to e9b55d9 Compare September 2, 2022 21:50

jarednorman approved these changes Sep 2, 2022

View reviewed changes

adammathys force-pushed the adammathys/flat-fee-taxes branch from e9b55d9 to 7d2fc19 Compare September 2, 2022 22:04

This was referenced Sep 2, 2022

Fix display of tax adjustments in checkout solidusio/solidus_starter_frontend#242

Merged

Fix display of tax adjustments in checkout solidusio/solidus_frontend#11

Merged

waiting-for-dev reviewed Sep 8, 2022

View reviewed changes

spaghetticode reviewed Sep 12, 2022

View reviewed changes

backend/app/views/spree/admin/tax_rates/_form.html.erb Outdated Show resolved Hide resolved

kennyadsl reviewed Sep 12, 2022

View reviewed changes

core/app/models/spree/tax/tax_helpers.rb Outdated Show resolved Hide resolved

kennyadsl approved these changes Sep 12, 2022

View reviewed changes

adammathys and others added 8 commits September 14, 2022 13:44

Add FlatFee tax calculator

988f4b8

Very simple calculator that will always return the same amount. Will hopefully be useful for the Colorado Delivery Fee, but might also be usable for different fees that come up.

Only apply item level taxes to items

01068e1

Updates TaxHelpers#rates_for_item to use our new enum to filter for rates that only apply to items. Co-Authored-By: Andrew Stewart <andrew@super.gd>

Calculate order-level taxes in default

a432b93

Looks for all order-level tax rates and computes order taxes based on the ones matching the order's ship address and tax categories.

Add radio for Spree::TaxRate#level

4625749

Just a simple radio input to help set the value in the admin.

Update translations for FlatFee tax calculator

65275eb

Adds the translation entry for the class and makes a small tweak to the tax rate calculator hint to explain the difference between the two calculators.

Update tax rate amount explanation

464a596

Now that this field can be used in two different ways, as a percentage or as a flat amount, we want to ensure the explanation is still helpful for users.

Add rake task for Colorado Delivery Fee

320d3ea

Simple task that can be used to help setup all of the records necessary for calculating the new Colorado Delivery Fee in stores.

Add explicit translation key for flat_fee taxes

d762e2e

Skips the logic for deciding if the "rate" should be shown in the label, since that never makes sense for flat fee tax rates.

adammathys force-pushed the adammathys/flat-fee-taxes branch from 7d2fc19 to d762e2e Compare September 14, 2022 20:44

kennyadsl merged commit 23930a3 into solidusio:master Sep 15, 2022

adammathys deleted the adammathys/flat-fee-taxes branch September 15, 2022 15:11

adammathys mentioned this pull request Sep 29, 2022

Add order-level tax changes to CHANGELOG.md #4653

Merged

gustavomrs mentioned this pull request Oct 3, 2022

[FM-1195] Support for Colorado fee by introducing order level tax and flat fee penrosehill/solidus#15

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for Colorado Delivery Fee (flat fee and order-level taxes) #4491

Support for Colorado Delivery Fee (flat fee and order-level taxes) #4491

adammathys commented Aug 5, 2022 •

edited

Loading

waiting-for-dev commented Aug 25, 2022

adammathys commented Aug 26, 2022

waiting-for-dev Sep 8, 2022

waiting-for-dev Sep 8, 2022

waiting-for-dev left a comment •

edited

Loading

adammathys commented Sep 8, 2022 •

edited

Loading

waiting-for-dev commented Sep 9, 2022 •

edited

Loading

spaghetticode left a comment

kennyadsl left a comment

kennyadsl left a comment •

edited

Loading

codecov bot commented Sep 14, 2022 •

edited

Loading

waiting-for-dev commented Sep 16, 2022

		@@ -0,0 +1,21 @@
		# frozen_string_literal: true

		require_dependency 'spree/calculator'

Support for Colorado Delivery Fee (flat fee and order-level taxes) #4491

Support for Colorado Delivery Fee (flat fee and order-level taxes) #4491

Conversation

adammathys commented Aug 5, 2022 • edited Loading

❗ TODO

Summary

Checklist

waiting-for-dev commented Aug 25, 2022

adammathys commented Aug 26, 2022

waiting-for-dev Sep 8, 2022

Choose a reason for hiding this comment

waiting-for-dev Sep 8, 2022

Choose a reason for hiding this comment

waiting-for-dev left a comment • edited Loading

Choose a reason for hiding this comment

adammathys commented Sep 8, 2022 • edited Loading

waiting-for-dev commented Sep 9, 2022 • edited Loading

spaghetticode left a comment

Choose a reason for hiding this comment

kennyadsl left a comment

Choose a reason for hiding this comment

kennyadsl left a comment • edited Loading

Choose a reason for hiding this comment

codecov bot commented Sep 14, 2022 • edited Loading

Codecov Report

waiting-for-dev commented Sep 16, 2022

adammathys commented Aug 5, 2022 •

edited

Loading

waiting-for-dev left a comment •

edited

Loading

adammathys commented Sep 8, 2022 •

edited

Loading

waiting-for-dev commented Sep 9, 2022 •

edited

Loading

kennyadsl left a comment •

edited

Loading

codecov bot commented Sep 14, 2022 •

edited

Loading