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

Added the :selected option with the default tax category #12989 #13009

Merged
merged 6 commits into from
Jan 7, 2025
Merged

Added the :selected option with the default tax category #12989 #13009

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7d5bb4a
Added the :selected option with the default tax category
MrBowmanXD Dec 1, 2024
d95bb77
Fixed the logic and moved the code to a helper file
MrBowmanXD Dec 1, 2024
980f86c
Fixed the lint errors
MrBowmanXD Dec 2, 2024
3199118
Removed the :selected attribute when the tax category is not required…
MrBowmanXD Dec 3, 2024
5def2b5
Added a spec file in order to test the tax categories helper
MrBowmanXD Dec 8, 2024
0d4904d
Small change in spec file
MrBowmanXD Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/controllers/spree/admin/products_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class ProductsController < ::Admin::ResourceController
include OrderCyclesHelper
include EnterprisesHelper
helper ::Admin::ProductsHelper
helper Spree::Admin::TaxCategoriesHelper

before_action :load_data
before_action :load_producers, only: [:index, :new]
Expand Down
8 changes: 8 additions & 0 deletions app/helpers/spree/admin/tax_categories_helper.rb
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's a good practise to add unit test for your code, even if the logic seems trivial. Could you add some test for the new helper ? Have a look here for some inspiration https://github.com/openfoodfoundation/openfoodnetwork/blob/master/spec/helpers/admin/products_helper_spec.rb

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can add some tests, sorry if i did not start with a test, i am still learning Rails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to ask if you guys know any good resource for rspec in rails? i will search for it as well, thanks in advance

Copy link
Collaborator

Choose a reason for hiding this comment

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

No worries 🙂
I find this playlist helpful, maybe it would help:
https://www.youtube.com/watch?v=2jX-FLcznDE&list=PLS6F722u-R6Ik3fbeLXbSclWkT6Qsp9ng

Please let us know you face any issues, no worries ⭐

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello, thank you. I will check it out.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module Spree::Admin::TaxCategoriesHelper

Check warning on line 1 in app/helpers/spree/admin/tax_categories_helper.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop 

[rubocop] reported by reviewdog 🐶
Missing frozen string literal comment.

Raw Output:
app/helpers/spree/admin/tax_categories_helper.rb:1:1: C: Style/FrozenStringLiteralComment: Missing frozen string literal comment.

Check warning on line 1 in app/helpers/spree/admin/tax_categories_helper.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop 

[rubocop] reported by reviewdog 🐶
Use nested module/class definitions instead of compact style.

Raw Output:
app/helpers/spree/admin/tax_categories_helper.rb:1:8: C: Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.
def tax_category_dropdown_options(require_tax_category)

Check warning on line 2 in app/helpers/spree/admin/tax_categories_helper.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop 

[rubocop] reported by reviewdog 🐶
Unused method argument - `require_tax_category`. If it's necessary, use `_` or `_require_tax_category` as an argument name to indicate that it won't be used. If it's unnecessary, remove it. You can also write as `tax_category_dropdown_options(*)` if you want the method to accept any arguments but don't care about them.

Raw Output:
app/helpers/spree/admin/tax_categories_helper.rb:2:37: W: Lint/UnusedMethodArgument: Unused method argument - `require_tax_category`. If it's necessary, use `_` or `_require_tax_category` as an argument name to indicate that it won't be used. If it's unnecessary, remove it. You can also write as `tax_category_dropdown_options(*)` if you want the method to accept any arguments but don't care about them.
{
:include_blank => Spree::Config.products_require_tax_category ? false : t(:none),

Check warning on line 4 in app/helpers/spree/admin/tax_categories_helper.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop 

[rubocop] reported by reviewdog 🐶
Use the new Ruby 1.9 hash syntax.

Raw Output:
app/helpers/spree/admin/tax_categories_helper.rb:4:7: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax.

Check warning on line 4 in app/helpers/spree/admin/tax_categories_helper.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop 

[rubocop] reported by reviewdog 🐶
Trailing whitespace detected.

Raw Output:
app/helpers/spree/admin/tax_categories_helper.rb:4:88: C: Layout/TrailingWhitespace: Trailing whitespace detected.
selected: Spree::Config.products_require_tax_category ? Spree::TaxCategory.find_by(is_default: true)&.id : nil

Check warning on line 5 in app/helpers/spree/admin/tax_categories_helper.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop 

[rubocop] reported by reviewdog 🐶
Line is too long. [116/100]

Raw Output:
app/helpers/spree/admin/tax_categories_helper.rb:5:101: C: Layout/LineLength: Line is too long. [116/100]
}
end
end

Check warning on line 8 in app/helpers/spree/admin/tax_categories_helper.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop 

[rubocop] reported by reviewdog 🐶
Final newline missing.

Raw Output:
app/helpers/spree/admin/tax_categories_helper.rb:8:4: C: Layout/TrailingEmptyLines: Final newline missing.
2 changes: 1 addition & 1 deletion app/views/spree/admin/products/_tax_category_form.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
= f.field_container :tax_category_id do
= f.label :tax_category_id, t(:tax_category)
%br
= f.collection_select(:tax_category_id, Spree::TaxCategory.all, :id, :name, {:include_blank => Spree::Config.products_require_tax_category ? false : t(:none)}, {:class => "select2 fullwidth"})
= f.collection_select(:tax_category_id, Spree::TaxCategory.all, :id, :name, tax_category_dropdown_options(Spree::Config.products_require_tax_category), {:class => "select2 fullwidth"})
= f.error_message_on :tax_category_id
Loading