Skip to content
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

5120 – Create Tags in the background #2005

Merged
merged 26 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
97d2cd3
wip: create GenericWorker
vasconsaurus Aug 26, 2024
5ab4d0c
Test creating the tag while creating project ProjectMedia
vasconsaurus Aug 26, 2024
7bc2a5f
wip: try to move create_tags to the background
vasconsaurus Aug 26, 2024
ee8d6a3
edit worker and tag test
vasconsaurus Aug 27, 2024
88661be
Updates Tag creation to not send the entire object
vasconsaurus Aug 27, 2024
370d300
update GenericWorker method signature
vasconsaurus Sep 2, 2024
dedd1ae
Add new test: make sure item's tags creation run in the same job
vasconsaurus Sep 2, 2024
61f48d0
move tests from TagTest to ProjectMedia8Test
vasconsaurus Sep 2, 2024
e0e5873
Create all tags in one background job
vasconsaurus Sep 2, 2024
9a2986d
avoid raising an error if project media is nil
vasconsaurus Sep 5, 2024
81e95ee
Add Sentry Notification and tests
vasconsaurus Sep 5, 2024
ef52d2f
Pass User.current to the GenericWorker
vasconsaurus Sep 5, 2024
6781a9c
update tests
vasconsaurus Sep 5, 2024
ffd4a14
update how we notify Sentry
vasconsaurus Sep 12, 2024
3a67d18
update how we set User.current
vasconsaurus Sep 12, 2024
c8a0dd2
switch the methods' names
vasconsaurus Sep 12, 2024
8a6efa5
create a wrapper method so this is easier to read
vasconsaurus Sep 13, 2024
93963ab
change `create_tags` to receive specific standalone parameters instea…
vasconsaurus Sep 16, 2024
e35c643
keep setup and teardown to the bare minimum
vasconsaurus Sep 16, 2024
324ad39
add test to ensure job is queued and clean up tests
vasconsaurus Sep 16, 2024
b510a7b
change assert_nothing_raised to assert_difference
vasconsaurus Sep 16, 2024
935f44f
move run_later methods to a module
vasconsaurus Sep 19, 2024
2c9c1d0
add GenericWorkerHelpersTest
vasconsaurus Sep 20, 2024
ff01619
add test
vasconsaurus Sep 20, 2024
5c1c941
change conditional
vasconsaurus Sep 23, 2024
9f1141e
update tests to assert_difference
vasconsaurus Sep 23, 2024
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
9 changes: 9 additions & 0 deletions app/lib/generic_worker_helpers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module GenericWorkerHelpers
def run_later(klass_method, *method_args)
GenericWorker.perform_async(self.to_s, klass_method, *method_args)
end

def run_later_in(time, klass_method, *method_args)
GenericWorker.perform_in(time, self.to_s, klass_method, *method_args)
end
end
8 changes: 6 additions & 2 deletions app/models/concerns/project_media_creators.rb
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,11 @@ def create_claim_description_and_fact_check
fc
end

def create_tags
self.set_tags.each { |tag| Tag.create!(annotated: self, tag: tag.strip, skip_check_ability: true) } if self.set_tags.is_a?(Array)
def create_tags_in_background
if self.set_tags.is_a?(Array)
project_media_id = self.id
tags_json = self.set_tags.to_json
ProjectMedia.run_later_in(1.second, 'create_tags', project_media_id, tags_json, user_id: self.user_id)
end
end
end
14 changes: 13 additions & 1 deletion app/models/project_media.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class ProjectMedia < ApplicationRecord

