From 44e55d752928b7c76d84aacd9bd41cb03f13a4ac Mon Sep 17 00:00:00 2001 From: esparratacus Date: Tue, 10 Jan 2023 18:01:03 +0100 Subject: [PATCH 1/2] Em 322 redundant prs (#568) (#569) * add pull request id to completed review turnaround * add test for completed review turnaround backfiller * add rake task to set pull request on review turnaround model * changed redundant task name * linting issues * reviewer changes * rubocop styling changes * add relation to factory Co-authored-by: Daniel Serrano Co-authored-by: Daniel Serrano --- app/models/completed_review_turnaround.rb | 4 ++ app/models/review_turnaround.rb | 4 ++ .../builders/completed_review_turnaround.rb | 3 +- app/services/builders/review_turnaround.rb | 4 +- ...view_turnaround_pull_request_backfiller.rb | 9 ++++ ...view_turnaround_pull_request_backfiller.rb | 9 ++++ ..._request_to_completed_review_turnaround.rb | 6 +++ ...9_add_pull_request_to_review_turnaround.rb | 5 +++ db/structure.sql | 44 ++++++++++++++++--- ...et_pull_request_to_review_turnarounds.rake | 15 +++++++ .../factories/completed_review_turnarounds.rb | 4 ++ spec/factories/review_turnaround.rb | 1 + spec/models/review_turnaround_spec.rb | 3 ++ .../chartkick/users_repository_data_spec.rb | 4 +- ...turnaround_pull_request_backfiller_spec.rb | 23 ++++++++++ ...turnaround_pull_request_backfiller_spec.rb | 21 +++++++++ 16 files changed, 151 insertions(+), 8 deletions(-) create mode 100644 app/services/processors/completed_review_turnaround_pull_request_backfiller.rb create mode 100644 app/services/processors/review_turnaround_pull_request_backfiller.rb create mode 100644 db/migrate/20221228101900_add_pull_request_to_completed_review_turnaround.rb create mode 100644 db/migrate/20221228121949_add_pull_request_to_review_turnaround.rb create mode 100644 lib/tasks/set_pull_request_to_review_turnarounds.rake create mode 100644 spec/services/processors/completed_review_turnaround_pull_request_backfiller_spec.rb create mode 100644 spec/services/processors/review_turnaround_pull_request_backfiller_spec.rb diff --git a/app/models/completed_review_turnaround.rb b/app/models/completed_review_turnaround.rb index 2b26070ea..371717853 100644 --- a/app/models/completed_review_turnaround.rb +++ b/app/models/completed_review_turnaround.rb @@ -7,14 +7,17 @@ # value :integer # created_at :datetime not null # updated_at :datetime not null +# pull_request_id :bigint # review_request_id :bigint not null # # Indexes # +# index_completed_review_turnarounds_on_pull_request_id (pull_request_id) # index_completed_review_turnarounds_on_review_request_id (review_request_id) # # Foreign Keys # +# fk_rails_... (pull_request_id => events_pull_requests.id) # fk_rails_... (review_request_id => review_requests.id) # @@ -24,6 +27,7 @@ class CompletedReviewTurnaround < ApplicationRecord include EntityTimeRepresentation belongs_to :review_request + belongs_to :pull_request, class_name: 'Events::PullRequest' validates :value, presence: true validates :review_request_id, uniqueness: true diff --git a/app/models/review_turnaround.rb b/app/models/review_turnaround.rb index ee03e086f..0159f01ad 100644 --- a/app/models/review_turnaround.rb +++ b/app/models/review_turnaround.rb @@ -6,14 +6,17 @@ # value :integer # created_at :datetime not null # updated_at :datetime not null +# pull_request_id :bigint # review_request_id :bigint not null # # Indexes # +# index_review_turnarounds_on_pull_request_id (pull_request_id) # index_review_turnarounds_on_review_request_id (review_request_id) # # Foreign Keys # +# fk_rails_... (pull_request_id => events_pull_requests.id) # fk_rails_... (review_request_id => review_requests.id) # @@ -21,6 +24,7 @@ class ReviewTurnaround < ApplicationRecord include EntityTimeRepresentation belongs_to :review_request + belongs_to :pull_request, class_name: 'Events::PullRequest' validates :value, presence: true validates :review_request_id, uniqueness: true, strict: Reviews::ReviewRequestUniquenessError diff --git a/app/services/builders/completed_review_turnaround.rb b/app/services/builders/completed_review_turnaround.rb index ef5d42d45..a82de3c6d 100644 --- a/app/services/builders/completed_review_turnaround.rb +++ b/app/services/builders/completed_review_turnaround.rb @@ -7,7 +7,8 @@ def initialize(review) def call ::CompletedReviewTurnaround.create!( review_request_id: @review.review_request_id, - value: calculate_completed_turnaround + value: calculate_completed_turnaround, + pull_request: @review.pull_request ) end diff --git a/app/services/builders/review_turnaround.rb b/app/services/builders/review_turnaround.rb index fc45b5835..e5072d7db 100644 --- a/app/services/builders/review_turnaround.rb +++ b/app/services/builders/review_turnaround.rb @@ -5,7 +5,9 @@ def initialize(review_request) end def call - ::ReviewTurnaround.create!(review_request: @review_request, value: calculate_turnaround) + ::ReviewTurnaround.create!(review_request: @review_request, + value: calculate_turnaround, + pull_request: @review_request.pull_request) end private diff --git a/app/services/processors/completed_review_turnaround_pull_request_backfiller.rb b/app/services/processors/completed_review_turnaround_pull_request_backfiller.rb new file mode 100644 index 000000000..3b077eaa7 --- /dev/null +++ b/app/services/processors/completed_review_turnaround_pull_request_backfiller.rb @@ -0,0 +1,9 @@ +module Processors + class CompletedReviewTurnaroundPullRequestBackfiller < BaseService + def call + CompletedReviewTurnaround.find_each do |review| + review.update(pull_request: review.review_request.pull_request) + end + end + end +end diff --git a/app/services/processors/review_turnaround_pull_request_backfiller.rb b/app/services/processors/review_turnaround_pull_request_backfiller.rb new file mode 100644 index 000000000..e7d15c121 --- /dev/null +++ b/app/services/processors/review_turnaround_pull_request_backfiller.rb @@ -0,0 +1,9 @@ +module Processors + class ReviewTurnaroundPullRequestBackfiller < BaseService + def call + ReviewTurnaround.find_each do |review| + review.update(pull_request: review.review_request.pull_request) + end + end + end +end diff --git a/db/migrate/20221228101900_add_pull_request_to_completed_review_turnaround.rb b/db/migrate/20221228101900_add_pull_request_to_completed_review_turnaround.rb new file mode 100644 index 000000000..eb741844c --- /dev/null +++ b/db/migrate/20221228101900_add_pull_request_to_completed_review_turnaround.rb @@ -0,0 +1,6 @@ + +class AddPullRequestToCompletedReviewTurnaround < ActiveRecord::Migration[6.0] + def change + add_reference :completed_review_turnarounds, :pull_request, foreign_key: { to_table: :events_pull_requests } + end +end diff --git a/db/migrate/20221228121949_add_pull_request_to_review_turnaround.rb b/db/migrate/20221228121949_add_pull_request_to_review_turnaround.rb new file mode 100644 index 000000000..9c50bbbad --- /dev/null +++ b/db/migrate/20221228121949_add_pull_request_to_review_turnaround.rb @@ -0,0 +1,5 @@ +class AddPullRequestToReviewTurnaround < ActiveRecord::Migration[6.0] + def change + add_reference :review_turnarounds, :pull_request, foreign_key: { to_table: :events_pull_requests } + end +end diff --git a/db/structure.sql b/db/structure.sql index 23ccd052b..88e9d8d6f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -458,7 +458,8 @@ CREATE TABLE public.completed_review_turnarounds ( value integer, created_at timestamp(6) without time zone NOT NULL, updated_at timestamp(6) without time zone NOT NULL, - deleted_at timestamp without time zone + deleted_at timestamp without time zone, + pull_request_id bigint ); @@ -858,9 +859,9 @@ CREATE TABLE public.external_pull_requests ( external_repository_id bigint NOT NULL, created_at timestamp(6) without time zone NOT NULL, updated_at timestamp(6) without time zone NOT NULL, + number integer, opened_at timestamp without time zone, - state public.external_pull_request_state, - number integer + state public.external_pull_request_state ); @@ -1324,7 +1325,8 @@ CREATE TABLE public.review_turnarounds ( review_request_id bigint NOT NULL, value integer, created_at timestamp(6) without time zone NOT NULL, - updated_at timestamp(6) without time zone NOT NULL + updated_at timestamp(6) without time zone NOT NULL, + pull_request_id bigint ); @@ -2125,6 +2127,13 @@ CREATE INDEX index_code_owner_repositories_on_repository_id ON public.code_owner CREATE INDEX index_code_owner_repositories_on_user_id ON public.code_owner_repositories USING btree (user_id); +-- +-- Name: index_completed_review_turnarounds_on_pull_request_id; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_completed_review_turnarounds_on_pull_request_id ON public.completed_review_turnarounds USING btree (pull_request_id); + + -- -- Name: index_completed_review_turnarounds_on_review_request_id; Type: INDEX; Schema: public; Owner: - -- @@ -2468,6 +2477,13 @@ CREATE INDEX index_review_requests_on_reviewer_id ON public.review_requests USIN CREATE INDEX index_review_requests_on_state ON public.review_requests USING btree (state); +-- +-- Name: index_review_turnarounds_on_pull_request_id; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_review_turnarounds_on_pull_request_id ON public.review_turnarounds USING btree (pull_request_id); + + -- -- Name: index_review_turnarounds_on_review_request_id; Type: INDEX; Schema: public; Owner: - -- @@ -2543,6 +2559,14 @@ ALTER TABLE ONLY public.events_pull_request_comments ADD CONSTRAINT fk_rails_161aa5ffd0 FOREIGN KEY (owner_id) REFERENCES public.users(id); +-- +-- Name: completed_review_turnarounds fk_rails_200ab21dcf; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.completed_review_turnarounds + ADD CONSTRAINT fk_rails_200ab21dcf FOREIGN KEY (pull_request_id) REFERENCES public.events_pull_requests(id); + + -- -- Name: repositories fk_rails_21e11c2480; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -2695,6 +2719,14 @@ ALTER TABLE ONLY public.events_reviews ADD CONSTRAINT fk_rails_65b0ea4a71 FOREIGN KEY (repository_id) REFERENCES public.repositories(id); +-- +-- Name: review_turnarounds fk_rails_7bc2fb6ebc; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.review_turnarounds + ADD CONSTRAINT fk_rails_7bc2fb6ebc FOREIGN KEY (pull_request_id) REFERENCES public.events_pull_requests(id); + + -- -- Name: languages fk_rails_822295ed05; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -3020,6 +3052,8 @@ INSERT INTO "schema_migrations" (version) VALUES ('20210902182225'), ('20210915145551'), ('20210916151310'), -('20220412181602'); +('20220412181602'), +('20221228101900'), +('20221228121949'); diff --git a/lib/tasks/set_pull_request_to_review_turnarounds.rake b/lib/tasks/set_pull_request_to_review_turnarounds.rake new file mode 100644 index 000000000..07f2f276f --- /dev/null +++ b/lib/tasks/set_pull_request_to_review_turnarounds.rake @@ -0,0 +1,15 @@ +namespace :one_time do + desc ' it sets pull_request to review turnarounds' + task set_pull_request_to_review_turnaround: :environment do + puts 'Setting pull_request to review_turnarounds...' + Processors::ReviewTurnaroundPullRequestBackfiller.call + puts 'Done!' + end + + desc 'it sets pull_request to completed review turnarounds' + task set_pull_request_to_completed_review_turnaround: :environment do + puts 'Setting pull_request to completed_review_turnarounds...' + Processors::CompletedReviewTurnaroundPullRequestBackfiller.call + puts 'Done!' + end +end diff --git a/spec/factories/completed_review_turnarounds.rb b/spec/factories/completed_review_turnarounds.rb index fa7b00c55..2610dbe5d 100644 --- a/spec/factories/completed_review_turnarounds.rb +++ b/spec/factories/completed_review_turnarounds.rb @@ -7,14 +7,17 @@ # value :integer # created_at :datetime not null # updated_at :datetime not null +# pull_request_id :bigint # review_request_id :bigint not null # # Indexes # +# index_completed_review_turnarounds_on_pull_request_id (pull_request_id) # index_completed_review_turnarounds_on_review_request_id (review_request_id) # # Foreign Keys # +# fk_rails_... (pull_request_id => events_pull_requests.id) # fk_rails_... (review_request_id => review_requests.id) # @@ -23,5 +26,6 @@ value { Faker::Number.number(digits: 4) } association :review_request + association :pull_request end end diff --git a/spec/factories/review_turnaround.rb b/spec/factories/review_turnaround.rb index e6b842df9..64505429b 100644 --- a/spec/factories/review_turnaround.rb +++ b/spec/factories/review_turnaround.rb @@ -3,5 +3,6 @@ value { Faker::Number.number(digits: 4) } association :review_request + association :pull_request end end diff --git a/spec/models/review_turnaround_spec.rb b/spec/models/review_turnaround_spec.rb index 4c49251ea..0fe7d1a84 100644 --- a/spec/models/review_turnaround_spec.rb +++ b/spec/models/review_turnaround_spec.rb @@ -6,14 +6,17 @@ # value :integer # created_at :datetime not null # updated_at :datetime not null +# pull_request_id :bigint # review_request_id :bigint not null # # Indexes # +# index_review_turnarounds_on_pull_request_id (pull_request_id) # index_review_turnarounds_on_review_request_id (review_request_id) # # Foreign Keys # +# fk_rails_... (pull_request_id => events_pull_requests.id) # fk_rails_... (review_request_id => review_requests.id) # diff --git a/spec/services/builders/chartkick/users_repository_data_spec.rb b/spec/services/builders/chartkick/users_repository_data_spec.rb index 702eae4cc..939394fd9 100644 --- a/spec/services/builders/chartkick/users_repository_data_spec.rb +++ b/spec/services/builders/chartkick/users_repository_data_spec.rb @@ -15,7 +15,9 @@ let(:review_request) { create(:review_request) } let!(:completed_review_turnaround) do - create(:completed_review_turnaround, review_request: review_request) + create(:completed_review_turnaround, + review_request: review_request, + pull_request: review_request.pull_request) end let!(:users_repository) do create(:users_repository, repository_id: review_request.repository_id, diff --git a/spec/services/processors/completed_review_turnaround_pull_request_backfiller_spec.rb b/spec/services/processors/completed_review_turnaround_pull_request_backfiller_spec.rb new file mode 100644 index 000000000..7c3871cf7 --- /dev/null +++ b/spec/services/processors/completed_review_turnaround_pull_request_backfiller_spec.rb @@ -0,0 +1,23 @@ +require 'rails_helper' + +RSpec.describe Processors::CompletedReviewTurnaroundPullRequestBackfiller do + describe '.call' do + let(:ruby_lang) { Language.find_by(name: 'ruby') } + let!(:repository) { create(:repository, language: ruby_lang) } + let!(:review_request) { create(:review_request, repository: repository) } + let(:completed_review_turnaround) do + build(:completed_review_turnaround, + review_request: review_request, + pull_request: nil) + end + + before { completed_review_turnaround.save!(validate: false) } + it 'updates pull_request foreign_key in the completed review turnaround records' do + expect { described_class.call }.to change { + completed_review_turnaround.reload.pull_request_id + } + .from(nil) + .to(completed_review_turnaround.review_request.pull_request_id) + end + end +end diff --git a/spec/services/processors/review_turnaround_pull_request_backfiller_spec.rb b/spec/services/processors/review_turnaround_pull_request_backfiller_spec.rb new file mode 100644 index 000000000..1eb8dd5b6 --- /dev/null +++ b/spec/services/processors/review_turnaround_pull_request_backfiller_spec.rb @@ -0,0 +1,21 @@ +require 'rails_helper' + +RSpec.describe Processors::ReviewTurnaroundPullRequestBackfiller do + describe '.call' do + let(:ruby_lang) { Language.find_by(name: 'ruby') } + let!(:repository) { create(:repository, language: ruby_lang) } + let!(:review_request) { create(:review_request, repository: repository) } + let(:review_turnaround) do + build(:review_turnaround, + review_request: review_request, + pull_request: nil) + end + + before { review_turnaround.save!(validate: false) } + it 'updates pull_request foreign_key in the completed review turnaround records' do + expect { described_class.call }.to change { review_turnaround.reload.pull_request_id } + .from(nil) + .to(review_turnaround.review_request.pull_request_id) + end + end +end From fa40e1bc22babdd77545ef1a6db7a647fee64023 Mon Sep 17 00:00:00 2001 From: antoalconti Date: Tue, 22 Oct 2024 18:41:45 +0200 Subject: [PATCH 2/2] Fix redis instance --- config/initializers/rails_performance.rb | 8 ++++++-- config/initializers/sidekiq.rb | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/config/initializers/rails_performance.rb b/config/initializers/rails_performance.rb index 37f468519..78f72cff2 100644 --- a/config/initializers/rails_performance.rb +++ b/config/initializers/rails_performance.rb @@ -1,8 +1,12 @@ if defined?(RailsPerformance) RailsPerformance.setup do |config| - config.redis = Redis.new(url: ENV.fetch('REDIS_PERFORMANCE_URL') { 'redis://localhost:6379/1' }) + config.redis = Redis.new( + url: ENV.fetch('REDIS_PERFORMANCE_URL') { 'redis://localhost:6379/1' }, + ssl_params: { verify_mode: OpenSSL::SSL::VERIFY_NONE } + ) config.duration = 4.hours - config.enabled = true + config.enabled = true + if Rails.env.production? config.http_basic_authentication_enabled = true config.http_basic_authentication_user_name = ENV.fetch('PERFORMANCE_USERNAME') diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 60898d9d7..91a9485fd 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -1,9 +1,19 @@ Sidekiq.configure_client do |config| - config.redis = { url: ENV.fetch('REDIS_URL') { 'redis://localhost:6379/1' }, size: 1, network_timeout: 5 } + config.redis = { + url: ENV.fetch('REDIS_URL') { 'redis://localhost:6379/1' }, + size: 1, + network_timeout: 5, + ssl_params: { verify_mode: OpenSSL::SSL::VERIFY_NONE } + } end Sidekiq.configure_server do |config| - config.redis = { url: ENV.fetch('REDIS_URL') { 'redis://localhost:6379/1' }, size: 12, network_timeout: 5 } + config.redis = { + url: ENV.fetch('REDIS_URL') { 'redis://localhost:6379/1' }, + size: 12, + network_timeout: 5, + ssl_params: { verify_mode: OpenSSL::SSL::VERIFY_NONE } + } end Sidekiq::Extensions.enable_delay!