Skip to content

Implementation to aggregate statistical information #12 #128

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

Merged
merged 37 commits into from
Aug 21, 2017
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
e3fd895
Gem `faraday`
nalabjp Jul 29, 2017
db0afac
Draft implementaion of API client
nalabjp Jul 30, 2017
04f645c
Create a table named `event_histories`
nalabjp Jul 30, 2017
6ff7f21
Create a model named `EventHistory`
nalabjp Jul 30, 2017
453d28f
Create a table named `dojo_event_services`
nalabjp Jul 30, 2017
3669f2a
Create a model named `DojoEventService`
nalabjp Jul 30, 2017
b3d39bc
Add associations
nalabjp Jul 30, 2017
f3580de
Add timezone as `Asia/Tokyo`
nalabjp Jul 30, 2017
3bc50c1
Draft implementation of importing data via API to `event_histories` t…
nalabjp Jul 30, 2017
dfdc16c
Fix
nalabjp Aug 6, 2017
c93dfb8
Support multiple requests
nalabjp Aug 6, 2017
9eb61aa
Support a period for search conditions
nalabjp Aug 6, 2017
a4cb3ec
Support idempotency in re-running with same conditions
nalabjp Aug 6, 2017
d75d2fe
Rename a class name
nalabjp Aug 6, 2017
8ba4506
Use instance variables instead of constants
nalabjp Aug 6, 2017
b1a5ad1
Fix typo
nalabjp Aug 6, 2017
7197558
Create a task for the monthly aggregation
nalabjp Aug 6, 2017
983d828
Want to see some search results
nalabjp Aug 13, 2017
cf033f8
Rake task for searching
nalabjp Aug 13, 2017
70ef722
Fix typo
nalabjp Aug 13, 2017
5168879
Migrate database on Travis CI
nalabjp Aug 13, 2017
ba6fa8f
Run RSpec on Travis CI
nalabjp Aug 13, 2017
59fe612
Ruby 2.4.0 on Travis CI
nalabjp Aug 13, 2017
ec719e1
Cache bundler on Travis CI
nalabjp Aug 13, 2017
0024192
Shared context for API stubs
nalabjp Aug 13, 2017
b7e8213
Remove nested Hash key
nalabjp Aug 13, 2017
58bfebd
Add testing for `Statistics::Client`
nalabjp Aug 13, 2017
ab9736d
Add testing for `Statistics::Aggregation`
nalabjp Aug 13, 2017
740ca73
Remove records of the target period before executing the aggregation
nalabjp Aug 13, 2017
86b1f6c
Add description for `statistics:search`
nalabjp Aug 13, 2017
563bd68
Remove needless configuration
nalabjp Aug 14, 2017
f44f78e
Remove needless configuration
nalabjp Aug 14, 2017
0d72302
Fix association name
nalabjp Aug 14, 2017
754cf52
Remove needless argument
nalabjp Aug 20, 2017
0366800
Move to `.travis.yml`
nalabjp Aug 20, 2017
9adb714
Add `NOT NULL` restriction
nalabjp Aug 20, 2017
d392c68
Change the column type of `dojo_event_services.name` to `integer` fro…
nalabjp Aug 20, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ gem 'secure_headers'
# Rendering legal documents
gem 'kramdown'

gem 'faraday'
# TODO: Delete this if the following issue is fixed
# https://github.com/bundler/bundler/issues/5332
gem 'faraday_middleware', '0.10'
Expand Down
3 changes: 2 additions & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ DEPENDENCIES
bootstrap-sass
capybara
coffee-rails
faraday
faraday_middleware (= 0.10)
font-awesome-rails
jbuilder
Expand Down Expand Up @@ -315,4 +316,4 @@ RUBY VERSION
ruby 2.4.0p0

BUNDLED WITH
1.13.7
1.15.3
3 changes: 3 additions & 0 deletions app/models/dojo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ class Dojo < ApplicationRecord
NUM_OF_WHOLE_DOJOS = "1,250"
NUM_OF_JAPAN_DOJOS = Dojo.count.to_s

has_one :dojo_event_service
has_many :event_history
Copy link
Contributor

Choose a reason for hiding this comment

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

event_historiesかも?

Copy link
Member Author

Choose a reason for hiding this comment

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

おぉ😓
失礼しました...
fixed 0d72302


serialize :tags
default_scope -> { order(order: :asc) }
before_save { self.email = self.email.downcase }
Expand Down
3 changes: 3 additions & 0 deletions app/models/dojo_event_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class DojoEventService < ApplicationRecord
belongs_to :dojo
end
11 changes: 11 additions & 0 deletions app/models/event_history.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class EventHistory < ApplicationRecord
belongs_to :dojo

