From 0e721653e5edd31bc56b1ad8c4751a7ca8ef78ef Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Tue, 23 Feb 2021 12:11:15 +0530 Subject: [PATCH] feat: Business hour Inbox APIs (#1821) * feat: Business hour Inbox APIs --- .../api/v1/accounts/inboxes_controller.rb | 3 +- app/models/account.rb | 1 - app/models/concerns/out_of_offisable.rb | 24 +++++++++++++- app/models/inbox.rb | 2 ++ app/models/working_hour.rb | 2 +- .../hook_execution_service.rb | 3 +- app/views/api/v1/models/_inbox.json.jbuilder | 4 +++ ...0222131048_change_working_hours_to_zero.rb | 11 +++++++ ..._switch_time_zone_from_account_to_inbox.rb | 6 ++++ db/schema.rb | 4 +-- lib/current.rb | 5 --- .../v1/accounts/inboxes_controller_spec.rb | 33 ++++++++++++++----- spec/models/concerns/out_of_offisable_spec.rb | 6 ++++ .../hook_execution_service_spec.rb | 5 +-- 14 files changed, 86 insertions(+), 23 deletions(-) create mode 100644 db/migrate/20210222131048_change_working_hours_to_zero.rb create mode 100644 db/migrate/20210222131155_switch_time_zone_from_account_to_inbox.rb diff --git a/app/controllers/api/v1/accounts/inboxes_controller.rb b/app/controllers/api/v1/accounts/inboxes_controller.rb index 6c078d28f..7928a3d41 100644 --- a/app/controllers/api/v1/accounts/inboxes_controller.rb +++ b/app/controllers/api/v1/accounts/inboxes_controller.rb @@ -23,6 +23,7 @@ class Api::V1::Accounts::InboxesController < Api::V1::Accounts::BaseController def update @inbox.update(inbox_update_params.except(:channel)) + @inbox.update_working_hours(params.permit(working_hours: Inbox::OFFISABLE_ATTRS)[:working_hours]) if params[:working_hours] return unless @inbox.channel.is_a?(Channel::WebWidget) && inbox_update_params[:channel].present? @inbox.channel.update!(inbox_update_params[:channel]) @@ -80,7 +81,7 @@ class Api::V1::Accounts::InboxesController < Api::V1::Accounts::BaseController def inbox_update_params params.permit(:enable_auto_assignment, :name, :avatar, :greeting_message, :greeting_enabled, - :working_hours_enabled, :out_of_office_message, + :working_hours_enabled, :out_of_office_message, :timezone, channel: [ :website_url, :widget_color, diff --git a/app/models/account.rb b/app/models/account.rb index 1bb9f1a19..d6f004777 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -10,7 +10,6 @@ # name :string not null # settings_flags :integer default(0), not null # support_email :string(100) -# timezone :string default("UTC") # created_at :datetime not null # updated_at :datetime not null # diff --git a/app/models/concerns/out_of_offisable.rb b/app/models/concerns/out_of_offisable.rb index 0bfcbde32..72d1e0467 100644 --- a/app/models/concerns/out_of_offisable.rb +++ b/app/models/concerns/out_of_offisable.rb @@ -3,6 +3,8 @@ module OutOfOffisable extend ActiveSupport::Concern + OFFISABLE_ATTRS = %w[day_of_week closed_all_day open_hour open_minutes close_hour close_minutes].freeze + included do has_many :working_hours, dependent: :destroy after_create :create_default_working_hours @@ -16,15 +18,35 @@ module OutOfOffisable !out_of_office? end + def weekly_schedule + working_hours.select(*OFFISABLE_ATTRS).as_json(except: :id) + end + + # accepts an array of hashes similiar to the format of weekly_schedule + # [ + # { "day_of_week"=>1, + # "closed_all_day"=>false, + # "open_hour"=>9, + # "open_minutes"=>0, + # "close_hour"=>17, + # "close_minutes"=>0},...] + def update_working_hours(params) + ActiveRecord::Base.transaction do + params.each do |working_hour| + working_hours.find_by(day_of_week: working_hour['day_of_week']).update(working_hour.slice(*OFFISABLE_ATTRS)) + end + end + end + private def create_default_working_hours + working_hours.create!(day_of_week: 0, closed_all_day: true) working_hours.create!(day_of_week: 1, open_hour: 9, open_minutes: 0, close_hour: 17, close_minutes: 0) working_hours.create!(day_of_week: 2, open_hour: 9, open_minutes: 0, close_hour: 17, close_minutes: 0) working_hours.create!(day_of_week: 3, open_hour: 9, open_minutes: 0, close_hour: 17, close_minutes: 0) working_hours.create!(day_of_week: 4, open_hour: 9, open_minutes: 0, close_hour: 17, close_minutes: 0) working_hours.create!(day_of_week: 5, open_hour: 9, open_minutes: 0, close_hour: 17, close_minutes: 0) working_hours.create!(day_of_week: 6, closed_all_day: true) - working_hours.create!(day_of_week: 7, closed_all_day: true) end end diff --git a/app/models/inbox.rb b/app/models/inbox.rb index 0e5fd490f..398af0352 100644 --- a/app/models/inbox.rb +++ b/app/models/inbox.rb @@ -12,6 +12,7 @@ # greeting_message :string # name :string not null # out_of_office_message :string +# timezone :string default("UTC") # working_hours_enabled :boolean default(FALSE) # created_at :datetime not null # updated_at :datetime not null @@ -29,6 +30,7 @@ class Inbox < ApplicationRecord include OutOfOffisable validates :account_id, presence: true + validates :timezone, inclusion: { in: TZInfo::Timezone.all_identifiers } belongs_to :account diff --git a/app/models/working_hour.rb b/app/models/working_hour.rb index e73759ac0..5874af20d 100644 --- a/app/models/working_hour.rb +++ b/app/models/working_hour.rb @@ -37,7 +37,7 @@ class WorkingHour < ApplicationRecord validate :close_after_open, unless: :closed_all_day? def self.today - find_by(day_of_week: Date.current.cwday) + find_by(day_of_week: Date.current.wday) end def open_at?(time) diff --git a/app/services/message_templates/hook_execution_service.rb b/app/services/message_templates/hook_execution_service.rb index f8371aad9..05429127e 100644 --- a/app/services/message_templates/hook_execution_service.rb +++ b/app/services/message_templates/hook_execution_service.rb @@ -5,7 +5,8 @@ class MessageTemplates::HookExecutionService return if inbox.agent_bot_inbox&.active? ::MessageTemplates::Template::OutOfOffice.new(conversation: conversation).perform if should_send_out_of_office_message? - ::MessageTemplates::Template::Greeting.new(conversation: conversation).perform if should_send_greeting? + # TODO: let's see whether this is needed and remove this and related logic if not + #::MessageTemplates::Template::Greeting.new(conversation: conversation).perform if should_send_greeting? ::MessageTemplates::Template::EmailCollect.new(conversation: conversation).perform if should_send_email_collect? end diff --git a/app/views/api/v1/models/_inbox.json.jbuilder b/app/views/api/v1/models/_inbox.json.jbuilder index 0ceaf8c5b..cee4e429d 100644 --- a/app/views/api/v1/models/_inbox.json.jbuilder +++ b/app/views/api/v1/models/_inbox.json.jbuilder @@ -4,6 +4,10 @@ json.name resource.name json.channel_type resource.channel_type json.greeting_enabled resource.greeting_enabled json.greeting_message resource.greeting_message +json.working_hours_enabled resource.working_hours_enabled +json.out_of_office_message resource.out_of_office_message +json.working_hours resource.weekly_schedule +json.timezone resource.timezone json.avatar_url resource.try(:avatar_url) json.page_id resource.channel.try(:page_id) json.widget_color resource.channel.try(:widget_color) diff --git a/db/migrate/20210222131048_change_working_hours_to_zero.rb b/db/migrate/20210222131048_change_working_hours_to_zero.rb new file mode 100644 index 000000000..7d163e55e --- /dev/null +++ b/db/migrate/20210222131048_change_working_hours_to_zero.rb @@ -0,0 +1,11 @@ +class ChangeWorkingHoursToZero < ActiveRecord::Migration[6.0] + # rubocop:disable Rails/SkipsModelValidations + def up + WorkingHour.where(day_of_week: 7).update_all(day_of_week: 0) + end + + def down + WorkingHour.where(day_of_week: 0).update_all(day_of_week: 7) + end + # rubocop:enable Rails/SkipsModelValidations +end diff --git a/db/migrate/20210222131155_switch_time_zone_from_account_to_inbox.rb b/db/migrate/20210222131155_switch_time_zone_from_account_to_inbox.rb new file mode 100644 index 000000000..b30d3dffc --- /dev/null +++ b/db/migrate/20210222131155_switch_time_zone_from_account_to_inbox.rb @@ -0,0 +1,6 @@ +class SwitchTimeZoneFromAccountToInbox < ActiveRecord::Migration[6.0] + def change + remove_column :accounts, :timezone, :string, default: 'UTC' + add_column :inboxes, :timezone, :string, default: 'UTC' + end +end diff --git a/db/schema.rb b/db/schema.rb index bf0f57714..db05d6392 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_02_19_085719) do +ActiveRecord::Schema.define(version: 2021_02_22_131155) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" @@ -50,7 +50,6 @@ ActiveRecord::Schema.define(version: 2021_02_19_085719) do t.integer "settings_flags", default: 0, null: false t.integer "feature_flags", default: 0, null: false t.integer "auto_resolve_duration" - t.string "timezone", default: "UTC" end create_table "action_mailbox_inbound_emails", force: :cascade do |t| @@ -303,6 +302,7 @@ ActiveRecord::Schema.define(version: 2021_02_19_085719) do t.string "email_address" t.boolean "working_hours_enabled", default: false t.string "out_of_office_message" + t.string "timezone", default: "UTC" t.index ["account_id"], name: "index_inboxes_on_account_id" end diff --git a/lib/current.rb b/lib/current.rb index 4490932f0..bfa380f43 100644 --- a/lib/current.rb +++ b/lib/current.rb @@ -3,11 +3,6 @@ module Current thread_mattr_accessor :account thread_mattr_accessor :account_user - def account=(account) - super - Time.zone = account.timezone - end - def self.reset Current.user = nil Current.account = nil diff --git a/spec/controllers/api/v1/accounts/inboxes_controller_spec.rb b/spec/controllers/api/v1/accounts/inboxes_controller_spec.rb index 3b01c36fc..0c52e869c 100644 --- a/spec/controllers/api/v1/accounts/inboxes_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/inboxes_controller_spec.rb @@ -141,7 +141,18 @@ RSpec.describe 'Inboxes API', type: :request do let(:admin) { create(:user, account: account, role: :administrator) } let(:valid_params) { { enable_auto_assignment: false, channel: { website_url: 'test.com' } } } - it 'updates inbox' do + it 'will not update inbox for agent' do + agent = create(:user, account: account, role: :agent) + + patch "/api/v1/accounts/#{account.id}/inboxes/#{inbox.id}", + headers: agent.create_new_auth_token, + params: valid_params, + as: :json + + expect(response).to have_http_status(:unauthorized) + end + + it 'updates inbox when administrator' do patch "/api/v1/accounts/#{account.id}/inboxes/#{inbox.id}", headers: admin.create_new_auth_token, params: valid_params, @@ -151,7 +162,7 @@ RSpec.describe 'Inboxes API', type: :request do expect(inbox.reload.enable_auto_assignment).to be_falsey end - it 'updates avatar' do + it 'updates avatar when administrator' do # no avatar before upload expect(inbox.avatar.attached?).to eq(false) file = fixture_file_upload(Rails.root.join('spec/assets/avatar.png'), 'image/png') @@ -165,15 +176,19 @@ RSpec.describe 'Inboxes API', type: :request do expect(inbox.avatar.attached?).to eq(true) end - it 'will not update inbox for agent' do - agent = create(:user, account: account, role: :agent) - + it 'updates working hours when administrator' do + params = { + working_hours: [{ 'day_of_week' => 0, 'open_hour' => 9, 'open_minutes' => 0, 'close_hour' => 17, 'close_minutes' => 0 }], + working_hours_enabled: true, + out_of_office_message: 'hello' + } patch "/api/v1/accounts/#{account.id}/inboxes/#{inbox.id}", - headers: agent.create_new_auth_token, - params: valid_params, - as: :json + params: valid_params.merge(params), + headers: admin.create_new_auth_token - expect(response).to have_http_status(:unauthorized) + expect(response).to have_http_status(:success) + inbox.reload + expect(inbox.reload.weekly_schedule.find { |schedule| schedule['day_of_week'] == 0 }['open_hour']).to eq 9 end end end diff --git a/spec/models/concerns/out_of_offisable_spec.rb b/spec/models/concerns/out_of_offisable_spec.rb index cbdef0e81..8f09f0753 100644 --- a/spec/models/concerns/out_of_offisable_spec.rb +++ b/spec/models/concerns/out_of_offisable_spec.rb @@ -16,4 +16,10 @@ shared_examples_for 'out_of_offisable' do travel_to '01.11.2020 13:00'.to_datetime expect(obj.out_of_office?).to be true end + + it 'updates the office hours via a hash' do + obj.update_working_hours([{ 'day_of_week' => 1, 'open_hour' => 10, 'open_minutes' => 0, + 'close_hour' => 17, 'close_minutes' => 0 }]) + expect(obj.reload.weekly_schedule.find { |schedule| schedule['day_of_week'] == 1 }['open_hour']).to eq 10 + end end diff --git a/spec/services/message_templates/hook_execution_service_spec.rb b/spec/services/message_templates/hook_execution_service_spec.rb index ce64d2bb2..0abc15790 100644 --- a/spec/services/message_templates/hook_execution_service_spec.rb +++ b/spec/services/message_templates/hook_execution_service_spec.rb @@ -19,8 +19,9 @@ describe ::MessageTemplates::HookExecutionService do # described class gets called in message after commit message = create(:message, conversation: conversation) - expect(::MessageTemplates::Template::Greeting).to have_received(:new).with(conversation: message.conversation) - expect(greeting_service).to have_received(:perform) + # TODO: remove this if this hook is removed + # expect(::MessageTemplates::Template::Greeting).to have_received(:new).with(conversation: message.conversation) + # expect(greeting_service).to have_received(:perform) expect(::MessageTemplates::Template::EmailCollect).to have_received(:new).with(conversation: message.conversation) expect(email_collect_service).to have_received(:perform) end