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

Zad5 #125

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions lecture_5/homework/.rspec
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--require spec_helper
3 changes: 3 additions & 0 deletions lecture_5/homework/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" }

ruby '2.6.2'

gem 'factory_bot_rails'
gem 'faker'
gem 'rubocop-rspec'
# Bundle edge Rails instead: gem 'rails', github: 'rails/rails'
gem 'rails', '~> 5.2.2'
# Use pg as the database for Active Record
Expand Down
96 changes: 54 additions & 42 deletions lecture_5/homework/Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
GEM
remote: https://rubygems.org/
specs:
actioncable (5.2.2.1)
actionpack (= 5.2.2.1)
actioncable (5.2.3)
actionpack (= 5.2.3)
nio4r (~> 2.0)
websocket-driver (>= 0.6.1)
actionmailer (5.2.2.1)
actionpack (= 5.2.2.1)
actionview (= 5.2.2.1)
activejob (= 5.2.2.1)
actionmailer (5.2.3)
actionpack (= 5.2.3)
actionview (= 5.2.3)
activejob (= 5.2.3)
mail (~> 2.5, >= 2.5.4)
rails-dom-testing (~> 2.0)
actionpack (5.2.2.1)
actionview (= 5.2.2.1)
activesupport (= 5.2.2.1)
actionpack (5.2.3)
actionview (= 5.2.3)
activesupport (= 5.2.3)
rack (~> 2.0)
rack-test (>= 0.6.3)
rails-dom-testing (~> 2.0)
rails-html-sanitizer (~> 1.0, >= 1.0.2)
actionview (5.2.2.1)
activesupport (= 5.2.2.1)
actionview (5.2.3)
activesupport (= 5.2.3)
builder (~> 3.1)
erubi (~> 1.4)
rails-dom-testing (~> 2.0)
Expand All @@ -29,36 +29,43 @@ GEM
activemodel (>= 4.1, < 6)
case_transform (>= 0.2)
jsonapi-renderer (>= 0.1.1.beta1, < 0.3)
activejob (5.2.2.1)
activesupport (= 5.2.2.1)
activejob (5.2.3)
activesupport (= 5.2.3)
globalid (>= 0.3.6)
activemodel (5.2.2.1)
activesupport (= 5.2.2.1)
activerecord (5.2.2.1)
activemodel (= 5.2.2.1)
activesupport (= 5.2.2.1)
activemodel (5.2.3)
activesupport (= 5.2.3)
activerecord (5.2.3)
activemodel (= 5.2.3)
activesupport (= 5.2.3)
arel (>= 9.0)
activestorage (5.2.2.1)
actionpack (= 5.2.2.1)
activerecord (= 5.2.2.1)
activestorage (5.2.3)
actionpack (= 5.2.3)
activerecord (= 5.2.3)
marcel (~> 0.3.1)
activesupport (5.2.2.1)
activesupport (5.2.3)
concurrent-ruby (~> 1.0, >= 1.0.2)
i18n (>= 0.7, < 2)
minitest (~> 5.1)
tzinfo (~> 1.1)
arel (9.0.0)
ast (2.4.0)
bootsnap (1.4.1)
bootsnap (1.4.3)
msgpack (~> 1.0)
builder (3.2.3)
byebug (11.0.0)
byebug (11.0.1)
case_transform (0.2)
activesupport
concurrent-ruby (1.1.5)
crass (1.0.4)
diff-lcs (1.3)
erubi (1.8.0)
factory_bot (5.0.2)
activesupport (>= 4.2.0)
factory_bot_rails (5.0.2)
factory_bot (~> 5.0.2)
railties (>= 4.2.0)
faker (1.9.3)
i18n (>= 0.7)
ffi (1.10.0)
globalid (0.4.2)
activesupport (>= 4.2.0)
Expand All @@ -84,38 +91,38 @@ GEM
minitest (5.11.3)
msgpack (1.2.9)
nio4r (2.3.1)
nokogiri (1.10.1)
nokogiri (1.10.2)
mini_portile2 (~> 2.4.0)
parallel (1.17.0)
parser (2.6.2.0)
parser (2.6.2.1)
ast (~> 2.4.0)
pg (1.1.4)
psych (3.1.0)
puma (3.12.0)
rack (2.0.6)
puma (3.12.1)
rack (2.0.7)
rack-test (1.1.0)
rack (>= 1.0, < 3)
rails (5.2.2.1)
actioncable (= 5.2.2.1)
actionmailer (= 5.2.2.1)
actionpack (= 5.2.2.1)
actionview (= 5.2.2.1)
activejob (= 5.2.2.1)
activemodel (= 5.2.2.1)
activerecord (= 5.2.2.1)
activestorage (= 5.2.2.1)
activesupport (= 5.2.2.1)
rails (5.2.3)
actioncable (= 5.2.3)
actionmailer (= 5.2.3)
actionpack (= 5.2.3)
actionview (= 5.2.3)
activejob (= 5.2.3)
activemodel (= 5.2.3)
activerecord (= 5.2.3)
activestorage (= 5.2.3)
activesupport (= 5.2.3)
bundler (>= 1.3.0)
railties (= 5.2.2.1)
railties (= 5.2.3)
sprockets-rails (>= 2.0.0)
rails-dom-testing (2.0.3)
activesupport (>= 4.2.0)
nokogiri (>= 1.6)
rails-html-sanitizer (1.0.4)
loofah (~> 2.2, >= 2.2.2)
railties (5.2.2.1)
actionpack (= 5.2.2.1)
activesupport (= 5.2.2.1)
railties (5.2.3)
actionpack (= 5.2.3)
activesupport (= 5.2.3)
method_source
rake (>= 0.8.7)
thor (>= 0.19.0, < 2.0)
Expand Down Expand Up @@ -149,6 +156,8 @@ GEM
rainbow (>= 2.2.2, < 4.0)
ruby-progressbar (~> 1.7)
unicode-display_width (>= 1.4.0, < 1.6)
rubocop-rspec (1.32.0)
rubocop (>= 0.60.0)
ruby-progressbar (1.10.0)
ruby_dep (1.5.0)
spring (2.0.2)
Expand Down Expand Up @@ -179,12 +188,15 @@ DEPENDENCIES
active_model_serializers
bootsnap (>= 1.1.0)
byebug
factory_bot_rails
faker
listen (>= 3.0.5, < 3.2)
pg
puma (~> 3.11)
rails (~> 5.2.2)
rspec-rails
rubocop
rubocop-rspec
spring
spring-watcher-listen (~> 2.0.0)
tzinfo-data
Expand Down
18 changes: 16 additions & 2 deletions lecture_5/homework/app/controllers/buildings_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,21 @@
# frozen_string_literal: true

