Feature web api #9

HashimotoLogly · 2019-06-21T02:04:55Z

OJTのAPI開発(view,click)を行いました。
まだ細かい部分（いらないファイル等）は修正できておりませんが随時修正していきます
RSpecは随時追加していきます。

対応issue #6

tmk-hsn

ひとまずコメントしたので修正お願い

tmk-hsn · 2019-06-21T02:08:26Z

app/controllers/ad_api_controller.rb

+
+class AdApiController < ApplicationController
+  def view
+    array=[]


イコールの左右は空けること

tmk-hsn · 2019-06-21T02:08:48Z

app/controllers/ad_api_controller.rb

+    array=[]
+
+    target_ids = Ad.pluck(:id).sample( params[:count].to_i)
+    ads=Ad.find(target_ids)


イコールの左右は空けること
同じことを言われないように注意すること

tmk-hsn · 2019-06-21T02:17:43Z

db/migrate/20190614041856_create_repos.rb

@@ -0,0 +1,15 @@
+class CreateRepos < ActiveRecord::Migration[5.2]
+  def change
+    create_table :repos do |t|


カラムを以下の順番に変更して。

t.integer :ad_id,null: false t.integer :adspot_id, null: false t.integer :click, null: false, default: 0 t.integer :imp, null: false, default: 0 t.integer :cv, null: false, default: 0 t.integer :price, null: false, default: 0

created_at, updated_atを足してほしい

tmk-hsn · 2019-06-21T02:32:40Z

db/schema.rb

-    t.integer 'cv'
+ActiveRecord::Schema.define(version: 2019_06_14_041856) do
+
+  create_table "ads", force: :cascade do |t|


adsテーブルってここで作るんだっけ？

tmk-hsn · 2019-06-21T02:33:18Z

db/schema.rb

+    t.datetime "updated_at", null: false
+  end
+
+  create_table "repos", force: :cascade do |t|


テーブル名を変更してほしい

カラムの並び順を変えてほしい

created_at, updated_atを足してほしい

tmk-hsn · 2019-06-21T02:37:15Z

app/controllers/ad_api_controller.rb

+
+  def click
+
+    if repo = Repo.find_by(ad_id:  params[:ad_id], adspot_id:  params[:adspot_id])


スペース2つ分空いてる？から1つ分に変更して

tmk-hsn · 2019-06-21T02:37:23Z

app/controllers/ad_api_controller.rb

+    if repo = Repo.find_by(ad_id:  params[:ad_id], adspot_id:  params[:adspot_id])
+
+      repo.click += 1
+      repo.totalcost +=  Ad.find( params[:ad_id]).price


不要スペースは削除で

tmk-hsn · 2019-06-21T02:37:35Z

app/controllers/ad_api_controller.rb

+
+
+  private
+  def count_p


このprivateメソッド使ってる？

tmk-hsn · 2019-06-21T02:39:04Z

app/controllers/ad_api_controller.rb

+      p repo.totalcost
+
+    else
+      render status: 500, json: { status: 500, message: 'Ad was not existed! ' }


レスポンス500ではない気がするので、改めて検討してみて

tmk-hsn · 2019-06-21T02:39:43Z

db/migrate/20190604073026_add_report_column_to_ads.rb

@@ -1,8 +0,0 @@
-# frozen_string_literal: true
-class AddReportColumnToAds < ActiveRecord::Migration[5.2]


この変更は入稿でしなければいけないのでは？

tmk-hsn · 2019-06-21T08:08:28Z

db/schema.rb


 ActiveRecord::Schema.define(version: 2019_06_14_041856) do
-  create_table 'ads', force: :cascade do |t|  # reportsテーブルのtotalpriceにpriceを加算する為に使用
+  create_table 'ads', force: :cascade do |t|  # reportsテーブルのtotalpriceにpriceを加算する為に使用しているのでここでも使っています


totalpriceではなく、priceで良いです。
日付が入っているので、その日付のtotal priceであることが明らかだからです。
また、totalpriceとつけたい場合は、total_priceといったように単語の間に区切りを入れましょう。

tmk-hsn · 2019-06-21T08:23:14Z

app/controllers/ad_api_controller.rb


+# frozen_string_literal: true
 class AdApiController < ApplicationController
  def view


view はメソッド名としてよくない（viewsというディレクトリが存在するので初見の人が混乱する）ので、register_impとかに変えてほしい

tmk-hsn · 2019-06-21T08:25:39Z

app/controllers/ad_api_controller.rb

    render json: array
  end

  def click


clickをどうするのか、明確にしたメソッド名にしてほしい

tmk-hsn · 2019-06-21T08:26:47Z

app/controllers/ad_api_controller.rb

-  def count_p
-
+    report.click += 1
+    report.totalcost += Ad.find(params[:ad_id]).price


totalcostって多分登録できてないよ

tmk-hsn · 2019-06-21T08:27:37Z

spec/controllers/ad_api_controller_spec.rb

-  describe "GET #view" do
-    it "returns http success" do
+  end
+  describe 'GET #view' do