validates :dojo_name, presence: true
validates :service_name, presence: true, uniqueness: { scope: :event_id }
validates :service_group_id, presence: true
validates :event_id, presence: true
validates :event_url, presence: true, uniqueness: true, allow_nil: true
validates :participants, presence: true
validates :evented_at, presence: true
end
4 changes: 4 additions & 0 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,9 @@ class Application < Rails::Application
# Settings in config/environments/* take precedence over those specified here.
# Application configuration should go into files in config/initializers
# -- all .rb files in that directory are automatically loaded.

# Timezone
config.time_zone = 'Asia/Tokyo'
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

ENV['TZ'] = 'Asia/Tokyo'
Copy link
Contributor

Choose a reason for hiding this comment

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

Heroku、travis-ciのほうで設定した方がいいかも?

Copy link
Member Author

Choose a reason for hiding this comment

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

Herokuのenvはどなたかにお願いするとして、そうしましょうか。

Copy link
Member

Choose a reason for hiding this comment

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

Heroku の env 確認しましたが、既に入ってました ;) ✅

$ heroku config
...
TZ:                       Asia/Tokyo

Copy link
Member Author

Choose a reason for hiding this comment

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

ご確認ありがとうございます!😁

Copy link
Member Author

Choose a reason for hiding this comment

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

ご確認ありがとうございます!😁

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed 0366800

end
end
18 changes: 18 additions & 0 deletions db/migrate/20170730050122_create_event_histories.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
class CreateEventHistories < ActiveRecord::Migration[5.0]
def change
create_table :event_histories do |t|
t.references :dojo, foreign_key: true, index: true, null: false
t.string :dojo_name, null: false
t.string :service_name, null: false
t.integer :service_group_id, null: false
t.integer :event_id, null: false
t.string :event_url, unique: true, null: false
t.integer :participants, null: false
t.datetime :evented_at, null: false
t.timestamps

t.index [:service_name, :event_id], unique: true
t.index [:evented_at, :dojo_id]
end
end
end
11 changes: 11 additions & 0 deletions db/migrate/20170730114150_create_dojo_event_services.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class CreateDojoEventServices < ActiveRecord::Migration[5.0]
def change
create_table :dojo_event_services do |t|
t.references :dojo, foreign_key: true, index: true, null: false
t.string :name
Copy link
Contributor

Choose a reason for hiding this comment

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

NOT NULL制約ついてるservice_nameカラムにここの値渡されると思うので、NOT NULL制約付けておいたほうがよさそう?

Copy link
Member Author

Choose a reason for hiding this comment

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

connpassとdoorkeeper以外で開催されているところの扱いをどうすべきか判断がつかなかった(どれくらいあるのかもわからなかった)のでなんとなく制約を付けませんでした。
統計情報の集計でサポートしているconnpassとdoorkeeperだけにしか使用しないということであれば、ご指摘のとおり制約を入れておいたほうが良いと思いますが、
どうしましょうか?🤔

Copy link
Member

Choose a reason for hiding this comment

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

統計情報の集計でサポートしているconnpassとdoorkeeperだけにしか使用しないということであれば、ご指摘のとおり制約を入れておいたほうが良いと思いますが、どうしましょうか?🤔

悩ましいですね🤔 NOT NULL 制約って、「もし必要になったら外す」みたいな運用だとマズイですかね? 😅 それで問題なければ、制約つけておいても良いのかなと思いました ><

Copy link
Member Author

Choose a reason for hiding this comment

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

あとから外しても問題ないなので、制約を付けるようにします。

Copy link
Contributor

Choose a reason for hiding this comment

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

event_historiesにこのあたりの情報をコピーして保存してるように見えるので、dojo_event_servicesでのnamegroup_idカラムの制約はevent_historiesservice_nameservice_group_idカラムの制約と合わせたほうが良いんじゃないかなーと思っています。

Copy link
Member Author

@nalabjp nalabjp Aug 20, 2017

Choose a reason for hiding this comment

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

fixed 9adb714

t.string :url
Copy link
Contributor

Choose a reason for hiding this comment

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

つかわれてなさそう...?

Copy link
Member Author

Choose a reason for hiding this comment

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

connpass / doorkeeperの各dojoごとのurlを入れようかと思って用意しています。
dojo_event_serivceのレコードを作成するのが手作業の想定になっているので、今のところ使われていませんね。😅
テーブル定義としても不要そうというご指摘であれば削除してしまおうかと思います。

Copy link
Contributor

Choose a reason for hiding this comment

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

なるほど、イベントページのトップページへのリンク等表示したいこともありそうなので、あったほうが良さそうですね。

t.integer :group_id
Copy link
Contributor

Choose a reason for hiding this comment

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

serviceによってはgroup_idが必要ないサービスもあるのでしょうか 🤔
今のところ対応してるサービスがどれもgroup_id必須なら、NOT NULL制約つけてもよさそうです。