class BuildingsController < ApplicationController
def index; end
def index
render json: Building.all.order(:id), include: [:warriors]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Id nie jest domyslnym orderem? :D

end

def show; end
def show
render json: building, include: [:warriors]
end

private

def build_params
params.permit(:name,:granary)
end

def building
@building ||= Building.find(params[:id])
end
end
3 changes: 2 additions & 1 deletion lecture_5/homework/app/models/building.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ class Building < ApplicationRecord
has_many :warriors, dependent: :nullify

validates :granary, presence: true,
numerically: { greater_than_or_equal_to: 0, only_integer: true }
numericality: { greater_than_or_equal_to: 0, only_integer: true }
validates :siege_ability, presence: true, numericality: { greater_than_or_equal_to: 0 }
end
37 changes: 37 additions & 0 deletions lecture_5/homework/app/models/warrior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,41 @@ class Warrior < ApplicationRecord

scope :alive, -> { where('death_date IS NULL') }
scope :dead, -> { where('death_date IS NOT NULL') }

after_create :update_siege
before_update :update_prev_building
# cant put same if: :building_id_changed? since it will always return false
after_update :update_siege_after
# after_commit :update_siege_after
after_destroy :update_siege

def update_siege
# call after warrior already was asigned to building list after create,update
# works for delete as well
Reports::SiegeReport.call(building: building) if building
end

# it seems like rspec didnt invoke those callbacks
# when i was testing manually it works
# probably messed up in factory and tests :(
def update_siege_after
if prev_building_id
# if warrior changed his building run this on both
# since in that moment he is no longer in old list , rather in new building
# so this way i can keep both old and new buildings updated
# puts "prev building: #{prev_building_id} current: #{building_id}"
Reports::SiegeReport.call(building: Building.find(prev_building_id))
Reports::SiegeReport.call(building: Building.find(building_id))
# set this to nil to prevent callback invoking from random update
update_column(:prev_building_id, nil)
end
end

def update_prev_building
# if warrior is changing his buildings
if building_id_was != building_id
# save this attribute wihtout triggering callbacks
update_column(:prev_building_id, building_id_was)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Z tego co pamiętam update_column omija walidację więc nie jest to najlepszy sposób na edycje kolumny :D Najprosciej byłoby dodać relację typu belongs_to z opcją optional: true :) Optional pozwoli na ustawianie nila na tej kolumnie, nie trzeba nic zmieniać w bazie, bo bazy domyślnie pozwalają na nile, a jedynie rails sam waliduje to że nie mogą być nilem :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Poza tym w sumie gdy sie wbijasz callbackiem miedzy zmiane danych a sve, masz dostep do zmiennej metody changes, ktora zwraca to co sie zmienilo, dzieki czemu nie ma potrzeby w tym przypadku poprzednim budynek na uzytkowniku, bo w callbacku informacja z jakiego na jakiego zmienilo sie building id juz jest

end
end
end
2 changes: 1 addition & 1 deletion lecture_5/homework/app/serializers/building_serializer.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

class BuildingSerializer < ActiveModel::Serializer
attributes :name
attributes :name, :siege_ability, :granary

has_many :warriors
end
18 changes: 14 additions & 4 deletions lecture_5/homework/app/services/reports/siege_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,21 @@