メソッド名を変えよう

tmk-hsn · 2019-06-21T08:27:48Z

spec/controllers/ad_api_controller_spec.rb


-  describe "GET #click" do
-    it "returns http success" do
+  describe 'GET #click' do


メソッド名を変えよう

tmk-hsn · 2019-06-27T08:57:07Z

app/controllers/ad_api_controller.rb

@@ -0,0 +1,40 @@
+


不要な行は削除しよう

tmk-hsn · 2019-06-27T09:18:13Z

app/controllers/ad_api_controller.rb

+
+# frozen_string_literal: true
+class AdApiController < ApplicationController
+  def view_make_report


本来であれば1メソッドで1つの役割を持たせるべきで、一言で説明できないメソッドにすべきではないです。
今回は時間がないのでそこまで対応せず大丈夫ですが、配属後はこうした部分にも気をつけて開発をしてみてくださいね。

tmk-hsn · 2019-06-27T09:18:22Z

app/controllers/ad_api_controller.rb

+    render json: array
+  end
+
+  def click_make_report


メソッド名変更で

tmk-hsn · 2019-06-27T09:19:32Z

config/routes.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 Rails.application.routes.draw do
+  get '/view' => 'ad_api#view_make_report'


routing変更

tmk-hsn · 2019-06-27T09:19:39Z

config/routes.rb

 # frozen_string_literal: true
 Rails.application.routes.draw do
+  get '/view' => 'ad_api#view_make_report'
+  get '/click' => 'ad_api#click_make_report'


routing変更

tmk-hsn · 2019-06-27T09:22:04Z

spec/controllers/ad_api_controller_spec.rb

+          ad3_ispresent = false
+
+          jsons.each do |json|
+            if json['ad_id'] == @ad1.id


rspecではif文は使わないで

tmk-hsn · 2019-06-27T09:22:17Z

spec/controllers/ad_api_controller_spec.rb

+    end
+  end
+
+  describe 'GET #click_make_report' do


メソッド名変更で

tmk-hsn · 2019-06-27T09:24:56Z

spec/controllers/ad_api_controller_spec.rb

+      expect(response).to have_http_status(:success)
+    end
+
+    describe 'Report' do


describeの囲いは統一してほしい

if, unlessで別れていたら都度テストケースを書いてほしい

tmk-hsn · 2019-06-27T09:25:26Z

spec/helpers/ad_api_helper_spec.rb

+# frozen_string_literal: true
+require 'rails_helper'
+
+# Specs in this file have access to a helper object that includes


消してOK

tmk-hsn · 2019-06-27T09:32:46Z

app/controllers/ad_api_controller.rb

+# frozen_string_literal: true
+class AdApiController < ApplicationController
+  def view_make_report
+    array = []


arrayは変えて

HashimotoLogly added 5 commits June 14, 2019 13:06

make api routes

b57d7ff

add imp,click counter on api_controller

9fc95ea

add errorlog if ad.id was matched nothing

159cd36

fix Db tables

ac3f826

add api html.erb

a543d86

tmk-hsn suggested changes Jun 21, 2019

View reviewed changes

HashimotoLogly added 7 commits June 21, 2019 15:28

fix ad_api

adfd2ca

fix model name

0fa59bc

fix db

592f6e1

fix model name

fc6c62b

autocorrected by rubocop

3675ac0

fix model saving

1de31b3

add comment

792c80f

tmk-hsn suggested changes Jun 21, 2019

View reviewed changes

tmk-hsn reviewed Jun 21, 2019

View reviewed changes

HashimotoLogly added 2 commits June 21, 2019 17:45

change totalprice to price

9f42631

fix db column type

a8952d5

HashimotoLogly force-pushed the feature_webAPI branch from f1ecd9b to a8952d5 Compare June 24, 2019 02:11

HashimotoLogly added 10 commits June 24, 2019 11:20

add travel_to method for testing rspec

53645a0

add view rspec

7445b15

add click rspec

69841ea

add_ media.site.htm

26e04f9

change action name(view,click)

7296024

add rspec

39d598f

rubocop --auto-correct

299dc5a

change html name corresponding to action names

c2c9734

delete unused files

0a6a8c2

rename files

7b9e86a

HashimotoLogly added 2 commits June 27, 2019 17:52

delete unused files

11e2cc2

delete comment

cf3dc25

tmk-hsn suggested changes Jun 27, 2019

View reviewed changes

fix action names

2e9eefd


		def click

		if repo = Repo.find_by(ad_id: params[:ad_id], adspot_id: params[:adspot_id])

		@@ -1,8 +0,0 @@
		# frozen_string_literal: true
		class AddReportColumnToAds < ActiveRecord::Migration[5.2]

Feature web api #9

Are you sure you want to change the base?

Feature web api #9

Uh oh!

Conversation

HashimotoLogly commented Jun 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tmk-hsn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmk-hsn Jun 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HashimotoLogly commented Jun 21, 2019 •

edited

Loading

tmk-hsn Jun 21, 2019 •

edited

Loading