Copy link
Member Author

Choose a reason for hiding this comment

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

そうですね、同感です。
nameのところのコメント( #128 (comment) )と同様の理由なので、
このテーブルをどう扱うかによって判断したい気持ちです。

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed 9adb714

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

ActiveRecord::Schema.define(version: 20161217063010) do
ActiveRecord::Schema.define(version: 20170730114150) do

create_table "dojo_event_services", force: :cascade do |t|
t.integer "dojo_id", null: false
t.string "name"
t.string "url"
t.integer "group_id"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["dojo_id"], name: "index_dojo_event_services_on_dojo_id"
end

create_table "dojos", force: :cascade do |t|
t.string "name"
Expand All @@ -24,4 +34,20 @@
t.datetime "updated_at", null: false
end

create_table "event_histories", force: :cascade do |t|
t.integer "dojo_id", null: false
t.string "dojo_name", null: false
t.string "service_name", null: false
t.integer "service_group_id", null: false
t.integer "event_id", null: false
t.string "event_url", null: false
t.integer "participants", null: false
t.datetime "evented_at", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["dojo_id"], name: "index_event_histories_on_dojo_id"
t.index ["evented_at", "dojo_id"], name: "index_event_histories_on_evented_at_and_dojo_id"
t.index ["service_name", "event_id"], name: "index_event_histories_on_service_name_and_event_id", unique: true
end

end
3 changes: 3 additions & 0 deletions lib/statistics.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module Statistics; end

require_relative 'statistics/client'
69 changes: 69 additions & 0 deletions lib/statistics/client.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
module Statistics
class Client
class_attribute :debug
self.debug = false

def initialize(endpoint, &block)
@conn = connection_for(endpoint, &block)
end

def get(path, params)
@conn.get(path, params).body
end

private

def connection_for(endpoint, &block)
Copy link
Contributor

Choose a reason for hiding this comment

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

&blockつかわれてなさそう

Copy link
Member Author

Choose a reason for hiding this comment

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

こちらで使っています〜
db0afac#diff-eaf50acb3e83c4f729603033a19f20c4R54

Copy link
Contributor

Choose a reason for hiding this comment

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

blockの変数自体は使ってなさそうに見えました。

Copy link
Member Author

Choose a reason for hiding this comment

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

あ、確かに。
失礼しました。

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed 754cf52

Faraday.new(endpoint) do |f|
f.response :logger if self.debug
f.response :json, :content_type => /\bjson$/
f.response :raise_error

yield f if block_given?

f.adapter Faraday.default_adapter
end
end

class Connpass
ENDPOINT = 'https://connpass.com/api/v1'.freeze

def initialize
@client = Client.new(ENDPOINT)
end

def fetch_series_id(**params)
Copy link
Contributor

Choose a reason for hiding this comment

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

ハッシュ受け取るだけなら単にparamsでよさそうな気も

Copy link
Member Author

Choose a reason for hiding this comment

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

インタフェースが変わって消えてしまいました〜

@client.get('event/', params.merge(count: 1))
.fetch('events')
.first
Copy link
Contributor

Choose a reason for hiding this comment

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

配列の1番目取るなら.dig(0, 'series', 'id')でよさそう

Copy link
Contributor

Choose a reason for hiding this comment

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

配列の要素がないときエラーにしたい感じなのかな?

[4] pry(main)> [{foo: 1}].dig(0, :foo)
=> 1
[5] pry(main)> [].dig(0, :foo)
=> nil
[6] pry(main)> [].first.dig(:foo)
NoMethodError: undefined method `dig' for nil:NilClass
from (pry):6:in `__pry__'

Copy link
Member Author

Choose a reason for hiding this comment

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

おぉ、indexも引数に渡せるからそういう使い方できるんですね。💡
勉強になりました。

.dig('series', 'id')
end

def fetch_events(series_id:)
@client.get('event/', series_id: series_id, count: 100)
.fetch('events')
end
end

class Doorkeeper
ENDPOINT = 'https://api.doorkeeper.jp'.freeze
DEFAULT_SINCE = Date.parse('2010-07-01')

def initialize
@client = Client.new(ENDPOINT) do |c|
c.authorization(:Bearer, ENV.fetch('DOORKEEPER_API_TOKEN'))
end
end

def fetch_group_id(keyword:)
@client.get('events', q: keyword, since: DEFAULT_SINCE)
.first
Copy link
Contributor

Choose a reason for hiding this comment

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

ここもdig(0, 'event', 'group')でよさそう

.dig('event', 'group')
end

def fetch_events(group_id:, offset: 1, since: DEFAULT_SINCE)
@client.get("groups/#{group_id}/events", offset: offset, since: since)
end
end
end
end