before_validation :set_team_id, :set_channel, :set_project_id, on: :create
before_validation :create_original_claim, if: proc { |pm| pm.set_original_claim.present? }, on: :create
after_create :create_annotation, :create_metrics_annotation, :send_slack_notification, :create_relationship, :create_team_tasks, :create_claim_description_and_fact_check, :create_tags
after_create :create_annotation, :create_metrics_annotation, :send_slack_notification, :create_relationship, :create_team_tasks, :create_claim_description_and_fact_check, :create_tags_in_background
after_create :add_source_creation_log, unless: proc { |pm| pm.source_id.blank? }
after_commit :apply_rules_and_actions_on_create, :set_quote_metadata, :notify_team_bots_create, on: [:create]
after_commit :create_relationship, on: [:update]
Expand Down Expand Up @@ -528,6 +528,18 @@ def add_nested_objects(ms)
ms.attributes[:requests] = requests
end

def self.create_tags(project_media_id, tags_json)
project_media = ProjectMedia.find_by_id(project_media_id)

if !project_media.nil?
tags = JSON.parse(tags_json)
tags.each { |tag| Tag.create! annotated: project_media, tag: tag.strip, skip_check_ability: true }
else
error = StandardError.new("[ProjectMedia] Exception creating project media's tags in background. Project media is nil.")
CheckSentry.notify(error, project_media_id: project_media_id)
end
end

# private
#
# Please add private methods to app/models/concerns/project_media_private.rb
Expand Down
18 changes: 18 additions & 0 deletions app/workers/generic_worker.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
class GenericWorker

include Sidekiq::Worker

def perform(klass_name, klass_method, *method_args)
klass = klass_name.constantize
melsawy marked this conversation as resolved.
Show resolved Hide resolved
options = method_args.extract_options!.with_indifferent_access
unless options.blank?
user_id = options.delete(:user_id) if options.key?(:user_id)
current_user = User.current
User.current = User.find_by_id(user_id)
klass.public_send(klass_method, *method_args, **options)
User.current = current_user
else
klass.public_send(klass_method)
end
end
end
3 changes: 3 additions & 0 deletions config/initializers/class_extensions.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class Class
include GenericWorkerHelpers
end
1 change: 1 addition & 0 deletions lib/sample_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,7 @@ def create_project_media(options = {})
end
options[:skip_autocreate_source] = true unless options.has_key?(:skip_autocreate_source)
pm.source = create_source({ team: options[:team], skip_check_ability: true }) if options[:skip_autocreate_source]
pm.set_tags = options[:tags] if options[:tags]
pm.save!
create_cluster_project_media({ cluster: options[:cluster], project_media: pm}) if options[:cluster]
pm.reload
Expand Down
34 changes: 34 additions & 0 deletions test/lib/generic_worker_helpers_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
require_relative '../test_helper'

class GenericWorkerHelpersTest < ActionView::TestCase
def setup
require 'sidekiq/testing'
Sidekiq::Worker.clear_all
end

def teardown
end

test "should run a job, without raising an error, for a method that takes a hash as a parameter" do
Sidekiq::Testing.inline!

assert_difference "Team.where(name: 'BackgroundTeam', slug: 'background-team').count" do
Team.run_later('create!', name: 'BackgroundTeam', slug: 'background-team')
end
end

test "should run a job, without raising an error, for a method that takes standalone parameters" do
Sidekiq::Testing.inline!

team = create_team
project = create_project team: team
pm = create_project_media project: project

project_media_id = pm.id
tags_json = ['one', 'two', 'three'].to_json

ProjectMedia.run_later_in(0.second, 'create_tags', project_media_id, tags_json, user_id: pm.user_id)

assert_equal 3, pm.annotations('tag').count
end
end
72 changes: 72 additions & 0 deletions test/models/project_media_8_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
require_relative '../test_helper'

class ProjectMedia8Test < ActiveSupport::TestCase
def setup
require 'sidekiq/testing'
Sidekiq::Worker.clear_all
end

def teardown
end

test ":create_tags should create tags when project media id and tags are present" do
team = create_team
project = create_project team: team
pm = create_project_media project: project

project_media_id = pm.id
tags_json = ['one', 'two'].to_json

assert_nothing_raised do
ProjectMedia.create_tags(project_media_id, tags_json)
end
assert_equal 2, pm.annotations('tag').count
end