module Reports
class SiegeReport
def initialize(building:)
end
def self.call(building:)
# this should stop the method if getting null
return unless building
Copy link
Collaborator

Choose a reason for hiding this comment

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

To sprawdzenie spokojnie by się mogło znaleźć gdzieś wyżej, żeby nie obciążać logiki serwisu :) Serwis najlepiej jeśli ma jedną odpowiedzialność, a Twój serwis poza liczeniem ile dni przetrwa dany budynek, stwierdza upadłość budynku jeśli nie ma w nim wojowników :)


def call
raise NotImprementedYet
# bulding cant be defended without warriors
if building.warriors.any?
warriors_count = building.warriors.where(type: 'Warriors::Samurai').count
hussar_count = building.warriors.where(type: 'Warriors::Hussar').count
total_rice_need = warriors_count + hussar_count * 2 + building.default_rice_need
Copy link
Collaborator

Choose a reason for hiding this comment

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

Te powyższe trzy linijki ładniej by wyglądałī, jeśli Warrior miałby zaimplementowaną metodę/atrybut np. supply_burn_rate, który zwraca 1 jeśli warrior nie ma konia i 2 jeśli go ma, wtedy nie trzeba by myśleć nad takimi ifami w tym serwisie :)

siege_ability = building.granary / total_rice_need
building.siege_ability = siege_ability
else
building.siege_ability = 0
end
building.save!
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddSiegeAbility < ActiveRecord::Migration[5.2]
def change
add_column :buildings, :siege_ability, :integer, default: 0, null: false

end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class AddDefaultRiceNeed < ActiveRecord::Migration[5.2]
def change
add_column :buildings, :default_rice_need, :integer, default: 0

Buildings::Stronghold.find_each { |building| update_attribute(:default_rice_need,10) }
Building.find_each { |building| Reports::SiegeReport.check_siege_ability(building: building) }
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddPreviousBuildingId < ActiveRecord::Migration[5.2]
def change
add_column :warriors, :prev_building_id, :integer

end
end
6 changes: 5 additions & 1 deletion lecture_5/homework/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2019_04_05_090544) do
ActiveRecord::Schema.define(version: 2019_04_17_075546) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand All @@ -20,6 +20,9 @@
t.string "type", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.integer "granary", default: 0, null: false
t.integer "siege_ability", default: 0, null: false
t.integer "default_rice_need", default: 0
end

create_table "clans", force: :cascade do |t|
Expand Down Expand Up @@ -53,6 +56,7 @@
t.string "type", default: "Warriors::Samurai", null: false
t.bigint "building_id"
t.string "preferred_weapon_kind"
t.integer "prev_building_id"
t.index ["building_id"], name: "index_warriors_on_building_id"
t.index ["clan_id"], name: "index_warriors_on_clan_id"
t.index ["name"], name: "index_warriors_on_name", unique: true, where: "(death_date IS NULL)"
Expand Down
4 changes: 3 additions & 1 deletion lecture_5/homework/db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
first_clan = Clan.create!(name: 'The very first clan')
training_clan = Clan.create!(name: 'Training clan')

stronghold = Buildings::Stronghold.create!(name: 'Main stronghold', granary: 5000)
stronghold = Buildings::Stronghold.create!(name: 'Main stronghold', granary: 5000,default_rice_need: 10)
stronghold_empty = Buildings::Stronghold.create!(name: 'Empty stronghold', granary: 5000,default_rice_need: 10)
northern_wall = Buildings::Walls.create!(name: 'Northern wall', granary: 100)
southern_wall = Buildings::Walls.create!(name: 'Southern wall', granary: 100)

samurai_1 = Warriors::Samurai.create!(name: '曽山大輝', clan: first_clan, death_date: 1.week.ago)
samurai_2 = Warriors::Samurai.create!(name: '曽山大輝', clan: first_clan, building: stronghold)
samurai_3 = Warriors::Samurai.create!(name: '澄田清', clan: training_clan, building: stronghold)
hussar_4 = Warriors::Hussar.create!(name: 'Banzai Hito', clan: first_clan, building: stronghold)
hussar_1 = Warriors::Hussar.create!(name: 'Idzi Kwiatkowski', clan: first_clan, building: northern_wall)
hussar_2 = Warriors::Hussar.create!(name: 'Świętopełk Król', clan: training_clan, building: southern_wall)

Expand Down
27 changes: 27 additions & 0 deletions lecture_5/homework/spec/controllers/buildings_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe BuildingsController, type: :controller do
subject(:building) { create(:building) }
describe 'it get all buildings' do
it 'responds with 200' do
get 'index'
expect(response).to have_http_status(200)
end
end

describe 'it should find some building' do
it 'responds with 200' do
get :show, params: { id: building.id }
expect(response).to have_http_status(200)
end
end

describe 'it shows no building found' do
it 'responds with 404' do
get :show, params: { id: 999 }
expect(response).to have_http_status(404)
end
end
end
Loading