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 30 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
8 changes: 7 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,10 @@ deploy:
run:
- "bundle exec rails db:migrate"
- "bundle exec rails dojos:update_db_by_yaml"

rvm:
- 2.4.0
Copy link
Contributor

Choose a reason for hiding this comment

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

travisのログみた感じだと.ruby-versionがあればここ指定する必要はないみたいです。

$ rvm use $(< .ruby-version) --install --binary --fuzzy

https://travis-ci.org/coderdojo-japan/coderdojo.jp/jobs/260468747

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 563bd68

Copy link
Contributor

Choose a reason for hiding this comment

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

複数バージョンのRubyに対してテスト実行したいときは便利そうですが今は使ってるRuby1つしかないので消しちゃって大丈夫そうですね 👍

cache:
- bundler
script:
- RAILS_ENV=test bundle exec rake db:migrate --trace
Copy link
Contributor

Choose a reason for hiding this comment

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

travisだとデフォルトでRAILS_ENV=testなのでここで環境変数指定する必要はなさそうです
https://docs.travis-ci.com/user/environment-variables/#Default-Environment-Variables

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 f44f78e

- bundle exec rake
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
4 changes: 4 additions & 0 deletions lib/statistics.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module Statistics; end

require_relative 'statistics/client'
require_relative 'statistics/aggregation'
66 changes: 66 additions & 0 deletions lib/statistics/aggregation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
module Statistics
class Aggregation
class << self
def run(date:)
cnps_dojos = Dojo.joins(:dojo_event_service).where(dojo_event_services: { name: 'connpass' }).to_a
Copy link
Contributor

Choose a reason for hiding this comment

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

ここでnameに渡している文字列、マジックナンバーになっていますね。このカラムをEnumか何かにしたいですね

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 d392c68

drkp_dojos = Dojo.joins(:dojo_event_service).where(dojo_event_services: { name: 'doorkeeper' }).to_a

Connpass.run(cnps_dojos, date)
Doorkeeper.run(drkp_dojos, date)
end
end

class Connpass
class << self
def run(dojos, date)
cnps = Client::Connpass.new
params = {
yyyymm: "#{date.year}#{date.month}"
}

dojos.each do |dojo|
cnps.fetch_events(params.merge(series_id: dojo.dojo_event_service.group_id)).each do |e|
next unless e.dig('series', 'id') == dojo.dojo_event_service.group_id

EventHistory.create!(dojo_id: dojo.id,
dojo_name: dojo.name,
service_name: dojo.dojo_event_service.name,
service_group_id: dojo.dojo_event_service.group_id,
event_id: e['event_id'],
event_url: e['event_url'],
participants: e['accepted'],
evented_at: Time.zone.parse(e['started_at']))
end
end
end
end
end

class Doorkeeper
class << self
def run(dojos, date)
drkp = Client::Doorkeeper.new
params = {
since_at: date.beginning_of_month,
until_at: date.end_of_month
}

dojos.each do |dojo|
drkp.fetch_events(params.merge(group_id: dojo.dojo_event_service.group_id)).each do |e|
next unless e['group'] == dojo.dojo_event_service.group_id

EventHistory.create!(dojo_id: dojo.id,
dojo_name: dojo.name,
service_name: dojo.dojo_event_service.name,
service_group_id: dojo.dojo_event_service.group_id,
event_id: e['id'],
event_url: e['public_url'],
participants: e['participants'],
evented_at: Time.zone.parse(e['starts_at']))
end
end
end
end
end
end
end
105 changes: 105 additions & 0 deletions lib/statistics/client.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
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.class.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 search(keyword:)
@client.get('event/', { keyword: keyword, count: 100 })
end

def fetch_events(series_id:, yyyymm: nil)
params = {
series_id: series_id,
start: 1,
count: 100
}
params[:ym] = yyyymm if yyyymm
events = []

loop do
part = @client.get('event/', params)

break if part['results_returned'].zero?

events.push(*part.fetch('events'))
Copy link
Contributor

Choose a reason for hiding this comment

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

