-
Notifications
You must be signed in to change notification settings - Fork 37
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
Add homework_5 #134
base: master
Are you sure you want to change the base?
Add homework_5 #134
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
--require spec_helper |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,17 @@ | ||
# frozen_string_literal: true | ||
|
||
class BuildingsController < ApplicationController | ||
def index; end | ||
def index | ||
render json: Building.all | ||
end | ||
|
||
def show; end | ||
def show | ||
render json: building, include: 'warrior' | ||
end | ||
|
||
private | ||
|
||
def building | ||
@building ||= Building.find(params[:id]) | ||
end | ||
end |
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, :granary, :siege_ability | ||
|
||
has_many :warriors | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,10 +3,28 @@ | |
module Reports | ||
class SiegeReport | ||
def initialize(building:) | ||
@building = building | ||
@warriors = building.warriors | ||
end | ||
|
||
def call | ||
raise NotImprementedYet | ||
siege = @warriors.present? ? @building.siege_ability = granary_ability : @building.siege_ability = 0 | ||
@building.save | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To zapisywanie atrybutu niepotrzebnie dociąża nam ten serwis, powinno też się znaleźć "wyżej" :) Wiąże się to też między innymi ze wzorcem dobrego projektowania aplikacji, Single Responsibility Principle. Ogólnie możnaby zoptymalizować nasz callback przez obsługę wyliczenia tych dni i zapisania tej wartości na budynku w kolejce asynchronicznej np. Sidekiq, Resque, ActiveJob :D Albo przez w ogóle zastanowieniem się czy nie wystarczy liczyć tych dni w serializerze, który by potrafił scachować wywołanie tego serwisu na jakiś czas, żeby oszczędzić pracy apce, ale to są tematy których nie było na LevelUp, nie mniej są bardzo ważne i polecam je zbadać :) napewno pomyślimy o nich w kolejnych edycjach, a napewno będą na praktykach :D There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hej, |
||
siege | ||
end | ||
|
||
private | ||
|
||
def granary_request | ||
daily_request = 10 | ||
samurais = @warriors.where(type: 'Warriors::Samurai').count | ||
hussars = @warriors.where(type: 'Warriors::Hussar').count * 2 | ||
daily_request + samurais + hussars | ||
end | ||
|
||
def granary_ability | ||
granary = @building.granary | ||
granary / granary_request | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
class AddSiegeAbilityOnBuilding < ActiveRecord::Migration[5.2] | ||
def change | ||
add_column :buildings, :siege_ability, :integer, default: 0, null: false | ||
|
||
Building.find_each { |building| Reports::SiegeReport.new(building: building).call } | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
# frozen_string_literal: true | ||
|
||
# This file is copied to spec/ when you run 'rails generate rspec:install' | ||
require 'spec_helper' | ||
ENV['RAILS_ENV'] ||= 'test' | ||
require File.expand_path('../../config/environment', __FILE__) | ||
# Prevent database truncation if the environment is production | ||
abort('The Rails environment is running in production mode!') if Rails.env.production? | ||
require 'rspec/rails' | ||
require 'support/factory_bot' | ||
# Add additional requires below this line. Rails is not loaded until this point! | ||
|
||
# Requires supporting ruby files with custom matchers and macros, etc, in | ||
# spec/support/ and its subdirectories. Files matching `spec/**/*_spec.rb` are | ||
# run as spec files by default. This means that files in spec/support that end | ||
# in _spec.rb will both be required and run as specs, causing the specs to be | ||
# run twice. It is recommended that you do not name files matching this glob to | ||
# end with _spec.rb. You can configure this pattern with the --pattern | ||
# option on the command line or in ~/.rspec, .rspec or `.rspec-local`. | ||
# | ||
# The following line is provided for convenience purposes. It has the downside | ||
# of increasing the boot-up time by auto-requiring all files in the support | ||
# directory. Alternatively, in the individual `*_spec.rb` files, manually | ||
# require only the support files necessary. | ||
# | ||
# Dir[Rails.root.join('spec', 'support', '**', '*.rb')].each { |f| require f } | ||
|
||
# Checks for pending migrations and applies them before tests are run. | ||
# If you are not using ActiveRecord, you can remove these lines. | ||
begin | ||
ActiveRecord::Migration.maintain_test_schema! | ||
rescue ActiveRecord::PendingMigrationError => e | ||
puts e.to_s.strip | ||
exit 1 | ||
end | ||
RSpec.configure do |config| | ||
# Remove this line if you're not using ActiveRecord or ActiveRecord fixtures | ||
config.fixture_path = "#{::Rails.root}/spec/fixtures" | ||
|
||
# If you're not using ActiveRecord, or you'd prefer not to run each of your | ||
# examples within a transaction, remove the following line or assign false | ||
# instead of true. | ||
config.use_transactional_fixtures = true | ||
|
||
# RSpec Rails can automatically mix in different behaviours to your tests | ||
# based on their file location, for example enabling you to call `get` and | ||
# `post` in specs under `spec/controllers`. | ||
# | ||
# You can disable this behaviour by removing the line below, and instead | ||
# explicitly tag your specs with their type, e.g.: | ||
# | ||
# RSpec.describe UsersController, :type => :controller do | ||
# # ... | ||
# end | ||
# | ||
# The different available types are documented in the features, such as in | ||
# https://relishapp.com/rspec/rspec-rails/docs | ||
config.infer_spec_type_from_file_location! | ||
|
||
# Filter lines from Rails gems in backtraces. | ||
config.filter_rails_from_backtrace! | ||
# arbitrary gems may also be filtered via: | ||
# config.filter_gems_from_backtrace("gem name") | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
# frozen_string_literal: true | ||
|
||
require 'rails_helper' | ||
|
||
RSpec.describe 'Buildings API', type: :request do | ||
describe 'GET /buildings' do | ||
it 'responds with 200' do | ||
get '/buildings' | ||
expect(response).to have_http_status(200) | ||
end | ||
end | ||
|
||
describe 'GET /buildings/:id' do | ||
it 'response with 404' do | ||
get '/buildings/1' | ||
expect(response).to have_http_status(404) | ||
end | ||
end | ||
|
||
describe 'GET /buildings/:id' do | ||
let(:building) { create(:building) } | ||
|
||
it 'response with 200' do | ||
get "/buildings/#{building.id}" | ||
expect(response).to have_http_status(200) | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
# frozen_string_literal: true | ||
|
||
require 'rails_helper' | ||
|
||
RSpec.describe Reports::SiegeReport, type: :service do | ||
subject(:siege_report) { Reports::SiegeReport.new(building: building).call } | ||
|
||
let(:building) { create(:building) } | ||
|
||
describe '#call' do | ||
context 'when building without warriors' do | ||
it 'returns 0' do | ||
expect(siege_report).to eq(0) | ||
end | ||
end | ||
end | ||
|
||
describe '#call' do | ||
context 'when building with one samurai' do | ||
let(:warrior) { build(:warrior, building: building) } | ||
|
||
it 'returns 18' do | ||
warrior.save | ||
expect(siege_report).to eq(18) | ||
end | ||
end | ||
end | ||
|
||
describe '#call' do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. jeden describe wrapujący całość dotyczących go testów by wystarczył :) |
||
context 'when building with one hussar' do | ||
let(:warrior) { build(:warrior, type: warrior_type, building: building) } | ||
let(:warrior_type) { 'Warriors::Hussar' } | ||
|
||
it 'returns 16' do | ||
warrior.save | ||
expect(siege_report).to eq(16) | ||
end | ||
end | ||
end | ||
|
||
describe '#call' do | ||
context 'when building with two types of warriors' do | ||
let(:warrior1) { build(:warrior, building: building) } | ||
let(:warrior2) { build(:warrior, name: hussar_name, type: warrior_type, building: building) } | ||
let(:hussar_name) { 'Frog Hussar' } | ||
let(:warrior_type) { 'Warriors::Hussar' } | ||
|
||
it 'returns 15' do | ||
warrior1.save | ||
warrior2.save | ||
expect(siege_report).to eq(15) | ||
end | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
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 :)