Refactor income credits calculation #81
Conversation
- Fixing single user creation
| @incomes = @game.incomes.where(round: @current_round) | ||
| if @last_round > 0 | ||
| @previous_income = @game.incomes.where(round: @last_round) |
app/models/income.rb
Outdated
| end | ||
|
|
||
| private | ||
| def get_income_to_credits_array |
There was a problem hiding this comment.
An other possibility to get this array for the corresponding team could be to add they array to the team model.
There was a problem hiding this comment.
Right now, the user/team structure is sort of in flux. It's sort of a messed up design which needs to be rethought. So it's fine to leave this here until.
app/models/income.rb
Outdated
| end | ||
|
|
||
| private | ||
| def get_income_to_credits_array |
There was a problem hiding this comment.
Right now, the user/team structure is sort of in flux. It's sort of a messed up design which needs to be rethought. So it's fine to leave this here until.
app/models/income.rb
Outdated
| when 'Germany' | ||
| [2, 5, 6, 7, 8, 9, 10, 12, 14] | ||
| else | ||
| [2, 5, 6, 7, 8, 9, 10, 11, 12] |
There was a problem hiding this comment.
We should explicitly state India here and then have an overall default (which can be the same array as India's anyway).
There was a problem hiding this comment.
On second thought, this should just be a hash where keys are the country names and values is the array. Store this at the top of the class as a constant like COUNTRY_CREDIT_LEVELS.
Then in this method, if self.team_name is in COUNTRY_CREDIT_LEVELS.keys then return COUNTRY_CREDIT_LEVELS[self.team_name]. Otherwise, you provide the default (which may as well also be stored as a constant).
spec/models/income_spec.rb
Outdated
| RSpec.describe Income, :type => :model do | ||
| pending "add some examples to (or delete) #{__FILE__}" | ||
|
|
||
| Game::COUNTRIES.each { |country| Team.find_or_create_by(team_name: country) } |
There was a problem hiding this comment.
You should just make the Team call in before(:each) be a find_or_create_by and delete this line. Otherwise, you may cause some unintended side effect since rails does not know how to clean up the created Teams after the rspec is run. In this case, you lucked out because we are doing a find_or_create but if another test eventually attempts to mess with Teams/Team creation we might have unexpected errors.
You could also do this in a before(:all) and then make an after(:all) hook that destroys all Teams. But really it's not worth it because we don't have that many examples that its slowing down the tests.
|
Also, sorry I took so long on this, I got caught up in setting up for our Dec. 3rd game. |
…income-level-to-credits # Conflicts: # app/views/users/new.html.erb
…nded side effects
|
Wow, I was not expecting to wake up and see PR updates for this repo haha. I'll review when I have a chance. |
|
Sorry for the long delay... I have been travelling for the past 5 months! (and will be travelling until this Septembre). |
Related to issue 55: #55
We might need to add more tests on the controllers.