✨ Marketplace: Create and Update TaxRate
#1186
Conversation
zspencer
force-pushed
the
marketplace/creating-tax-rates
branch
from
da2d35d to
71b7a3a
Compare
| <h3>Tax Rates</h3> | ||
| <%= render policy_scope(marketplace.tax_rates) %> | ||
|
|
||
|
|
There was a problem hiding this comment.
Extra blank line. </high value comment>
There was a problem hiding this comment.
Allegedly, prettier has an erb linter... allegedly
| class Marketplace | ||
| class TaxRate < Record | ||
| self.table_name = "marketplace_tax_rates" | ||
| belongs_to :marketplace |
There was a problem hiding this comment.
Nit: I think it might be a bit more scannable to have the self.... lines grouped, and the belongs_to in a separate "block" (i.e. separated by a blank line). (Unless they have to be declared in this order?)
app/furniture/marketplace/routes.rb
Outdated
| router.resources :orders, only: [:show] | ||
| router.resources :products | ||
| router.resources :products do | ||
| router.resources :tax_rates, only: [:create, :destroy, :index, :show] |
There was a problem hiding this comment.
I only see code to edit and display the Marketplace Tax Rates, but there is this products/tax_rates set of routes, and also a reference from the marketplace products table to tax rates, although the TaxRate model merely belongs_to :marketplace -- maybe you are missing the has_many :marketplace_products on TaxRate and the belongs_to :tax_rate, optional: true on MarketplaceProduct?
I'm not sure if the Tax Rates on Products are left over from a previous version, or if you intend to use those somehow differently?
There was a problem hiding this comment.
I should get rid of this since I haven't started the associating TaxRates to Products yet.
| class CreateMarketplaceTaxRates < ActiveRecord::Migration[7.0] | ||
| def change | ||
| create_table :marketplace_tax_rates, id: :uuid do |t| | ||
| t.float :tax_rate |
There was a problem hiding this comment.
Float feels correct to me, since I agree that it could be a percent, or a fractional amount, or an integer fixed amount. (Maybe eventually we'll end up with an enum here to say if it is %age or "fixed".)
- #1137 This throws together the basic `CRUD` operations for `TaxRate`. Going to do a bit of tidying and add some tests, then record a video of the use case. I may decide I want to add in the setting of the `TaxRate` on the `Product`, but maybe I'll wait and add that independently...
I think mayyybeeeeeeee it's better to support decimals.
zspencer
force-pushed
the
marketplace/creating-tax-rates
branch
from
2e96cb9 to
da72da4
Compare
This also addresses Ana's comments re: Tidying
Marketplace:TaxCollection and Compliance #1137Slaps some
updateandcreateactions together for settingTaxRates.That said, I don't know that an integer or a float is better, since I suppose there could be a partial tax?
Also, I tried to set it up so tax rates applied across all spaces; but then I ran into some weirdness... if y'all have ideas happy to hear.