Fixes filtering of active ordergroups #934
Conversation
spec/models/ordergroup_spec.rb
Outdated
| @@ -1,5 +1,21 @@ | |||
| require './db/seeds/seed_helper' | |||
There was a problem hiding this comment.
i don't think that testing should depend on seeding helpers. i'd suggest to create helpers for that in the spec directory, if you think code can be shared with other tests
There was a problem hiding this comment.
Yes, I totally agree. Luckily, it's not required anymore. Thanks for the hint.
spec/models/ordergroup_spec.rb
Outdated
| def create_order(starts) | ||
| og = user.ordergroup | ||
|
|
||
| SupplierCategory.create!(:id => 1, :name => "Other", :financial_transaction_class_id => ftc1.id) |
There was a problem hiding this comment.
please use the <key>: <value> syntax, like you did in line 13, so this line becomes SupplierCategory.create!(id: 1, name: "Other", financial_transaction_class_id: ftc1.id). at the moment there is a mix in the codebase, but it would be great we use the preferred syntax at least in new code.
There was a problem hiding this comment.
Good to know. The method is not required anymore but I will watch out to use that syntax in the future.
Do you know if rubocop could fix this automatically?
There was a problem hiding this comment.
IIRC it could, but it's deactivated ATM to avoid a lot of unrelated code changes when "format on save" is activated.
spec/models/ordergroup_spec.rb
Outdated
| @@ -8,6 +24,16 @@ | |||
| let(:ftt3) { create :financial_transaction_type, financial_transaction_class: ftc2 } | |||
| let(:user) { create :user, groups: [create(:ordergroup)] } | |||
|
|
|||
| it 'shows no active ordergroups when all orders are older than 3 months' do | |||
| create_order(4.months.ago) | |||
There was a problem hiding this comment.
could you use the let(:<name>) { create :<type> } syntax (like in line 20) here too? if not, you should at least use the factory functions (see spec/factories directory) to create the objects instead of hardcoding all parameters (like the supplier name)
There was a problem hiding this comment.
Yess! Thanks for pointing me to the factories. This is much simpler of course 😊
While working on the first complete version of #716 I realized that the filtering of active ordergroups does not work anymore. At least this was my observation in my development environment.
This change fixes the issue for me.
Thinking about it, I guess an autotest would be nice... Will create this PR as a draft and check if it's easy for me to create a failing test for this bug.
Update:
I've added 2 tests for the
Ordergroup.activescope. The helper method probably should be moved to a better location, but wanted to hear your opinion first if this is useful. Also, it contains duplications fromseeds_helper.rb...