test ":create_tags should not raise an error when no project media is sent" do
project_media_id = nil
tags_json = ['one', 'two'].to_json

assert_nothing_raised do
CheckSentry.expects(:notify).once
ProjectMedia.create_tags(project_media_id, tags_json)
end
melsawy marked this conversation as resolved.
Show resolved Hide resolved
end

test "when creating an item with tag/tags, after_create :create_tags_in_background callback should create tags in the background" do
Sidekiq::Testing.inline!

team = create_team
project = create_project team: team
pm = create_project_media project: project, tags: ['one']
caiosba marked this conversation as resolved.
Show resolved Hide resolved

assert_equal 1, pm.annotations('tag').count
end

test "when creating an item with multiple tags, after_create :create_tags_in_background callback should only schedule one job" do
Sidekiq::Testing.fake!

team = create_team
project = create_project team: team

assert_nothing_raised do
create_project_media project: project, tags: ['one', 'two', 'three']
caiosba marked this conversation as resolved.
Show resolved Hide resolved
end
assert_equal 1, GenericWorker.jobs.size
end

test "when using :run_later_in to schedule multiple tags creation, it should only schedule one job" do
Sidekiq::Testing.fake!

team = create_team
project = create_project team: team
pm = create_project_media project: project

project_media_id = pm.id
tags_json = ['one', 'two', 'three'].to_json

ProjectMedia.run_later_in(0.second, 'create_tags', project_media_id, tags_json, user_id: pm.user_id)

assert_equal 1, GenericWorker.jobs.size
end
end
79 changes: 79 additions & 0 deletions test/workers/generic_worker_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
require 'test_helper'

class GenericWorkerTest < ActiveSupport::TestCase
def setup
require 'sidekiq/testing'
Sidekiq::Worker.clear_all
end

def teardown
end

test "should run a job, without raising an error, for a method that takes no parameters" do
Sidekiq::Testing.inline!

assert_difference 'Blank.count' do
GenericWorker.perform_async('Blank', 'create!')
end
end

test "should run a job, without raising an error, for a method that takes a hash as a parameter" do
Sidekiq::Testing.inline!

assert_difference "Team.where(name: 'BackgroundTeam', slug: 'background-team').count" do
GenericWorker.perform_async('Team', 'create!', name: 'BackgroundTeam', slug: 'background-team')
end
end

test "should run a job, without raising an error, for a method that takes standalone parameters" do
Sidekiq::Testing.inline!

t = create_team
p = create_project team: t
pm = create_project_media project: p

project_media_id = pm.id
tags_json = ['one', 'two'].to_json

assert_difference "Tag.where(annotation_type: 'tag').count", difference = 2 do
GenericWorker.perform_async('ProjectMedia', 'create_tags', project_media_id, tags_json, user_id: pm.user_id)
end
end

test "should schedule a job, without raising an error, for a method that takes no parameters" do
Sidekiq::Testing.fake!

assert_nothing_raised do
GenericWorker.perform_async('Media', 'types')
end

assert_equal 1, GenericWorker.jobs.size
end

test "should schedule a job, without raising an error, for a method that takes a hash as a parameter" do
Sidekiq::Testing.fake!

assert_nothing_raised do
caiosba marked this conversation as resolved.
Show resolved Hide resolved
GenericWorker.perform_async('Team', 'create!', name: 'BackgroundTeam', slug: 'background-team')
end

assert_equal 1, GenericWorker.jobs.size
end

test "should schedule a job, without raising an error, for a method that takes standalone parameters" do
Sidekiq::Testing.fake!

t = create_team
p = create_project team: t
pm = create_project_media project: p

project_media_id = pm.id
tags_json = ['one', 'two'].to_json

assert_nothing_raised do
GenericWorker.perform_async('ProjectMedia', 'create_tags', project_media_id, tags_json, user_id: pm.user_id)
end

assert_equal 1, GenericWorker.jobs.size
end
caiosba marked this conversation as resolved.
Show resolved Hide resolved
end
Loading