diff --git a/app/controllers/ad_api_controller.rb b/app/controllers/ad_api_controller.rb index bee16c1..d0f5bd7 100644 --- a/app/controllers/ad_api_controller.rb +++ b/app/controllers/ad_api_controller.rb @@ -8,7 +8,7 @@ def view target_ids = Ad.pluck(:id).sample(params[:count].to_i) ads = Ad.find(target_ids) ads.each do |ad| - report = Report.find_by(ad_id: ad.id, adspot_id: params[:adspot_id],date: Date.today) + report = Report.find_by(ad_id: ad.id, adspot_id: params[:adspot_id], date: Date.today) unless report report = Report.new(ad_id: ad.id, adspot_id: params[:adspot_id], date: Date.today) end @@ -27,7 +27,7 @@ def view end def click - report = Report.find_by(ad_id: params[:ad_id], adspot_id: params[:adspot_id],date: Date.today) + report = Report.find_by(ad_id: params[:ad_id], adspot_id: params[:adspot_id], date: Date.today) unless report report = Report.new(ad_id: params[:ad_id], adspot_id: params[:adspot_id], date: Date.today) end @@ -37,4 +37,3 @@ def click end end - diff --git a/app/controllers/ad_controller.rb b/app/controllers/ad_controller.rb index e7c1ade..1988e4e 100644 --- a/app/controllers/ad_controller.rb +++ b/app/controllers/ad_controller.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true +require 'date' class AdController < ApplicationController def index @ads = Ad.all @@ -38,9 +39,30 @@ def destroy redirect_to(ad_index_path) end + def report #report.htmlに全レポートを出力 + @reports = Report.select(" + ad_id, + SUM(reports.imp) AS imp, + SUM(reports.click) AS click, + SUM(reports.cv) AS cv, + SUM(reports.price) AS price + ").group(:ad_id) + end + + def report_period #report.htmlに一定期間のレポートを出力 + @reports = Report.select(" + ad_id, + SUM(reports.imp) AS imp, + SUM(reports.click) AS click, + SUM(reports.cv) AS cv, + SUM(reports.price) AS price + ").where(date: Date.parse(params[:date_min])..Date.parse(params[:date_max])).group(:ad_id) + render('/ad/report') + end + private def ad_params # Adオブジェクト作成時にフォームから入力したパラメーターを渡す。 - params.require(:ad).permit(:price, :text, :advertiser_id, :image) + params.require(:ad).permit(:price, :text, :image) end end diff --git a/app/models/ad.rb b/app/models/ad.rb index 22cd01c..9939b33 100644 --- a/app/models/ad.rb +++ b/app/models/ad.rb @@ -4,6 +4,6 @@ class Ad < ApplicationRecord validates :text, presence: true validates :price, presence: true validates :image, presence: true - validates :advertiser_id, presence: true + has_many :reports end diff --git a/app/models/report.rb b/app/models/report.rb index eb2599f..f9180b6 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -1,3 +1,4 @@ # frozen_string_literal: true class Report < ApplicationRecord + belongs_to :ad end diff --git a/app/views/ad/edit.html.erb b/app/views/ad/edit.html.erb index 8e25f42..6040959 100644 --- a/app/views/ad/edit.html.erb +++ b/app/views/ad/edit.html.erb @@ -5,8 +5,6 @@ <%= f.text_area :text,value: @ad.text,class: "form-control" %>

price

<%= f.number_field :price,value: @ad.price %> -

advertiser_id

- <%= f.number_field :advertiser_id,value: @ad.advertiser_id %>

image

<%= f.file_field :image,value: @ad.image,class: "form-control-file" %> <%= f.submit class: "btn btn-primary",value: "更新" %> diff --git a/app/views/ad/new.html.erb b/app/views/ad/new.html.erb index a841e3b..fe3587d 100644 --- a/app/views/ad/new.html.erb +++ b/app/views/ad/new.html.erb @@ -5,8 +5,6 @@ <%= f.text_area :text,value: @ad.text,class: "form-control" %>

price

<%= f.number_field :price,value: @ad.price %> -

advertiser_id

- <%= f.number_field :advertiser_id,value: @ad.advertiser_id %>

image

