-
-
Notifications
You must be signed in to change notification settings - Fork 108
直近のイベント情報を集計するtaskの追加(仕切り直し) #459
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
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
33a16a6
近日開催イベント情報を収集する upcoming_events:aggregation のタスクを追加
chicaco 8250708
rake upcoming_events:aggregation の説明ドキュメントを追加
chicaco 41cc1cb
upcoming_events:aggregation のタスクのリファクタ
chicaco 79c3150
レビュー指摘事項対応
chicaco b7e0ec4
レビュー指摘事項対応(2)
chicaco 6bd5817
Merge commit '4f51d40cf65debbdbd42086cfff7781a09bc0a05' into add_upco…
chicaco 306a6b1
Merge commit '437c9018975561adab6fefd0d7657ee91090677b' into add_upco…
chicaco b3bc088
レビュー指摘事項対応(3)
chicaco File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
class UpcomingEvent < ApplicationRecord | ||
belongs_to :dojo_event_service | ||
|
||
validates :service_name, presence: true, uniqueness: { scope: :event_id } | ||
validates :event_id, presence: true | ||
validates :event_url, presence: true | ||
validates :event_at, presence: true | ||
validates :participants, presence: true | ||
|
||
scope :for, ->(service) { where(dojo_event_service: DojoEventService.for(service)) } | ||
scope :until, ->(date) { where('event_at < ?', date.beginning_of_day) } | ||
end |
19 changes: 19 additions & 0 deletions
19
db/migrate/20190526151359_mod_columns_to_upcoming_event.rb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
class ModColumnsToUpcomingEvent < ActiveRecord::Migration[5.1] | ||
def up | ||
remove_index :upcoming_events, :event_at | ||
|
||
add_column :upcoming_events, :service_name, :string, null: false | ||
add_column :upcoming_events, :participants, :integer, null: false | ||
|
||
add_index :upcoming_events, [:service_name, :event_id], :unique => true | ||
end | ||
|
||
def down | ||
remove_index :upcoming_events, [:service_name, :event_id] | ||
|
||
remove_column :upcoming_events, :service_name, :string, null: false | ||
remove_column :upcoming_events, :participants, :integer, null: false | ||
|
||
add_index :upcoming_events, :event_at, name: "index_upcoming_events_on_event_at" | ||
end | ||
end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
# rake upcoming_events:aggregation[provider] | ||
|
||
## 概要 | ||
|
||
近日開催(2ヶ月分)のイベント情報を収集する | ||
|
||
## 引数 | ||
|
||
|引数名|型|必須|説明| | ||
|--|--|--|--| | ||
|provider|string|(省略可)|集計対象プロバイダ| | ||
|
||
## 説明 | ||
|
||
過去(昨日分まで)のイベント情報を削除し、本日から 2 ヶ月後までのイベント情報を収集する。 | ||
|
||
provider が指定されたとき、指定プロバイダに対してのみ集計を行う。 | ||
|
||
+ provider には、connpass, doorkeeper, facebook が指定可能。ただし、現時点で facebook は収集対象外のため処理を skip する。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
require_relative '../upcoming_events.rb' | ||
|
||
namespace :upcoming_events do | ||
desc '指定期間/プロバイダのイベント履歴を集計します' | ||
task :aggregation, [:provider] => :environment do |tasks, args| | ||
UpcomingEvent.transaction do | ||
UpcomingEvents::Aggregation.new(args).run | ||
end | ||
end | ||
end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
module UpcomingEvents; end | ||
|
||
require_relative 'upcoming_events/tasks' | ||
require_relative 'upcoming_events/aggregation' | ||
require_relative 'event_service' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
module UpcomingEvents | ||
class Aggregation | ||
def initialize(args) | ||
@from = Time.zone.today | ||
@to = @from + 2.months | ||
@provider = args[:provider] | ||
# NOTE: 対象は一旦収集可能な connpass, doorkeeper のみにする | ||
@externals = fetch_dojos(@provider) | ||
end | ||
|
||
def run | ||
puts "UpcomingEvents aggregate" | ||
with_notifying do | ||
delete_upcoming_events | ||
execute | ||
end | ||
end | ||
|
||
private | ||
|
||
def fetch_dojos(provider) | ||
base_providers = DojoEventService::EXTERNAL_SERVICES - [:facebook] | ||
services = if provider.blank? | ||
# 全プロバイダ対象 | ||
base_providers | ||
elsif base_providers.include?(provider.to_sym) | ||
[provider.to_sym] | ||
end | ||
return [] unless services | ||
find_dojos_by(services) | ||
end | ||
|
||
def find_dojos_by(services) | ||
services.each.with_object({}) do |name, hash| | ||
hash[name] = Dojo.eager_load(:dojo_event_services).where(dojo_event_services: { name: name }).to_a | ||
end | ||
end | ||
|
||
def with_notifying | ||
yield | ||
Notifier.notify_success(@provider) | ||
rescue => e | ||
Notifier.notify_failure(@provider, e) | ||
end | ||
|
||
def delete_upcoming_events | ||
UpcomingEvent.until(@from).delete_all | ||
end | ||
|
||
def execute | ||
target_period = @from..@to | ||
@externals.each do |kind, list| | ||
puts "Aggregate of #{kind}" | ||
"UpcomingEvents::Tasks::#{kind.to_s.camelize}".constantize.new(list, target_period).run | ||
end | ||
end | ||
|
||
class Notifier | ||
class << self | ||
def notify_success(provider) | ||
notify("近日開催イベント情報#{provider_info(provider)}を収集しました") | ||
end | ||
|
||
def notify_failure(provider, exception) | ||
notify("近日開催イベント情報の収集#{provider_info(provider)}でエラーが発生しました\n#{exception.message}\n#{exception.backtrace.join("\n")}") | ||
end | ||
|
||
private | ||
|
||
def provider_info(provider) | ||
provider ? "(#{provider})" : nil | ||
end | ||
|
||
def idobata_hook_url | ||
return @idobata_hook_url if defined?(@idobata_hook_url) | ||
@idobata_hook_url = ENV['IDOBATA_HOOK_URL'] | ||
end | ||
|
||
def notifierable? | ||
idobata_hook_url.present? | ||
end | ||
|
||
def notify(msg) | ||
$stdout.puts msg | ||
puts `curl --data-urlencode "source=#{msg}" -s #{idobata_hook_url} -o /dev/null -w "idobata: %{http_code}"` if notifierable? | ||
end | ||
end | ||
end | ||
end | ||
end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
module UpcomingEvents | ||
module Tasks | ||
end | ||
end | ||
|
||
require_relative 'tasks/connpass' | ||
require_relative 'tasks/doorkeeper' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
module UpcomingEvents | ||
module Tasks | ||
class Connpass | ||
def initialize(dojos, period) | ||
@client = EventService::Providers::Connpass.new | ||
@dojos = dojos | ||
@params = build_params(period) | ||
end | ||
|
||
def run | ||
@dojos.each do |dojo| | ||
dojo.dojo_event_services.for(:connpass).each do |dojo_event_service| | ||
@client.fetch_events(@params.merge(series_id: dojo_event_service.group_id)).each do |e| | ||
next unless e.dig('series', 'id').to_s == dojo_event_service.group_id | ||
|
||
record = dojo_event_service.upcoming_events.find_or_initialize_by(event_id: e['event_id']) | ||
record.update!(service_name: dojo_event_service.name, | ||
event_url: e['event_url'], | ||
event_at: Time.zone.parse(e['started_at']), | ||
participants: e['accepted']) | ||
end | ||
end | ||
end | ||
end | ||
|
||
private | ||
|
||
def build_params(period) | ||
yyyymmdd = [] | ||
yyyymm = [] | ||
|
||
st_date = period.first | ||
ed_date = period.last | ||
|
||
date = period.first | ||
while date <= ed_date | ||
if date.day == 1 && date.end_of_month <= ed_date | ||
yyyymm << date.strftime('%Y%m') | ||
date += 1.month | ||
else | ||
yyyymmdd << date.strftime('%Y%m%d') | ||
date += 1.day | ||
end | ||
end | ||
|
||
{ | ||
yyyymmdd: yyyymmdd, | ||
yyyymm: yyyymm | ||
} | ||
end | ||
end | ||
end | ||
end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
module UpcomingEvents | ||
module Tasks | ||
class Doorkeeper | ||
def initialize(dojos, period) | ||
@client = EventService::Providers::Doorkeeper.new | ||
@dojos = dojos | ||
@params = build_params(period) | ||
end | ||
|
||
def run | ||
@dojos.each do |dojo| | ||
dojo.dojo_event_services.for(:doorkeeper).each do |dojo_event_service| | ||
@client.fetch_events(@params.merge(group_id: dojo_event_service.group_id)).each do |e| | ||
next unless e['group'].to_s == dojo_event_service.group_id | ||
|
||
record = dojo_event_service.upcoming_events.find_or_initialize_by(event_id: e['id']) | ||
record.update!(service_name: dojo_event_service.name, | ||
event_url: e['public_url'], | ||
participants: e['participants'], | ||
event_at: Time.zone.parse(e['starts_at'])) | ||
end | ||
end | ||
end | ||
end | ||
|
||
private | ||
|
||
def build_params(period) | ||
{ | ||
since_at: period.first.beginning_of_day, | ||
until_at: period.last.end_of_day | ||
} | ||
end | ||
end | ||
end | ||
end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
require 'factory_bot' | ||
|
||
FactoryBot.define do | ||
factory :upcoming_event do | ||
service_name { :connpass } | ||
event_id { '1234' } | ||
event_url { 'http:/www.aaa.com/events/1224' } | ||
event_at { '2019-05-01 10:00'.in_time_zone } | ||
participants { 1 } | ||
end | ||
end | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
require 'rails_helper' | ||
require 'upcoming_events' | ||
|
||
RSpec.describe UpcomingEvents::Aggregation do | ||
include_context 'Use stubs UpcomingEvents for Connpass' | ||
include_context 'Use stubs UpcomingEvents for Doorkeeper' | ||
|
||
describe '.run' do | ||
before do | ||
@d1 = create(:dojo, name: 'Dojo1', email: 'info@dojo1.com', description: 'CoderDojo1', tags: %w(CoderDojo1), url: 'https://dojo1.com') | ||
@d2 = create(:dojo, name: 'Dojo2', email: 'info@dojo2.com', description: 'CoderDojo2', tags: %w(CoderDojo2), url: 'https://dojo2.com') | ||
@es1 = create(:dojo_event_service, dojo_id: @d1.id, name: :connpass, group_id: 9876) | ||
@es2 = create(:dojo_event_service, dojo_id: @d2.id, name: :doorkeeper, group_id: 5555) | ||
end | ||
|
||
it 'プロバイダ指定なし' do | ||
expect{ UpcomingEvents::Aggregation.new({}).run }.to change{ UpcomingEvent.count }.from(0).to(3) | ||
end | ||
|
||
it 'プロバイダ指定(connpass)' do | ||
expect{ UpcomingEvents::Aggregation.new(provider: 'connpass').run }.to change{ UpcomingEvent.count }.from(0).to(1) | ||
end | ||
|
||
it 'プロバイダ指定(doorkeeper)' do | ||
expect{ UpcomingEvents::Aggregation.new(provider: 'doorkeeper').run }.to change{ UpcomingEvent.count }.from(0).to(2) | ||
end | ||
|
||
it '昨日分までは削除' do | ||
create(:upcoming_event, dojo_event_service_id: @es1.id, service_name: 'connpass', event_id: '1111', event_at: "#{Time.zone.today - 3.days} 13:00:00".in_time_zone) | ||
create(:upcoming_event, dojo_event_service_id: @es1.id, service_name: 'connpass', event_id: '2222', event_at: "#{Time.zone.today - 2.days} 14:00:00".in_time_zone) | ||
create(:upcoming_event, dojo_event_service_id: @es1.id, service_name: 'connpass', event_id: '3333', event_at: "#{Time.zone.today - 1.days} 15:00:00".in_time_zone) | ||
create(:upcoming_event, dojo_event_service_id: @es2.id, service_name: 'doorkeeper', event_id: '4444', event_at: "#{Time.zone.today - 2.days} 10:00:00".in_time_zone) | ||
create(:upcoming_event, dojo_event_service_id: @es2.id, service_name: 'doorkeeper', event_id: '5555', event_at: "#{Time.zone.today - 1.days} 11:00:00".in_time_zone) | ||
|
||
expect{ UpcomingEvents::Aggregation.new({}).run }.to change{ UpcomingEvent.count }.from(5).to(3) | ||
end | ||
end | ||
end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
ちょっと思慮不足で見落としていたのでいまさらで申し訳ないんですが、
dojo_name
とservice_name
が非正規化されているのってどういう意図でしょうか?正規化のほうがされている方が良さそうに思ったので、お聞きしたいです〜。
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.
dojo_name を持っているは直近のイベント一覧を表示するときに、dojo レコードまで辿らなくても表示できるかな、という目論見によります。
service_name は service_name + event_id でユニーク制約を付けたかったので持ちました。
後者については、関連する(親の)レコードの attribute (今回の場合は service_name or service_id)と自レコードの attribute (今回の場合は event_id) でのユニーク制約を設定する方法があれば、その方がよいと思っています。
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.
履歴テーブルの場合は過去の不変の情報なので非正規化、未来の予定に関するテーブルの場合は変更に強い情報の持ち方で正規化、というのが良いかなと思っています。
このテーブルで扱ってる情報としてはあまりないユースケースになるとは思いますがdojo_nameが変わってしまうケースは変更に追従できなさそうに思います。
こちらはこれからユニーク制約が追加されるという事になりますか?
についてはちょっと理解が追いつきませんでした。
ユニーク制約がないと困るケースが想像できなかったので、例を挙げて貰えると助かります。
Uh oh!
There was an error while loading. Please reload this page.
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.
dojo_name については、ご指摘の通りです。
少し楽をしようと思ってしまいましたが、正しい姿ではありませんよね。
となると、毎週 Update していく UpcomingEvent のレコードよりも、一旦収集したらそのまま残りそうな EventHistory のレコードの方が気になってしまいました...。
service_name + event_id でのユニーク制約は、ActiveRecord レベルでは設定していたのですが、SQL レベルでは設定できていないという不十分な状態でした。
対応漏れですので、追記します。
connpass や doorkeeper という service_name + event_id で UpcomingEvent が一意になるという前提で 2 ヶ月後までのイベント情報を upsert していますので、ユニーク制約は必要であると考えました。
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.
event_historiesは履歴なのでそのままで大丈夫です。
過去の事実を記録しているので、仮に現在のdojo_nameと差異があったとしてもそれを最新に更新すると事実と異なる事が履歴にあることになってしまいます。
あ、event_idだけだと、重複しないと言い切れないからってことですか。
event_urlでも事足りませんかね、ユニーク制約🤔
まあ、service_nameが変わることはほぼ無いと思うので、問題があるわけではないですが。
Uh oh!
There was an error while loading. Please reload this page.
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.
「event_historiesは履歴」なのですが、イベントが開催されたときの Dojo名 ではなく、統計情報を収集したときの Dojo 名ですよね。イベント開催時でも現在でもないので、気になりました。
connpass, doorkeeper のみでしたら event_url でユニーク制約でよいのですが、
static_yaml や現 event_histories でやむなく採用している facebook の yaml になりますと URL を持たないので断念しました。
URL は「直近のイベント一覧」で表示したいので、そのまま URL として有効な値を設定しておきたいと考え、yaml 用にダミーでユニークになりそうな文字列を生成するような対応も採りませんでした。
yaml からの更新の場合は、event_id も独自採番になるのですが... (ここ、あまり考慮できていなかったです)
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.
そこまでハンドリングするのはさすがに難しそうな印象ですねぇ🤔
なるほど、そういう事情なんですね、理解しました。
ありがとうございます〜。
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.
ひとつ訂正です。
facebook イベントの yaml からの読み込みの場合、event_id は facebook 側のイベントから拾えますし、イベントの URL もあります。
static_yaml の場合だけが URL も event_id もありません。直近のイベント情報収集で static_yaml に対応するときに注意が必要になります。