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.

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

Refactor income credits calculation #81

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
37 changes: 9 additions & 28 deletions app/controllers/api/api_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,33 +149,14 @@ def income
user_team = Team.find_by_id(current_user.team_id)

public_relations_list = PublicRelation.where(game: @game, team: user_team, round: round - 1)
income_level = Income.where(game: @game, round: round, team: user_team).limit(1).pluck(:amount)

# TO DO: This is in progress of being renamed.
reserves = BonusCredit.where(game: @game, round: round, team: user_team).limit(1).pluck(:amount)

# TO DO: This still needs to be moved.
@income_values = {}
@income_values['Brazil'] =[2,5,6,7,8,9,10,11,12]
@income_values['China']= [2,3,5,7,9,10,12,14,16]
@income_values['France']= [2,5,6,7,8,9,10,11,12]
@income_values['India']= [2,5,6,7,8,9,10,11,12]
@income_values['Japan']= [3,5,6,8,9,10,12,13,14]
@income_values['Russian Federation'] = [2,4,5,6,7,8,9,10,11]
@income_values['United Kingdom'] =[2,4,6,7,8,9,10,11,12]
@income_values['USA']= [1,3,5,7,9,11,13,15,17]
@income_values['Germany'] =[2,5,6,7,8,9,10,12,14]

# Don't explode if there is no data yet.
if income_level[0]
if income_level[0] >= @income_values[user_team.team_name].length
income_value = @income_values[user_team.team_name][@income_values[user_team.team_name].length - 1]
else
income_value = @income_values[user_team.team_name][income_level[0] - 1]
end
else
income_level = 0
income_value = 0
end
team_bonus_credits = BonusCredit.find_by(game: @game, round: round, team: user_team)
reserves = team_bonus_credits ? team_bonus_credits.amount : 0

team_income = Income.find_by(game: @game, round: round, team: user_team)
income_level = team_income ? team_income.amount : 0
income_value = team_income ? team_income.credits : 0

begin
#generate overall embedded result
Expand All @@ -184,8 +165,8 @@ def income
"round"=> round,
},
"pr" => public_relations_list,
"income_level" => income_level[0],
"reserves" => reserves[0],
"income_level" => income_level,
"reserves" => reserves,
"income_value" => income_value,
}
rescue
Expand Down
17 changes: 0 additions & 17 deletions app/controllers/games_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,7 @@ def human_control
)
@countries = Team.countries
@teams = Team.all_without_incomes

#To Do: Move income values into stored structure somewhere
@income_values = {}
@income_values['Brazil'] =[2,5,6,7,8,9,10,11,12]
@income_values['China']= [2,3,5,7,9,10,12,14,16]
@income_values['France']= [2,5,6,7,8,9,10,11,12]
@income_values['India']= [2,5,6,7,8,9,10,11,12]
@income_values['Japan']= [3,5,6,8,9,10,12,13,14]
@income_values['Russian Federation'] = [2,4,5,6,7,8,9,10,11]
@income_values['United Kingdom'] =[2,4,6,7,8,9,10,11,12]
@income_values['USA']= [1,3,5,7,9,11,13,15,17]
@income_values['Germany'] =[2,5,6,7,8,9,10,12,14]

# Todo: Refactor this so that we get the team name with the income
@incomes = @game.incomes.where(round: @current_round)
if @last_round > 0
@previous_income = @game.incomes.where(round: @last_round)
end
end

def update_den
Expand Down
36 changes: 36 additions & 0 deletions app/models/income.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,40 @@ class Income < ActiveRecord::Base
def team_name
self.team.team_name
end

def credits
income_to_credit = get_income_to_credits_array
safe_get_credits_value(income_to_credit)
end

private
def get_income_to_credits_array
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An other possibility to get this array for the corresponding team could be to add they array to the team model.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

case self.team_name
when 'China'
[2, 3, 5, 7, 9, 10, 12, 14, 16]
when 'Japan'
[3, 5, 6, 8, 9, 10, 12, 13, 14]
when 'Russian Federation'
[2, 4, 5, 6, 7, 8, 9, 10, 11]
when 'United Kingdom'
[2, 4, 6, 7, 8, 9, 10, 11, 12]
when 'USA'
[1, 3, 5, 7, 9, 11, 13, 15, 17]
when 'Germany'
[2, 5, 6, 7, 8, 9, 10, 12, 14]
else
[2, 5, 6, 7, 8, 9, 10, 11, 12]
Copy link
Contributor

