レビュー指摘事項対応

・関連見直し
・リファクタ
・不要なコメント削除

Author: chicaco

app/models/dojo.rb

@@ -6,7 +6,6 @@ class Dojo < ApplicationRecord
   belongs_to :prefecture
   has_many   :dojo_event_services, dependent: :destroy
   has_many   :event_histories,     dependent: :destroy
-  has_many   :upcoming_events,     dependent: :destroy
 
   serialize :tags
   before_save { self.email = self.email.downcase }

app/models/dojo_event_service.rb

@@ -3,6 +3,7 @@ class DojoEventService < ApplicationRecord
   INTERNAL_SERVICES = %i( static_yaml )
 
   belongs_to :dojo
+  has_many :upcoming_events, dependent: :destroy
 
   enum name: EXTERNAL_SERVICES + INTERNAL_SERVICES
 

app/models/upcoming_event.rb

@@ -1,5 +1,5 @@
 class UpcomingEvent < ApplicationRecord
-  belongs_to :dojo
+  belongs_to :dojo_event_service
 
   validates :dojo_name,    presence: true
   validates :service_name, presence: true, uniqueness: { scope: :event_id  }

db/migrate/20190526151359_mod_columns_to_upcoming_event.rb

@@ -1,23 +1,17 @@
 class ModColumnsToUpcomingEvent < ActiveRecord::Migration[5.1]
   def up
-    remove_index :upcoming_events, :dojo_event_service_id
     remove_index :upcoming_events, :event_at
-    remove_column :upcoming_events, :dojo_event_service_id
 
-    add_reference :upcoming_events, :dojo, foreign_key: true, index: true, null: false
     add_column :upcoming_events, :dojo_name, :string, null: false
     add_column :upcoming_events, :service_name, :string, null: false
     add_column :upcoming_events, :participants, :integer, null: false
   end
 
   def down
-    remove_reference :upcoming_events, :dojo, findex: true
     remove_column :upcoming_events, :dojo_name, :string, null: false
     remove_column :upcoming_events, :service_name, :string, null: false
     remove_column :upcoming_events, :participants, :integer, null: false
 
-    add_column :upcoming_events, :dojo_event_service_id, :integer, null: false, default: 1
-    add_index :upcoming_events, :dojo_event_service_id, name: "index_upcoming_events_on_dojo_event_service_id"
     add_index :upcoming_events, :event_at, name: "index_upcoming_events_on_event_at"
   end
 end

db/schema.rb

@@ -83,14 +83,13 @@
     t.string "event_id", null: false
     t.string "event_url", null: false
     t.datetime "event_at", null: false
-    t.bigint "dojo_id", null: false
+    t.integer "dojo_event_service_id", default: 1, null: false
     t.string "dojo_name", null: false
     t.string "service_name", null: false
     t.integer "participants", null: false
-    t.index ["dojo_id"], name: "index_upcoming_events_on_dojo_id"
+    t.index ["dojo_event_service_id"], name: "index_upcoming_events_on_dojo_event_service_id"
   end
 
   add_foreign_key "dojo_event_services", "dojos"
   add_foreign_key "event_histories", "dojos"
-  add_foreign_key "upcoming_events", "dojos"
 end

lib/upcoming_events/aggregation.rb

@@ -4,10 +4,8 @@ def initialize(args)
       @from = Time.zone.today
       @to = @from + 2.months
       @provider = args[:provider]
-      dojos = fetch_dojos(@provider)
       # NOTE: 対象は一旦収集可能な connpass, doorkeeper のみにする
-      @externals = dojos[:externals]
-      # @internals = dojos[:internals]
+      @externals = fetch_dojos(@provider)
     end
 
     def run
@@ -21,25 +19,15 @@ def run
     private
 
     def fetch_dojos(provider)
-      if provider.blank?
-        # 全プロバイダ対象
-        external_services = DojoEventService::EXTERNAL_SERVICES
-        internal_services = DojoEventService::INTERNAL_SERVICES
-      else
-        external_services = []
-        internal_services = []
-        case provider
-        when 'connpass', 'doorkeeper', 'facebook'
-          external_services = [provider.to_sym]
-        when 'static_yaml'
-          internal_services = [provider.to_sym]
-        end
-      end
-
-      {
-        externals: find_dojos_by(external_services),
-        internals: find_dojos_by(internal_services)
-      }
+      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)
@@ -62,10 +50,6 @@ def delete_upcoming_events
     def execute
       target_period = @from..@to
       @externals.each do |kind, list|