<%= f.file_field :image,value: @ad.image,class: "form-control-file" %> <%= f.submit class: "btn btn-primary",value: "入稿" %> diff --git a/app/views/ad/report.html.erb b/app/views/ad/report.html.erb new file mode 100644 index 0000000..25848c2 --- /dev/null +++ b/app/views/ad/report.html.erb @@ -0,0 +1,33 @@ +<%= form_tag(report_period_path, method: :get) do %> + <%= date_field_tag :date_min %> + <%= date_field_tag :date_max %> + <%= submit_tag "期間指定", class: "btn btn-primary" %> +<% end %> +
+ + + + + + + + + + + + + + <% @reports.each do |report| %> + + + + + + + + <% end %> + +
広告IDImpression数Click数総費用(円)CTRCPM
<%= report.ad_id %><%= report.imp %><%= report.click %><%= report.price %> + <%= (report.click.to_f / report.imp.to_f).round(3) %> + <%= (report.price / (1000 / report.imp)) %>
+
diff --git a/config/routes.rb b/config/routes.rb index f0de26f..3d739d7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -3,5 +3,7 @@ get '/view' => 'ad_api#view' get '/click' => 'ad_api#click' resources :ad + get '/report' => 'ad#report' + get '/report_period' => 'ad#report_period' # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html end diff --git a/db/migrate/20190528085221_create_ads.rb b/db/migrate/20190528085221_create_ads.rb index 27ae217..a7435e9 100644 --- a/db/migrate/20190528085221_create_ads.rb +++ b/db/migrate/20190528085221_create_ads.rb @@ -2,7 +2,6 @@ class CreateAds < ActiveRecord::Migration[5.2] # Reportテーブルのtotalpriceにpriceを加算する為に使用しているのでここでも使っています def change create_table :ads do |t| - t.integer :advertiser_id, null: false # 広告主ID t.string :image, null: false, default: '' # 広告の画像URL t.integer :price, null: false # 広告の価格 t.string :text, null: false, default: '' # 広告の説明文 diff --git a/db/migrate/20190614041856_create_repos.rb b/db/migrate/20190614041856_create_repos.rb index afcdf16..3e1ddf2 100644 --- a/db/migrate/20190614041856_create_repos.rb +++ b/db/migrate/20190614041856_create_repos.rb @@ -8,7 +8,7 @@ def change t.integer :imp, null: false, default: 0 t.integer :cv, null: false, default: 0 t.integer :price, null: false, default: 0 - t.date :date,null: false,unique: true + t.date :date, null: false, unique: true t.timestamps end end diff --git a/db/schema.rb b/db/schema.rb index 9b77ba1..fb855ca 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true # This file is auto-generated from the current state of the database. Instead # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. @@ -11,26 +12,23 @@ # It's strongly recommended that you check this file into your version control system. ActiveRecord::Schema.define(version: 2019_06_14_041856) do - - create_table "ads", force: :cascade do |t| - t.integer "advertiser_id", null: false - t.string "image", default: "", null: false - t.integer "price", null: false - t.string "text", default: "", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + create_table 'ads', force: :cascade do |t| + t.string 'image', default: '', null: false + t.integer 'price', null: false + t.string 'text', default: '', null: false + t.datetime 'created_at', null: false + t.datetime 'updated_at', null: false end - create_table "reports", force: :cascade do |t| - t.integer "ad_id", null: false - t.integer "adspot_id", null: false - t.integer "click", default: 0, null: false - t.integer "imp", default: 0, null: false - t.integer "cv", default: 0, null: false - t.integer "price", default: 0, null: false - t.date "date", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + create_table 'reports', force: :cascade do |t| + t.integer 'ad_id', null: false + t.integer 'adspot_id', null: false + t.integer 'click', default: 0, null: false + t.integer 'imp', default: 0, null: false + t.integer 'cv', default: 0, null: false + t.integer 'price', default: 0, null: false + t.date 'date', null: false + t.datetime 'created_at', null: false + t.datetime 'updated_at', null: false end - end diff --git a/spec/controllers/ad_api_controller_spec.rb b/spec/controllers/ad_api_controller_spec.rb index 072b322..065255f 100644 --- a/spec/controllers/ad_api_controller_spec.rb +++ b/spec/controllers/ad_api_controller_spec.rb @@ -3,101 +3,94 @@ RSpec.describe AdApiController, type: :controller do before do - @ad1 =FactoryBot.create(:ad) + @ad1 = FactoryBot.create(:ad) end describe 'GET #view' do it 'returns http success with valid params' do - get :view,params: {adspot_id: 1,count: 1} + get :view, params: { adspot_id: 1, count: 1 } expect(response).to have_http_status(:success) end - describe "Report" do - it "an ad should be recorded" do - expect { get :view,params: {adspot_id: 1,count: 1} }.to change(Report, :count).by(1) + describe 'Report' do + it 'an ad should be recorded' do + expect { get :view, params: { adspot_id: 1, count: 1 } }.to change(Report, :count).by(1) end it "record 'imp' should be increased" do - get :view,params: {adspot_id: 1,count: 1} - report = Report.find_by(ad_id: @ad1.id, adspot_id: 1, date: Date.today) + get :view, params: { adspot_id: 1, count: 1 } + report = Report.find_by(ad_id: @ad1.id, adspot_id: 1, date: Date.today) expect(report.imp).to eq 1 end it "an ad shouldn't be recorded twice from the same adspot_id on the same day" do - get :view,params: {adspot_id: 100,count: 1} - expect { get :view,params: {adspot_id: 100,count: 1}}.to change(Report, :count).by(0) + get :view, params: { adspot_id: 100, count: 1 } + expect { get :view, params: { adspot_id: 100, count: 1 } }.to change(Report, :count).by(0) end - it "an ad should be recorded from other adspots" do - get :view,params: {adspot_id: 100,count: 1} - expect { get :view,params: {adspot_id: 101,count: 1}}.to change(Report, :count).from(1).to(2) + it 'an ad should be recorded from other adspots' do + get :view, params: { adspot_id: 100, count: 1 } + expect { get :view, params: { adspot_id: 101, count: 1 } }.to change(Report, :count).from(1).to(2) end - it "ads should be recorded at the same time" do - ad2 =FactoryBot.create(:ad) - expect { get :view,params: {adspot_id: 101,count: 2}}.to change(Report, :count).from(0).to(2) + it 'ads should be recorded at the same time' do + ad2 = FactoryBot.create(:ad) + expect { get :view, params: { adspot_id: 101, count: 2 } }.to change(Report, :count).from(0).to(2) end - it "an ad should be recorded on the other day" do - get :view,params: {adspot_id: 101,count: 1} + it 'an ad should be recorded on the other day' do + get :view, params: { adspot_id: 101, count: 1 } travel 1.day do - expect { get :view,params: {adspot_id: 101,count: 1}}.to change(Report, :count).from(1).to(2) + expect { get :view, params: { adspot_id: 101, count: 1 } }.to change(Report, :count).from(1).to(2) end + end - end - - - context "when many ads are requested" do - - it "all ads should be visible " do - @ad2 =FactoryBot.create(:ad) - @ad3 =FactoryBot.create(:ad) - get :view,params: {adspot_id: 1,count: 3} - jsons = JSON.parse(response.body) - - - ad1_ispresent = false - ad2_ispresent = false - ad3_ispresent = false - - jsons.each do |json| - if json['ad_id'] == @ad1.id - ad1_ispresent = true - elsif json['ad_id'] == @ad2.id - ad2_ispresent = true - elsif json['ad_id'] == @ad3.id - ad3_ispresent = true + context 'when many ads are requested' do + it 'all ads should be visible ' do + @ad2 = FactoryBot.create(:ad) + @ad3 = FactoryBot.create(:ad) + get :view, params: { adspot_id: 1, count: 3 } + jsons = JSON.parse(response.body) + + ad1_ispresent = false + ad2_ispresent = false + ad3_ispresent = false + + jsons.each do |json| + if json['ad_id'] == @ad1.id + ad1_ispresent = true + elsif json['ad_id'] == @ad2.id + ad2_ispresent = true + elsif json['ad_id'] == @ad3.id + ad3_ispresent = true + end end + expect(ad1_ispresent).to be_truthy + expect(ad2_ispresent).to be_truthy + expect(ad3_ispresent).to be_truthy end - expect(ad1_ispresent).to be_truthy - expect(ad2_ispresent).to be_truthy - expect(ad3_ispresent).to be_truthy end end end -end - -describe 'GET #click' do - before do - @ad =FactoryBot.create(:ad) - get :view,params: {adspot_id: 1,count: 1} - # get :click,params: {ad_id: ad.id,adspot_id: 1} - #@report = Report.new(ad_id: params[:ad_id], adspot_id: params[:adspot_id], date: Date.today) - - end - it 'returns http success' do - get :click,params: {ad_id: @ad.id,adspot_id: 1} - expect(response).to have_http_status(:success) - end + describe 'GET #click' do + before do + @ad = FactoryBot.create(:ad) + get :view, params: { adspot_id: 1, count: 1 } + # get :click,params: {ad_id: ad.id,adspot_id: 1} + # @report = Report.new(ad_id: params[:ad_id], adspot_id: params[:adspot_id], date: Date.today) + end - describe "Report" do + it 'returns http success' do + get :click, params: { ad_id: @ad.id, adspot_id: 1 } + expect(response).to have_http_status(:success) + end - it "an ad should be recorded " do - expect { get :click,params: {ad_id: @ad.id,adspot_id: 1} }.to change(Report, :count).by(0) + describe 'Report' do + it 'an ad should be recorded ' do + expect { get :click, params: { ad_id: @ad.id, adspot_id: 1 } }.to change(Report, :count).by(0) end - # it "record 'click' should be increased" do # report = Report.find_by(ad_id: @ad.id, adspot_id: 1,date: Date.today) # expect{get :click,params: {ad_id: @ad.id,adspot_id: 1}}.to change(report.click).by(0) @@ -125,8 +118,7 @@ # end # end end - -end + end end # it 'returns http success with valid params' do @@ -167,7 +159,6 @@ # end # end - # end # end # end diff --git a/spec/controllers/ad_controller_spec.rb b/spec/controllers/ad_controller_spec.rb index 59b89a3..873815d 100644 --- a/spec/controllers/ad_controller_spec.rb +++ b/spec/controllers/ad_controller_spec.rb @@ -7,7 +7,6 @@ expect do post :create, params: { ad: { 'text' => 'Updated!', 'price' => 1010101, - 'advertiser_id' => 1010101, 'image' => Rack::Test::UploadedFile.new(File.join(Rails.root, 'spec/fixture/image.jpg')) } } end.to change(Ad, :count).by(1) expect(response).to redirect_to(ad_index_path) @@ -16,7 +15,6 @@ it "without text should render 'new'" do post :create, params: { ad: { 'text' => nil, 'price' => 1010101, - 'advertiser_id' => 1010101, 'image' => Rack::Test::UploadedFile.new(File.join(Rails.root, 'spec/fixture/image.jpg')) } } expect(response).to render_template :new end @@ -24,15 +22,6 @@ it "without price should render 'new'" do post :create, params: { ad: { 'text' => 'Updated!', 'price' => nil, - 'advertiser_id' => 1010101, - 'image' => Rack::Test::UploadedFile.new(File.join(Rails.root, 'spec/fixture/image.jpg')) } } - expect(response).to render_template :new - end - - it "without advertiser_id should render 'new'" do - post :create, params: { ad: { 'text' => 'Updated!', - 'price' => 1010101, - 'advertiser_id' => nil, 'image' => Rack::Test::UploadedFile.new(File.join(Rails.root, 'spec/fixture/image.jpg')) } } expect(response).to render_template :new end @@ -40,7 +29,6 @@ it "without image should render 'new'" do post :create, params: { ad: { 'text' => 'Updated!', 'price' => 1010101, - 'advertiser_id' => 1010101, 'image' => nil } } expect(response).to render_template :new end @@ -56,10 +44,8 @@ patch :update, params: { ad: { 'text' => nil, 'price' => nil, - 'advertiser_id' => nil, 'image' => nil - }, - id: @ad.id } + }, id: @ad.id } expect(response).to render_template :edit end @@ -73,11 +59,6 @@ expect(response).to redirect_to(ad_index_path) end - it 'with advertiser_id should be redirected' do - patch :update, params: { ad: { 'advertiser_id' => 1010101 }, id: @ad.id } - expect(response).to redirect_to(ad_index_path) - end - it 'with image should be redirected' do patch :update, params: { ad: { 'image' => Rack::Test::UploadedFile.new(File.join(Rails.root, 'spec/fixture/image.jpg')) }, id: @ad.id } expect(response).to redirect_to(ad_index_path) @@ -87,10 +68,8 @@ patch :update, params: { ad: { 'text' => 'Updated!', 'price' => 1010101, - 'advertiser_id' => 1010101, 'image' => Rack::Test::UploadedFile.new(File.join(Rails.root, 'spec/fixture/image.jpg')) - }, - id: @ad.id } + }, id: @ad.id } expect(response).to redirect_to(ad_index_path) end end @@ -106,11 +85,6 @@ expect(response).to render_template :edit end - it 'without advertiser_id should be rendered' do - patch :update, params: { ad: { 'advertiser_id' => nil }, id: @ad.id } - expect(response).to render_template :edit - end - it 'without image should be rendered' do patch :update, params: { ad: { 'image' => nil }, id: @ad.id } expect(response).to render_template :edit @@ -120,10 +94,8 @@ patch :update, params: { ad: { 'text' => nil, 'price' => nil, - 'advertiser_id' => nil, 'image' => nil - }, - id: @ad.id } + }, id: @ad.id } expect(response).to render_template :edit end end diff --git a/spec/factories/ads.rb b/spec/factories/ads.rb index 747dd8f..152680b 100644 --- a/spec/factories/ads.rb +++ b/spec/factories/ads.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true FactoryBot.define do factory :ad do - advertiser_id { 1 } image { Rack::Test::UploadedFile.new(File.join(Rails.root, 'spec/fixture/image.jpg')) } price { 1 } text { 'Test Text' } diff --git a/spec/features/path_spec.rb b/spec/features/path_spec.rb index aab875b..6224e16 100644 --- a/spec/features/path_spec.rb +++ b/spec/features/path_spec.rb @@ -16,7 +16,6 @@ it 'is Forms valid?' do fill_in 'ad_text', with: 'TextForm' fill_in 'ad_price', with: '1000' - fill_in 'ad_advertiser_id', with: '1' fill_in 'ad_text', with: 'TextForm' page.attach_file('ad_image', '/Users/hashimototakuma/Downloads/PNG_transparency_demonstration_1.png') end @@ -24,7 +23,6 @@ it 'is create action valid?' do fill_in 'ad_text', with: 'TextForm' fill_in 'ad_price', with: '1000' - fill_in 'ad_advertiser_id', with: '1' fill_in 'ad_text', with: 'TextForm' page.attach_file('ad_image', '/Users/hashimototakuma/Downloads/PNG_transparency_demonstration_1.png') @@ -53,7 +51,6 @@ visit 'ad/new' fill_in 'ad_text', with: 'TextForm' fill_in 'ad_price', with: '1000' - fill_in 'ad_advertiser_id', with: '1' fill_in 'ad_text', with: 'TextForm' page.attach_file('ad_image', '/Users/hashimototakuma/Downloads/PNG_transparency_demonstration_1.png') click_button '入稿' diff --git a/spec/models/ad_spec.rb b/spec/models/ad_spec.rb index 33218c4..569d53d 100644 --- a/spec/models/ad_spec.rb +++ b/spec/models/ad_spec.rb @@ -11,11 +11,6 @@ expect(@ad).not_to be_valid end - it 'without advertiser_id should be invalid' do - @ad.advertiser_id = nil - expect(@ad).not_to be_valid - end - it 'without image should be invalid' do @ad.image = nil expect(@ad).not_to be_valid diff --git a/test/fixtures/ads.yml b/test/fixtures/ads.yml index 3d12954..914bbff 100644 --- a/test/fixtures/ads.yml +++ b/test/fixtures/ads.yml @@ -1,13 +1,11 @@ # Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html one: - advertiser_id: 1 image: MyString price: MyString text: MyString two: - advertiser_id: 1 image: MyString price: MyString text: MyString