Choose a reason for hiding this comment

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

We should explicitly state India here and then have an overall default (which can be the same array as India's anyway).

Copy link
Contributor

Choose a reason for hiding this comment

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

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).

end
end

def safe_get_credits_value(income_to_credit)
if self.amount > income_to_credit.length
income_to_credit[income_to_credit.length - 1]
elsif self.amount < 1
0
else
income_to_credit[self.amount - 1]
end
end

end
8 changes: 1 addition & 7 deletions app/views/games/human_control.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,7 @@
<tr>
<td><%= income.team_name %></td>
<td><%= income.amount%></td>
<td>
<% if @income_values[income.team_name].length <= income.amount %>
<%= @income_values[income.team_name][@income_values[income.team_name].length - 1] %>
<% else %>
<%= @income_values[income.team_name][income.amount-1]%>
<% end %>
</td>
<td><%= income.credits%></td>
<td><%= @game.bonus_credits.where(team: income.team, round: @current_round, recurring: false).sum(:amount) %></td>
<td><%= @game.bonus_credits.where('team_id = ? AND round <= ? AND recurring = TRUE', income.team_id, @current_round).sum(:amount) %></td>
</tr>
Expand Down
11 changes: 6 additions & 5 deletions app/views/users/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,17 @@
<%= f.hidden_field :role, :value => @user.role %>

<%= f.label :team %> </br>
<%= f.select :team, options_for_select(@teams.collect{ |team| [team.team_name, team.id]}) %></br><br />
<%= f.select :team, options_for_select(@teams.collect{ |team| [team.team_name, team.id]}) %></br></br>

<%= f.label :team_role %> </br>
<%= f.select :team_role, options_for_select(@team_roles.collect{ |role| [role.role_display_name, role.id]}) %></br><br />
<%= f.select :team_role, options_for_select(@team_roles.collect{ |role| [role.role_display_name, role.id]}) %></br></br>

<% if @super_admin? %>
<% if @super_admin %>
<%= f.label :game %> </br>
<%= f.select :game, options_for_select(@games.collect{ |game| [game.name, game.id]}) %></br><br />
<%= f.submit %>
<%= f.select :game, options_for_select(@games.collect{ |game| [game.name, game.id]}) %></br></br>
<% end %>

<%= f.submit %></br></br>
</div>
<% end %>
</div>
3 changes: 2 additions & 1 deletion spec/models/game_spec.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
require 'rails_helper'
RSpec.describe Game, :type => :model do
let(:game){ FactoryGirl.create(:game)}

Game::COUNTRIES.each{|country| Team.find_or_create_by(team_name: country)}

let(:game){ FactoryGirl.create(:game)}

before(:each) do
Game::COUNTRIES.each do |country|
#Income starts at 6.
Expand Down
52 changes: 51 additions & 1 deletion spec/models/income_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,55 @@
require 'rails_helper'

def income_for_team(team_name)
team = Team.find_by_team_name(team_name)
Income.find_by(team: team)
end

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) }
Copy link
Contributor

Choose a reason for hiding this comment

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

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.


let(:game) { FactoryGirl.create(:game) }

before(:each) do
Game::COUNTRIES.each do |country|
#Income starts at 6.
team = Team.find_by_team_name(country)
income = Income.find_or_create_by(game: game, round: 0, amount: 6, team: team)
end
end

it 'can get amount of credits from income object' do
income_for_usa = income_for_team('USA')
income_for_china = income_for_team('China')
income_for_france = income_for_team('France')

expect(income_for_usa.credits).to eq(11)
expect(income_for_china.credits).to eq(10)
expect(income_for_france.credits).to eq(9)

income_for_usa.amount = 7
income_for_china.amount = 7
income_for_france.amount = 7

expect(income_for_usa.credits).to eq(13)
expect(income_for_china.credits).to eq(12)
expect(income_for_france.credits).to eq(10)
end

it 'can get amount of credits from income object when amount higher than expected maximum' do
income_for_usa = income_for_team('USA')

income_for_usa.amount = 9
expect(income_for_usa.credits).to eq(17)
income_for_usa.amount = 15
expect(income_for_usa.credits).to eq(17)
end

it 'can get amount of credits from income object when amount lower than 1' do
income_for_usa = income_for_team('USA')

income_for_usa.amount = 0
expect(income_for_usa.credits).to eq(0)
end
end