-        unless [:connpass, :doorkeeper].include?(kind)
-          puts "Aggregate of #{kind} --> skip"
-          next
-        end
         puts "Aggregate of #{kind}"
         "UpcomingEvents::Tasks::#{kind.to_s.camelize}".constantize.new(list, target_period).run
       end

lib/upcoming_events/tasks.rb

@@ -5,5 +5,3 @@ module Tasks
 
 require_relative 'tasks/connpass'
 require_relative 'tasks/doorkeeper'
-# require_relative 'tasks/facebook'
-# require_relative 'tasks/static_yaml'

lib/upcoming_events/tasks/connpass.rb

@@ -13,12 +13,12 @@ def run
             @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 = UpcomingEvent.find_or_initialize_by(dojo_id: dojo.id,
-                                                           service_name: dojo_event_service.name,
-                                                           event_id: e['event_id'])
-              record.update!(dojo_name: dojo.name,
-                             event_url: e['event_url'],
-                             event_at: Time.zone.parse(e['started_at']),
+              record = dojo_event_service.upcoming_events.find_by(event_id: e['event_id'])
+              record ||= dojo_event_service.upcoming_events.build(event_id: e['event_id'])
+              record.update!(dojo_name:    dojo.name,
+                             service_name: dojo_event_service.name,
+                             event_url:    e['event_url'],
+                             event_at:     Time.zone.parse(e['started_at']),
                              participants: e['accepted'])
             end
           end

lib/upcoming_events/tasks/doorkeeper.rb

@@ -13,10 +13,10 @@ def run
             @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 = UpcomingEvent.find_or_initialize_by(dojo_id: dojo.id,
-                                                           service_name: dojo_event_service.name,
-                                                           event_id: e['id'])
+              record = dojo_event_service.upcoming_events.find_by(event_id: e['id'])
+              record ||= dojo_event_service.upcoming_events.build(event_id: e['id'])
               record.update!(dojo_name: dojo.name,
+                             service_name: dojo_event_service.name,
                              event_url: e['public_url'],
                              participants: e['participants'],
                              event_at: Time.zone.parse(e['starts_at']))

spec/factories/upcoming_events.rb

@@ -2,7 +2,6 @@
 
 FactoryBot.define do
   factory :upcoming_event do
-    # dojo_id
     dojo_name    { 'Dojo Name' }
     service_name { :connpass }
     event_id     { '1234' }

spec/lib/upcoming_events/aggregation_spec.rb

@@ -9,8 +9,8 @@
     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')
-      create(:dojo_event_service, dojo_id: @d1.id, name: :connpass,   group_id: 9876)
-      create(:dojo_event_service, dojo_id: @d2.id, name: :doorkeeper, group_id: 5555)
+      @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
@@ -26,11 +26,11 @@
     end
 
     it '昨日分までは削除' do
-      create(:upcoming_event, dojo_id: @d1.id, service_name: 'connpass', event_id: '1111', event_at: "#{Time.zone.today - 3.days} 13:00:00".in_time_zone)
-      create(:upcoming_event, dojo_id: @d1.id, service_name: 'connpass', event_id: '2222', event_at: "#{Time.zone.today - 2.days} 14:00:00".in_time_zone)
-      create(:upcoming_event, dojo_id: @d1.id, service_name: 'connpass', event_id: '3333', event_at: "#{Time.zone.today - 1.days} 15:00:00".in_time_zone)
-      create(:upcoming_event, dojo_id: @d2.id, service_name: 'doorkeeper', event_id: '4444', event_at: "#{Time.zone.today - 2.days} 10:00:00".in_time_zone)
-      create(:upcoming_event, dojo_id: @d2.id, service_name: 'doorkeeper', event_id: '5555', event_at: "#{Time.zone.today - 1.days} 11:00:00".in_time_zone)
+      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