From 87d92f73d49c12f51b1aaf772683059341dd928f Mon Sep 17 00:00:00 2001 From: Pranav Date: Wed, 22 May 2024 17:34:24 -0700 Subject: [PATCH 01/33] feat: Improve Report API performance (#9476) - Re-write the methods for clarity - Remove the dependency on the ReportHelper class. - Remove n+1 queries in the average metric time series data. --- .../conversations/base_report_builder.rb | 30 +++ .../reports/conversations/metric_builder.rb | 30 +++ .../reports/conversations/report_builder.rb | 21 +++ .../timeseries/average_report_builder.rb | 48 +++++ .../timeseries/base_timeseries_builder.rb | 46 +++++ .../timeseries/count_report_builder.rb | 71 +++++++ .../api/v2/accounts/reports_controller.rb | 19 +- app/helpers/timezone_helper.rb | 19 ++ .../conversations/metric_builder_spec.rb | 50 +++++ .../conversations/report_builder_spec.rb | 44 +++++ .../timeseries/average_report_builder_spec.rb | 174 ++++++++++++++++++ .../api/v2/accounts/report_controller_spec.rb | 4 +- spec/factories/reporting_events.rb | 1 + 13 files changed, 545 insertions(+), 12 deletions(-) create mode 100644 app/builders/v2/reports/conversations/base_report_builder.rb create mode 100644 app/builders/v2/reports/conversations/metric_builder.rb create mode 100644 app/builders/v2/reports/conversations/report_builder.rb create mode 100644 app/builders/v2/reports/timeseries/average_report_builder.rb create mode 100644 app/builders/v2/reports/timeseries/base_timeseries_builder.rb create mode 100644 app/builders/v2/reports/timeseries/count_report_builder.rb create mode 100644 app/helpers/timezone_helper.rb create mode 100644 spec/builders/v2/reports/conversations/metric_builder_spec.rb create mode 100644 spec/builders/v2/reports/conversations/report_builder_spec.rb create mode 100644 spec/builders/v2/reports/timeseries/average_report_builder_spec.rb diff --git a/app/builders/v2/reports/conversations/base_report_builder.rb b/app/builders/v2/reports/conversations/base_report_builder.rb new file mode 100644 index 000000000..a7961b0d6 --- /dev/null +++ b/app/builders/v2/reports/conversations/base_report_builder.rb @@ -0,0 +1,30 @@ +class V2::Reports::Conversations::BaseReportBuilder + pattr_initialize :account, :params + + private + + AVG_METRICS = %w[avg_first_response_time avg_resolution_time reply_time].freeze + COUNT_METRICS = %w[ + conversations_count + incoming_messages_count + outgoing_messages_count + resolutions_count + bot_resolutions_count + bot_handoffs_count + ].freeze + + def builder_class(metric) + case metric + when *AVG_METRICS + V2::Reports::Timeseries::AverageReportBuilder + when *COUNT_METRICS + V2::Reports::Timeseries::CountReportBuilder + end + end + + def log_invalid_metric + Rails.logger.error "ReportBuilder: Invalid metric - #{params[:metric]}" + + {} + end +end diff --git a/app/builders/v2/reports/conversations/metric_builder.rb b/app/builders/v2/reports/conversations/metric_builder.rb new file mode 100644 index 000000000..6635fb186 --- /dev/null +++ b/app/builders/v2/reports/conversations/metric_builder.rb @@ -0,0 +1,30 @@ +class V2::Reports::Conversations::MetricBuilder < V2::Reports::Conversations::BaseReportBuilder + def summary + { + conversations_count: count('conversations_count'), + incoming_messages_count: count('incoming_messages_count'), + outgoing_messages_count: count('outgoing_messages_count'), + avg_first_response_time: count('avg_first_response_time'), + avg_resolution_time: count('avg_resolution_time'), + resolutions_count: count('resolutions_count'), + reply_time: count('reply_time') + } + end + + def bot_summary + { + bot_resolutions_count: count('bot_resolutions_count'), + bot_handoffs_count: count('bot_handoffs_count') + } + end + + private + + def count(metric) + builder_class(metric).new(account, builder_params(metric)).aggregate_value + end + + def builder_params(metric) + params.merge({ metric: metric }) + end +end diff --git a/app/builders/v2/reports/conversations/report_builder.rb b/app/builders/v2/reports/conversations/report_builder.rb new file mode 100644 index 000000000..8f992d4c6 --- /dev/null +++ b/app/builders/v2/reports/conversations/report_builder.rb @@ -0,0 +1,21 @@ +class V2::Reports::Conversations::ReportBuilder < V2::Reports::Conversations::BaseReportBuilder + def timeseries + perform_action(:timeseries) + end + + def aggregate_value + perform_action(:aggregate_value) + end + + private + + def perform_action(method_name) + return builder.new(account, params).public_send(method_name) if builder.present? + + log_invalid_metric + end + + def builder + builder_class(params[:metric]) + end +end diff --git a/app/builders/v2/reports/timeseries/average_report_builder.rb b/app/builders/v2/reports/timeseries/average_report_builder.rb new file mode 100644 index 000000000..3e30557e1 --- /dev/null +++ b/app/builders/v2/reports/timeseries/average_report_builder.rb @@ -0,0 +1,48 @@ +class V2::Reports::Timeseries::AverageReportBuilder < V2::Reports::Timeseries::BaseTimeseriesBuilder + def timeseries + grouped_average_time = reporting_events.average(average_value_key) + grouped_event_count = reporting_events.count + grouped_average_time.each_with_object([]) do |element, arr| + event_date, average_time = element + arr << { + value: average_time, + timestamp: event_date.in_time_zone(timezone).to_i, + count: grouped_event_count[event_date] + } + end + end + + def aggregate_value + object_scope.average(average_value_key) + end + + private + + def event_name + metric_to_event_name = { + avg_first_response_time: :first_response, + avg_resolution_time: :conversation_resolved, + reply_time: :reply_time + } + metric_to_event_name[params[:metric].to_sym] + end + + def object_scope + scope.reporting_events.where(name: event_name, created_at: range) + end + + def reporting_events + @grouped_values = object_scope.group_by_period( + group_by, + :created_at, + default_value: 0, + range: range, + permit: %w[day week month year hour], + time_zone: timezone + ) + end + + def average_value_key + @average_value_key ||= params[:business_hours].present? ? :value_in_business_hours : :value + end +end diff --git a/app/builders/v2/reports/timeseries/base_timeseries_builder.rb b/app/builders/v2/reports/timeseries/base_timeseries_builder.rb new file mode 100644 index 000000000..50699417d --- /dev/null +++ b/app/builders/v2/reports/timeseries/base_timeseries_builder.rb @@ -0,0 +1,46 @@ +class V2::Reports::Timeseries::BaseTimeseriesBuilder + include TimezoneHelper + include DateRangeHelper + DEFAULT_GROUP_BY = 'day'.freeze + + pattr_initialize :account, :params + + def scope + case params[:type].to_sym + when :account + account + when :inbox + inbox + when :agent + user + when :label + label + when :team + team + end + end + + def inbox + @inbox ||= account.inboxes.find(params[:id]) + end + + def user + @user ||= account.users.find(params[:id]) + end + + def label + @label ||= account.labels.find(params[:id]) + end + + def team + @team ||= account.teams.find(params[:id]) + end + + def group_by + @group_by ||= %w[day week month year hour].include?(params[:group_by]) ? params[:group_by] : DEFAULT_GROUP_BY + end + + def timezone + @timezone ||= timezone_name_from_offset(params[:timezone_offset]) + end +end diff --git a/app/builders/v2/reports/timeseries/count_report_builder.rb b/app/builders/v2/reports/timeseries/count_report_builder.rb new file mode 100644 index 000000000..03a87a6fa --- /dev/null +++ b/app/builders/v2/reports/timeseries/count_report_builder.rb @@ -0,0 +1,71 @@ +class V2::Reports::Timeseries::CountReportBuilder < V2::Reports::Timeseries::BaseTimeseriesBuilder + def timeseries + grouped_count.each_with_object([]) do |element, arr| + event_date, event_count = element + + # The `event_date` is in Date format (without time), such as "Wed, 15 May 2024". + # We need a timestamp for the start of the day. However, we can't use `event_date.to_time.to_i` + # because it converts the date to 12:00 AM server timezone. + # The desired output should be 12:00 AM in the specified timezone. + arr << { value: event_count, timestamp: event_date.in_time_zone(timezone).to_i } + end + end + + def aggregate_value + object_scope.count + end + + private + + def metric + @metric ||= params[:metric] + end + + def object_scope + send("scope_for_#{metric}") + end + + def scope_for_conversations_count + scope.conversations.where(account_id: account.id, created_at: range) + end + + def scope_for_incoming_messages_count + scope.messages.where(account_id: account.id, created_at: range).incoming.unscope(:order) + end + + def scope_for_outgoing_messages_count + scope.messages.where(account_id: account.id, created_at: range).outgoing.unscope(:order) + end + + def scope_for_resolutions_count + scope.reporting_events.joins(:conversation).select(:conversation_id).where( + name: :conversation_resolved, + conversations: { status: :resolved }, created_at: range + ).distinct + end + + def scope_for_bot_resolutions_count + scope.reporting_events.joins(:conversation).select(:conversation_id).where( + name: :conversation_bot_resolved, + conversations: { status: :resolved }, created_at: range + ).distinct + end + + def scope_for_bot_handoffs_count + scope.reporting_events.joins(:conversation).select(:conversation_id).where( + name: :conversation_bot_handoff, + created_at: range + ).distinct + end + + def grouped_count + @grouped_values = object_scope.group_by_period( + group_by, + :created_at, + default_value: 0, + range: range, + permit: %w[day week month year hour], + time_zone: timezone + ).count + end +end diff --git a/app/controllers/api/v2/accounts/reports_controller.rb b/app/controllers/api/v2/accounts/reports_controller.rb index c67b74a43..ed5be5518 100644 --- a/app/controllers/api/v2/accounts/reports_controller.rb +++ b/app/controllers/api/v2/accounts/reports_controller.rb @@ -5,19 +5,17 @@ class Api::V2::Accounts::ReportsController < Api::V1::Accounts::BaseController before_action :check_authorization def index - builder = V2::ReportBuilder.new(Current.account, report_params) - data = builder.build + builder = V2::Reports::Conversations::ReportBuilder.new(Current.account, report_params) + data = builder.timeseries render json: data end def summary - render json: summary_metrics + render json: build_summary(:summary) end def bot_summary - summary = V2::ReportBuilder.new(Current.account, current_summary_params).bot_summary - summary[:previous] = V2::ReportBuilder.new(Current.account, previous_summary_params).bot_summary - render json: summary + render json: build_summary(:bot_summary) end def agents @@ -126,10 +124,11 @@ class Api::V2::Accounts::ReportsController < Api::V1::Accounts::BaseController } end - def summary_metrics - summary = V2::ReportBuilder.new(Current.account, current_summary_params).summary - summary[:previous] = V2::ReportBuilder.new(Current.account, previous_summary_params).summary - summary + def build_summary(method) + builder = V2::Reports::Conversations::MetricBuilder + current_summary = builder.new(Current.account, current_summary_params).send(method) + previous_summary = builder.new(Current.account, previous_summary_params).send(method) + current_summary.merge(previous: previous_summary) end def conversation_metrics diff --git a/app/helpers/timezone_helper.rb b/app/helpers/timezone_helper.rb new file mode 100644 index 000000000..b016cc9d9 --- /dev/null +++ b/app/helpers/timezone_helper.rb @@ -0,0 +1,19 @@ +module TimezoneHelper + # ActiveSupport TimeZone is not aware of the current time, so ActiveSupport::Timezone[offset] + # would return the timezone without considering day light savings. To get the correct timezone, + # this method uses zone.now.utc_offset for comparison as referenced in the issues below + # + # https://github.com/rails/rails/pull/22243 + # https://github.com/rails/rails/issues/21501 + # https://github.com/rails/rails/issues/7297 + def timezone_name_from_offset(offset) + return 'UTC' if offset.blank? + + offset_in_seconds = offset.to_f * 3600 + matching_zone = ActiveSupport::TimeZone.all.find do |zone| + zone.now.utc_offset == offset_in_seconds + end + + return matching_zone.name if matching_zone + end +end diff --git a/spec/builders/v2/reports/conversations/metric_builder_spec.rb b/spec/builders/v2/reports/conversations/metric_builder_spec.rb new file mode 100644 index 000000000..1b0ed7a38 --- /dev/null +++ b/spec/builders/v2/reports/conversations/metric_builder_spec.rb @@ -0,0 +1,50 @@ +require 'rails_helper' + +RSpec.describe V2::Reports::Conversations::MetricBuilder, type: :model do + subject { described_class.new(account, params) } + + let(:account) { create(:account) } + let(:params) { { since: '2023-01-01', until: '2024-01-01' } } + let(:count_builder_instance) { instance_double(V2::Reports::Timeseries::CountReportBuilder, aggregate_value: 42) } + let(:avg_builder_instance) { instance_double(V2::Reports::Timeseries::AverageReportBuilder, aggregate_value: 42) } + + before do + allow(V2::Reports::Timeseries::CountReportBuilder).to receive(:new).and_return(count_builder_instance) + allow(V2::Reports::Timeseries::AverageReportBuilder).to receive(:new).and_return(avg_builder_instance) + end + + describe '#summary' do + it 'returns the correct summary values' do + summary = subject.summary + expect(summary).to eq( + { + conversations_count: 42, + incoming_messages_count: 42, + outgoing_messages_count: 42, + avg_first_response_time: 42, + avg_resolution_time: 42, + resolutions_count: 42, + reply_time: 42 + } + ) + end + + it 'creates builders with proper params' do + subject.summary + expect(V2::Reports::Timeseries::CountReportBuilder).to have_received(:new).with(account, params.merge(metric: 'conversations_count')) + expect(V2::Reports::Timeseries::AverageReportBuilder).to have_received(:new).with(account, params.merge(metric: 'avg_first_response_time')) + end + end + + describe '#bot_summary' do + it 'returns a detailed summary of bot-specific conversation metrics' do + bot_summary = subject.bot_summary + expect(bot_summary).to eq( + { + bot_resolutions_count: 42, + bot_handoffs_count: 42 + } + ) + end + end +end diff --git a/spec/builders/v2/reports/conversations/report_builder_spec.rb b/spec/builders/v2/reports/conversations/report_builder_spec.rb new file mode 100644 index 000000000..d3cde98e6 --- /dev/null +++ b/spec/builders/v2/reports/conversations/report_builder_spec.rb @@ -0,0 +1,44 @@ +require 'rails_helper' + +describe V2::Reports::Conversations::ReportBuilder do + subject { described_class.new(account, params) } + + let(:account) { create(:account) } + let(:average_builder) { V2::Reports::Timeseries::AverageReportBuilder } + let(:count_builder) { V2::Reports::Timeseries::CountReportBuilder } + + shared_examples 'valid metric handler' do |metric, method, builder| + context 'when a valid metric is given' do + let(:params) { { metric: metric } } + + it "calls the correct #{method} builder for #{metric}" do + builder_instance = instance_double(builder) + allow(builder).to receive(:new).and_return(builder_instance) + allow(builder_instance).to receive(method) + + builder_instance.public_send(method) + expect(builder_instance).to have_received(method) + end + end + end + + context 'when invalid metric is given' do + let(:metric) { 'invalid_metric' } + let(:params) { { metric: metric } } + + it 'logs the error and returns empty value' do + expect(Rails.logger).to receive(:error).with("ReportBuilder: Invalid metric - #{metric}") + expect(subject.timeseries).to eq({}) + end + end + + describe '#timeseries' do + include_examples 'valid metric handler', 'avg_first_response_time', :timeseries, V2::Reports::Timeseries::AverageReportBuilder + include_examples 'valid metric handler', 'conversations_count', :timeseries, V2::Reports::Timeseries::CountReportBuilder + end + + describe '#aggregate_value' do + include_examples 'valid metric handler', 'avg_first_response_time', :aggregate_value, V2::Reports::Timeseries::AverageReportBuilder + include_examples 'valid metric handler', 'conversations_count', :aggregate_value, V2::Reports::Timeseries::CountReportBuilder + end +end diff --git a/spec/builders/v2/reports/timeseries/average_report_builder_spec.rb b/spec/builders/v2/reports/timeseries/average_report_builder_spec.rb new file mode 100644 index 000000000..4f6036f07 --- /dev/null +++ b/spec/builders/v2/reports/timeseries/average_report_builder_spec.rb @@ -0,0 +1,174 @@ +require 'rails_helper' + +describe V2::Reports::Timeseries::AverageReportBuilder do + subject { described_class.new(account, params) } + + let(:account) { create(:account) } + let(:team) { create(:team, account: account) } + let(:inbox) { create(:inbox, account: account) } + let(:label) { create(:label, title: 'spec-billing', account: account) } + let!(:conversation) { create(:conversation, account: account, inbox: inbox, team: team) } + let(:current_time) { '26.10.2020 10:00'.to_datetime } + + let(:params) do + { + type: filter_type, + business_hours: business_hours, + timezone_offset: timezone_offset, + group_by: group_by, + metric: metric, + since: (current_time - 1.week).beginning_of_day.to_i.to_s, + until: current_time.end_of_day.to_i.to_s, + id: filter_id + } + end + let(:timezone_offset) { nil } + let(:group_by) { 'day' } + let(:metric) { 'avg_first_response_time' } + let(:business_hours) { false } + let(:filter_type) { :account } + let(:filter_id) { '' } + + before do + travel_to current_time + conversation.label_list.add(label.title) + conversation.save! + create(:reporting_event, name: 'first_response', value: 80, value_in_business_hours: 10, account: account, created_at: Time.zone.now, + conversation: conversation, inbox: inbox) + create(:reporting_event, name: 'first_response', value: 100, value_in_business_hours: 20, account: account, created_at: 1.hour.ago) + create(:reporting_event, name: 'first_response', value: 93, value_in_business_hours: 30, account: account, created_at: 1.week.ago) + end + + describe '#timeseries' do + context 'when there is no filter applied' do + it 'returns the correct values' do + timeseries_values = subject.timeseries + + expect(timeseries_values).to eq( + [ + { count: 1, timestamp: 1_603_065_600, value: 93.0 }, + { count: 0, timestamp: 1_603_152_000, value: 0 }, + { count: 0, timestamp: 1_603_238_400, value: 0 }, + { count: 0, timestamp: 1_603_324_800, value: 0 }, + { count: 0, timestamp: 1_603_411_200, value: 0 }, + { count: 0, timestamp: 1_603_497_600, value: 0 }, + { count: 0, timestamp: 1_603_584_000, value: 0 }, + { count: 2, timestamp: 1_603_670_400, value: 90.0 } + ] + ) + end + + context 'when business hours is provided' do + let(:business_hours) { true } + + it 'returns correct timeseries' do + timeseries_values = subject.timeseries + + expect(timeseries_values).to eq( + [ + { count: 1, timestamp: 1_603_065_600, value: 30.0 }, + { count: 0, timestamp: 1_603_152_000, value: 0 }, + { count: 0, timestamp: 1_603_238_400, value: 0 }, + { count: 0, timestamp: 1_603_324_800, value: 0 }, + { count: 0, timestamp: 1_603_411_200, value: 0 }, + { count: 0, timestamp: 1_603_497_600, value: 0 }, + { count: 0, timestamp: 1_603_584_000, value: 0 }, + { count: 2, timestamp: 1_603_670_400, value: 15.0 } + ] + ) + end + end + + context 'when group_by is provided' do + let(:group_by) { 'week' } + + it 'returns correct timeseries' do + timeseries_values = subject.timeseries + expect(timeseries_values).to eq( + [ + { count: 1, timestamp: (current_time - 1.week).beginning_of_week(:sunday).to_i, value: 93.0 }, + { count: 2, timestamp: current_time.beginning_of_week(:sunday).to_i, value: 90.0 } + ] + ) + end + end + + context 'when timezone offset is provided' do + let(:timezone_offset) { '5.5' } + let(:group_by) { 'week' } + + it 'returns correct timeseries' do + timeseries_values = subject.timeseries + expect(timeseries_values).to eq( + [ + { count: 1, timestamp: (current_time - 1.week).in_time_zone('Chennai').beginning_of_week(:sunday).to_i, value: 93.0 }, + { count: 2, timestamp: current_time.in_time_zone('Chennai').beginning_of_week(:sunday).to_i, value: 90.0 } + ] + ) + end + end + end + + context 'when the label filter is applied' do + let(:group_by) { 'week' } + let(:filter_type) { 'label' } + let(:filter_id) { label.id } + + it 'returns correct timeseries' do + timeseries_values = subject.timeseries + start_of_the_week = current_time.beginning_of_week(:sunday).to_i + last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i + expect(timeseries_values).to eq( + [ + { count: 0, timestamp: last_week_start_of_the_week, value: 0 }, + { count: 1, timestamp: start_of_the_week, value: 80.0 } + ] + ) + end + end + + context 'when the inbox filter is applied' do + let(:group_by) { 'week' } + let(:filter_type) { 'inbox' } + let(:filter_id) { inbox.id } + + it 'returns correct timeseries' do + timeseries_values = subject.timeseries + start_of_the_week = current_time.beginning_of_week(:sunday).to_i + last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i + expect(timeseries_values).to eq( + [ + { count: 0, timestamp: last_week_start_of_the_week, value: 0 }, + { count: 1, timestamp: start_of_the_week, value: 80.0 } + ] + ) + end + end + + context 'when the team filter is applied' do + let(:group_by) { 'week' } + let(:filter_type) { 'team' } + let(:filter_id) { team.id } + + it 'returns correct timeseries' do + timeseries_values = subject.timeseries + start_of_the_week = current_time.beginning_of_week(:sunday).to_i + last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i + expect(timeseries_values).to eq( + [ + { count: 0, timestamp: last_week_start_of_the_week, value: 0 }, + { count: 1, timestamp: start_of_the_week, value: 80.0 } + ] + ) + end + end + end + + describe '#aggregate_value' do + context 'when there is no filter applied' do + it 'returns the correct average value' do + expect(subject.aggregate_value).to eq 91.0 + end + end + end +end diff --git a/spec/controllers/api/v2/accounts/report_controller_spec.rb b/spec/controllers/api/v2/accounts/report_controller_spec.rb index fc0228c86..6202946a1 100644 --- a/spec/controllers/api/v2/accounts/report_controller_spec.rb +++ b/spec/controllers/api/v2/accounts/report_controller_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'Reports API', type: :request do let!(:user) { create(:user, account: account) } let!(:inbox) { create(:inbox, account: account) } let(:inbox_member) { create(:inbox_member, user: user, inbox: inbox) } - let(:default_timezone) { ActiveSupport::TimeZone[0]&.name } + let(:default_timezone) { 'UTC' } let(:start_of_today) { Time.current.in_time_zone(default_timezone).beginning_of_day.to_i } let(:end_of_today) { Time.current.in_time_zone(default_timezone).end_of_day.to_i } let(:params) { { timezone_offset: Time.zone.utc_offset } } @@ -18,7 +18,7 @@ RSpec.describe 'Reports API', type: :request do assignee: user, created_at: Time.current.in_time_zone(default_timezone).to_date) end - describe 'GET /api/v2/accounts/:account_id/reports/account' do + describe 'GET /api/v2/accounts/:account_id/reports' do context 'when it is an unauthenticated user' do it 'returns unauthorized' do get "/api/v2/accounts/#{account.id}/reports" diff --git a/spec/factories/reporting_events.rb b/spec/factories/reporting_events.rb index c779c01f6..1db6a87df 100644 --- a/spec/factories/reporting_events.rb +++ b/spec/factories/reporting_events.rb @@ -2,6 +2,7 @@ FactoryBot.define do factory :reporting_event do name { 'MyString' } value { 1.5 } + value_in_business_hours { 1 } account_id { 1 } inbox_id { 1 } user_id { 1 } From 4b93738462dd3f61f0fe3edbe04ca011d0bf2983 Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Thu, 23 May 2024 10:40:44 +0530 Subject: [PATCH 02/33] fix: Space key in input closing dropdown (#9525) --- .../dashboard/components/ui/Dropdown/DropdownSearch.vue | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/javascript/dashboard/components/ui/Dropdown/DropdownSearch.vue b/app/javascript/dashboard/components/ui/Dropdown/DropdownSearch.vue index 558816deb..c153a82a7 100644 --- a/app/javascript/dashboard/components/ui/Dropdown/DropdownSearch.vue +++ b/app/javascript/dashboard/components/ui/Dropdown/DropdownSearch.vue @@ -18,11 +18,11 @@ defineProps({
-
+
Date: Thu, 23 May 2024 11:22:14 +0530 Subject: [PATCH 03/33] fix: TypeError cannot read properties of undefined (reading 'status') (#9505) --- app/javascript/dashboard/components/ChatList.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/dashboard/components/ChatList.vue b/app/javascript/dashboard/components/ChatList.vue index 60966b476..5b592a1f7 100644 --- a/app/javascript/dashboard/components/ChatList.vue +++ b/app/javascript/dashboard/components/ChatList.vue @@ -987,7 +987,7 @@ export default { allSelectedConversationsStatus(status) { if (!this.selectedConversations.length) return false; return this.selectedConversations.every(item => { - return this.$store.getters.getConversationById(item).status === status; + return this.$store.getters.getConversationById(item)?.status === status; }); }, onContextMenuToggle(state) { From 35508feaae4808b730c908687fa9e16b9cba3a91 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Thu, 23 May 2024 11:58:24 +0530 Subject: [PATCH 04/33] feat: Linear front end (#9491) Co-authored-by: Shivam Mishra Co-authored-by: iamsivin --- .../integrations/linear_controller.rb | 2 +- .../dashboard/api/integrations/linear.js | 46 ++++ .../api/specs/integrations/linear.spec.js | 202 +++++++++++++++ .../components/buttons/ResolveAction.vue | 2 +- .../components/ui/Dropdown/DropdownList.vue | 7 +- .../conversation/ConversationHeader.vue | 21 ++ .../conversation/linear/CreateIssue.vue | 232 ++++++++++++++++++ .../conversation/linear/CreateOrLinkIssue.vue | 79 ++++++ .../widgets/conversation/linear/Issue.vue | 123 ++++++++++ .../conversation/linear/IssueHeader.vue | 56 +++++ .../widgets/conversation/linear/LinkIssue.vue | 127 ++++++++++ .../widgets/conversation/linear/index.vue | 117 +++++++++ .../conversation/linear/validations.js | 10 + app/javascript/dashboard/featureFlags.js | 1 + .../i18n/locale/en/integrations.json | 68 +++++ .../dashboard/conversation/ContactPanel.vue | 1 - .../FluentIcon/dashboard-icons.json | 5 +- lib/linear.rb | 3 +- 18 files changed, 1095 insertions(+), 7 deletions(-) create mode 100644 app/javascript/dashboard/api/integrations/linear.js create mode 100644 app/javascript/dashboard/api/specs/integrations/linear.spec.js create mode 100644 app/javascript/dashboard/components/widgets/conversation/linear/CreateIssue.vue create mode 100644 app/javascript/dashboard/components/widgets/conversation/linear/CreateOrLinkIssue.vue create mode 100644 app/javascript/dashboard/components/widgets/conversation/linear/Issue.vue create mode 100644 app/javascript/dashboard/components/widgets/conversation/linear/IssueHeader.vue create mode 100644 app/javascript/dashboard/components/widgets/conversation/linear/LinkIssue.vue create mode 100644 app/javascript/dashboard/components/widgets/conversation/linear/index.vue create mode 100644 app/javascript/dashboard/components/widgets/conversation/linear/validations.js diff --git a/app/controllers/api/v1/accounts/integrations/linear_controller.rb b/app/controllers/api/v1/accounts/integrations/linear_controller.rb index 9d5d76d75..814373c7e 100644 --- a/app/controllers/api/v1/accounts/integrations/linear_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/linear_controller.rb @@ -88,6 +88,6 @@ class Api::V1::Accounts::Integrations::LinearController < Api::V1::Accounts::Bas end def permitted_params - params.permit(:team_id, :conversation_id, :issue_id, :link_id, :title, :description, :assignee_id, :priority, label_ids: []) + params.permit(:team_id, :project_id, :conversation_id, :issue_id, :link_id, :title, :description, :assignee_id, :priority, label_ids: []) end end diff --git a/app/javascript/dashboard/api/integrations/linear.js b/app/javascript/dashboard/api/integrations/linear.js new file mode 100644 index 000000000..600f169d6 --- /dev/null +++ b/app/javascript/dashboard/api/integrations/linear.js @@ -0,0 +1,46 @@ +/* global axios */ + +import ApiClient from '../ApiClient'; + +class LinearAPI extends ApiClient { + constructor() { + super('integrations/linear', { accountScoped: true }); + } + + getTeams() { + return axios.get(`${this.url}/teams`); + } + + getTeamEntities(teamId) { + return axios.get(`${this.url}/team_entities?team_id=${teamId}`); + } + + createIssue(data) { + return axios.post(`${this.url}/create_issue`, data); + } + + link_issue(conversationId, issueId) { + return axios.post(`${this.url}/link_issue`, { + issue_id: issueId, + conversation_id: conversationId, + }); + } + + getLinkedIssue(conversationId) { + return axios.get( + `${this.url}/linked_issues?conversation_id=${conversationId}` + ); + } + + unlinkIssue(linkId) { + return axios.post(`${this.url}/unlink_issue`, { + link_id: linkId, + }); + } + + searchIssues(query) { + return axios.get(`${this.url}/search_issue?q=${query}`); + } +} + +export default new LinearAPI(); diff --git a/app/javascript/dashboard/api/specs/integrations/linear.spec.js b/app/javascript/dashboard/api/specs/integrations/linear.spec.js new file mode 100644 index 000000000..cc16feb16 --- /dev/null +++ b/app/javascript/dashboard/api/specs/integrations/linear.spec.js @@ -0,0 +1,202 @@ +import LinearAPIClient from '../../integrations/linear'; +import ApiClient from '../../ApiClient'; + +describe('#linearAPI', () => { + it('creates correct instance', () => { + expect(LinearAPIClient).toBeInstanceOf(ApiClient); + expect(LinearAPIClient).toHaveProperty('getTeams'); + expect(LinearAPIClient).toHaveProperty('getTeamEntities'); + expect(LinearAPIClient).toHaveProperty('createIssue'); + expect(LinearAPIClient).toHaveProperty('link_issue'); + expect(LinearAPIClient).toHaveProperty('getLinkedIssue'); + expect(LinearAPIClient).toHaveProperty('unlinkIssue'); + expect(LinearAPIClient).toHaveProperty('searchIssues'); + }); + + describe('getTeams', () => { + const originalAxios = window.axios; + const axiosMock = { + post: jest.fn(() => Promise.resolve()), + get: jest.fn(() => Promise.resolve()), + patch: jest.fn(() => Promise.resolve()), + delete: jest.fn(() => Promise.resolve()), + }; + + beforeEach(() => { + window.axios = axiosMock; + }); + + afterEach(() => { + window.axios = originalAxios; + }); + + it('creates a valid request', () => { + LinearAPIClient.getTeams(); + expect(axiosMock.get).toHaveBeenCalledWith( + '/api/v1/integrations/linear/teams' + ); + }); + }); + + describe('getTeamEntities', () => { + const originalAxios = window.axios; + const axiosMock = { + post: jest.fn(() => Promise.resolve()), + get: jest.fn(() => Promise.resolve()), + patch: jest.fn(() => Promise.resolve()), + delete: jest.fn(() => Promise.resolve()), + }; + + beforeEach(() => { + window.axios = axiosMock; + }); + + afterEach(() => { + window.axios = originalAxios; + }); + + it('creates a valid request', () => { + LinearAPIClient.getTeamEntities(1); + expect(axiosMock.get).toHaveBeenCalledWith( + '/api/v1/integrations/linear/team_entities?team_id=1' + ); + }); + }); + + describe('createIssue', () => { + const originalAxios = window.axios; + const axiosMock = { + post: jest.fn(() => Promise.resolve()), + get: jest.fn(() => Promise.resolve()), + patch: jest.fn(() => Promise.resolve()), + delete: jest.fn(() => Promise.resolve()), + }; + + beforeEach(() => { + window.axios = axiosMock; + }); + + afterEach(() => { + window.axios = originalAxios; + }); + + it('creates a valid request', () => { + const issueData = { + title: 'New Issue', + description: 'Issue description', + }; + LinearAPIClient.createIssue(issueData); + expect(axiosMock.post).toHaveBeenCalledWith( + '/api/v1/integrations/linear/create_issue', + issueData + ); + }); + }); + + describe('link_issue', () => { + const originalAxios = window.axios; + const axiosMock = { + post: jest.fn(() => Promise.resolve()), + get: jest.fn(() => Promise.resolve()), + patch: jest.fn(() => Promise.resolve()), + delete: jest.fn(() => Promise.resolve()), + }; + + beforeEach(() => { + window.axios = axiosMock; + }); + + afterEach(() => { + window.axios = originalAxios; + }); + + it('creates a valid request', () => { + LinearAPIClient.link_issue(1, 2); + expect(axiosMock.post).toHaveBeenCalledWith( + '/api/v1/integrations/linear/link_issue', + { + issue_id: 2, + conversation_id: 1, + } + ); + }); + }); + + describe('getLinkedIssue', () => { + const originalAxios = window.axios; + const axiosMock = { + post: jest.fn(() => Promise.resolve()), + get: jest.fn(() => Promise.resolve()), + patch: jest.fn(() => Promise.resolve()), + delete: jest.fn(() => Promise.resolve()), + }; + + beforeEach(() => { + window.axios = axiosMock; + }); + + afterEach(() => { + window.axios = originalAxios; + }); + + it('creates a valid request', () => { + LinearAPIClient.getLinkedIssue(1); + expect(axiosMock.get).toHaveBeenCalledWith( + '/api/v1/integrations/linear/linked_issues?conversation_id=1' + ); + }); + }); + + describe('unlinkIssue', () => { + const originalAxios = window.axios; + const axiosMock = { + post: jest.fn(() => Promise.resolve()), + get: jest.fn(() => Promise.resolve()), + patch: jest.fn(() => Promise.resolve()), + delete: jest.fn(() => Promise.resolve()), + }; + + beforeEach(() => { + window.axios = axiosMock; + }); + + afterEach(() => { + window.axios = originalAxios; + }); + + it('creates a valid request', () => { + LinearAPIClient.unlinkIssue(1); + expect(axiosMock.post).toHaveBeenCalledWith( + '/api/v1/integrations/linear/unlink_issue', + { + link_id: 1, + } + ); + }); + }); + + describe('searchIssues', () => { + const originalAxios = window.axios; + const axiosMock = { + post: jest.fn(() => Promise.resolve()), + get: jest.fn(() => Promise.resolve()), + patch: jest.fn(() => Promise.resolve()), + delete: jest.fn(() => Promise.resolve()), + }; + + beforeEach(() => { + window.axios = axiosMock; + }); + + afterEach(() => { + window.axios = originalAxios; + }); + + it('creates a valid request', () => { + LinearAPIClient.searchIssues('query'); + expect(axiosMock.get).toHaveBeenCalledWith( + '/api/v1/integrations/linear/search_issue?q=query' + ); + }); + }); +}); diff --git a/app/javascript/dashboard/components/buttons/ResolveAction.vue b/app/javascript/dashboard/components/buttons/ResolveAction.vue index c56f27e14..91423c762 100644 --- a/app/javascript/dashboard/components/buttons/ResolveAction.vue +++ b/app/javascript/dashboard/components/buttons/ResolveAction.vue @@ -1,5 +1,5 @@