こことL99、count: 100なので大丈夫そうですがevents個数がものすごく多いとスタック溢れそうですね

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はcount: 100がAPIの上限値なのでまぁ良いかなぁと判断しました。
doorkeeperは25固定です。


break if part.size < params[:count]

break if params[:start] + params[:count] > part['results_available']

params[:start] += params[:count]
end

events
end
end

class Doorkeeper
ENDPOINT = 'https://api.doorkeeper.jp'.freeze

def initialize
@client = Client.new(ENDPOINT) do |c|
c.authorization(:Bearer, ENV.fetch('DOORKEEPER_API_TOKEN'))
end
@default_since = Time.zone.parse('2010-07-01')
@default_until = Time.zone.now.end_of_day
end

def search(keyword:)
@client.get('events', q: keyword, since: @default_since)
end

def fetch_events(group_id:, since_at: @default_since, until_at: @default_until)
params = {
page: 1,
since: since_at,
until: until_at
}
events = []

loop do
part = @client.get("groups/#{group_id}/events", params)

break if part.size.zero?

events.push(*part.map { |e| e['event'] })

break if part.size < 25 # 25 items / 1 request

params[:page] += 1
end

events
end
end
end
end
36 changes: 36 additions & 0 deletions lib/tasks/statistics.rake
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
require_relative '../statistics.rb'
Copy link
Contributor

Choose a reason for hiding this comment

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

Statisticsはこのrakeタスク以外ではつかわないのでlib以下においてるかんじですかねー?

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.

なるほど 🦀


namespace :statistics do
desc '月次のイベント履歴を集計します'
task :aggregation, [:yyyymm] => :environment do |tasks, args|
date = Time.current.prev_month.beginning_of_month
if args[:yyyymm].present?
date = %w(%Y%m %Y/%m %Y-%m).map do |fmt|
begin
Time.zone.strptime(args[:yyyymm], fmt)
rescue ArgumentError
end
end.compact.first
end

raise ArgumentError, "Invalid format: #{args[:yyyymm]}" if date.nil?


EventHistory.where(evented_at: date.beginning_of_month..date.end_of_month).delete_all
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.

あまり無いとは思いますがたまにやり直したいケースがあったりするのでそのようにしました。

Copy link
Contributor

Choose a reason for hiding this comment

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

いいと思います!


Statistics::Aggregation.run(date: date)
end

desc 'キーワードからイベント情報を検索します'
task :search, [:keyword] => :environment do |tasks, args|
raise ArgumentError, 'Require the keyword' if args[:keyword].nil?

require 'pp'

puts 'Searching Connpass'
pp Statistics::Client::Connpass.new.search(keyword: args[:keyword])

puts 'Searching Doorkeeper'
pp Statistics::Client::Doorkeeper.new.search(keyword: args[:keyword])
end
end
33 changes: 33 additions & 0 deletions spec/lib/statistics/aggregation_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
require 'rails_helper'
require 'statistics'

RSpec.describe Statistics::Aggregation do
include_context 'Use stubs for Faraday'

before(:all) do
Dojo.delete_all
DojoEventService.delete_all
EventHistory.delete_all
end

after do
Dojo.delete_all
DojoEventService.delete_all
EventHistory.delete_all
end

describe '.run' do
before do
d1 = Dojo.create(name: 'Dojo1', email: 'info@dojo1.com', description: 'CoderDojo1', tags: %w(CoderDojo1), url: 'https://dojo1.com')
d2 = Dojo.create(name: 'Dojo2', email: 'info@dojo2.com', description: 'CoderDojo2', tags: %w(CoderDojo2), url: 'https://dojo2.com')
DojoEventService.create(dojo_id: d1.id, name: 'connpass', group_id: 9876)
DojoEventService.create(dojo_id: d2.id, name: 'doorkeeper', group_id: 5555)
end

subject { Statistics::Aggregation.run(date: Time.current) }

it do
expect{ subject }.to change{EventHistory.count}.from(0).to(2)
end
end
end
Loading