Solve Zad5 #136
Solve Zad5 #136
Conversation
| @@ -3,20 +3,20 @@ | |||
| module Clans | |||
| class WarriorsController < ApplicationController | |||
| def show | |||
| render json: warrior, include: %i[weapon building] | |||
| render json: warrior, include: %i[weapon building], status: 200 | |||
There was a problem hiding this comment.
200 nie jest domyślnym statusem? :D
| end | ||
|
|
||
| def destroy | ||
| warrior.destroy! | ||
| head 204 |
There was a problem hiding this comment.
Wydaje mi się że przez brak render tu i tak leciało 204 :)
| @@ -14,4 +14,10 @@ class Warrior < ApplicationRecord | |||
|
|
|||
| scope :alive, -> { where('death_date IS NULL') } | |||
| scope :dead, -> { where('death_date IS NOT NULL') } | |||
|
|
|||
| after_save :update_siege_abilities | |||
There was a problem hiding this comment.
Jeśli usuniemy warriora z budynku, to w budynku siege ability nie zaktualizuje sie :(
| daily_supplies = 10 | ||
| if @building.warriors.alive.any? | ||
| @building.warriors.alive.each do |warrior| | ||
| daily_supplies += if warrior.type == 'Warriors::Hussar' |
There was a problem hiding this comment.
Ładniej by to chyba wyglądało, 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 :)
| before { get '/buildings' } | ||
| context 'with valid params' do | ||
| context 'with correct name' do | ||
| include_examples 'OK' |
There was a problem hiding this comment.
trochę mało czytelne nazwy tych shared examples, ale duży plus za zastosowanie takich technik! :D
| end | ||
|
|
||
| def call | ||
| raise NotImprementedYet | ||
| daily_supplies = 10 | ||
| if @building.warriors.alive.any? |
There was a problem hiding this comment.
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 :)
| factory :warrior, class: 'Warrior' do | ||
| association(:building) | ||
| association(:clan) | ||
| name { 'WarriorName' } |
There was a problem hiding this comment.
Do problemu z imionami można zastosować konstruckję z sequence żeby zapewnić im unikalność :) NP.:
sequence :email do |n|
"person#{n}@example.com"
end| end | ||
| end | ||
|
|
||
| # BuildingsController POST /buildings with valid params with correct name |
There was a problem hiding this comment.
Tu chodzi o typ testu :) Dla testów typu controller jest trochę inna, powinno się w nich pisać np.:
get :index
post :create
Testy Ci zadziałają, jeśli zmienisz typ testu na request
Większość testów przechodzi. W kodzie znajduje się komentarz z treścią komunikatów zwracanych przez te, które nie przeszły - na wypadek, gdybym nie zdążył poprawić. Póki co brakuje mi już pomysłów:
:name=>"ABCD", :type=>"Buildings::Walls"} w
buildings_controller_spec.rbsiege_report_spec.rb(próbowałem podstawiać na siłę inne imiona - zawsze drugi w danym teście dodany wojownik rzuca tym błędem)