feat: Add support for minutes in auto resolve feature (#11269)
### Summary - Converts conversation auto-resolution duration from days to minutes for more granular control - Updates validation to allow values from 10 minutes (minimum) to 999 days (maximum) - Implements smart messaging to show appropriate time units in activity messages ### Changes - Created migration to convert existing durations from days to minutes (x1440) - Updated conversation resolver to use minutes instead of days - Added dynamic translation key selection based on duration value - Updated related specs and documentation - Added support for displaying durations in days, hours, or minutes based on value ### Test plan - Verify account validation accepts new minute-based ranges - Confirm existing account settings are correctly migrated - Test auto-resolution works properly with minute values - Ensure proper time unit display in activity messages --------- Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com>
This commit is contained in:
@@ -119,7 +119,7 @@ RSpec.describe 'Accounts API', type: :request do
|
||||
|
||||
context 'when it is an authenticated user' do
|
||||
it 'shows an account' do
|
||||
account.update(auto_resolve_duration: 30)
|
||||
account.update(name: 'new name')
|
||||
|
||||
get "/api/v1/accounts/#{account.id}",
|
||||
headers: admin.create_new_auth_token,
|
||||
@@ -130,7 +130,6 @@ RSpec.describe 'Accounts API', type: :request do
|
||||
expect(response.body).to include(account.locale)
|
||||
expect(response.body).to include(account.domain)
|
||||
expect(response.body).to include(account.support_email)
|
||||
expect(response.body).to include(account.auto_resolve_duration.to_s)
|
||||
expect(response.body).to include(account.locale)
|
||||
end
|
||||
end
|
||||
@@ -189,7 +188,8 @@ RSpec.describe 'Accounts API', type: :request do
|
||||
locale: 'en',
|
||||
domain: 'example.com',
|
||||
support_email: 'care@example.com',
|
||||
auto_resolve_duration: 40,
|
||||
auto_resolve_after: 40,
|
||||
auto_resolve_message: 'Auto resolved',
|
||||
timezone: 'Asia/Kolkata',
|
||||
industry: 'Technology',
|
||||
company_size: '1-10'
|
||||
@@ -206,7 +206,10 @@ RSpec.describe 'Accounts API', type: :request do
|
||||
expect(account.reload.locale).to eq(params[:locale])
|
||||
expect(account.reload.domain).to eq(params[:domain])
|
||||
expect(account.reload.support_email).to eq(params[:support_email])
|
||||
expect(account.reload.auto_resolve_duration).to eq(params[:auto_resolve_duration])
|
||||
|
||||
%w[auto_resolve_after auto_resolve_message].each do |attribute|
|
||||
expect(account.reload.settings[attribute]).to eq(params[attribute.to_sym])
|
||||
end
|
||||
|
||||
%w[timezone industry company_size].each do |attribute|
|
||||
expect(account.reload.custom_attributes[attribute]).to eq(params[attribute.to_sym])
|
||||
|
||||
@@ -11,7 +11,7 @@ RSpec.describe Account::ConversationsResolutionSchedulerJob do
|
||||
end
|
||||
|
||||
it 'enqueues Conversations::ResolutionJob' do
|
||||
account.update(auto_resolve_duration: 10)
|
||||
account.update(auto_resolve_after: 10 * 60 * 24)
|
||||
expect(Conversations::ResolutionJob).to receive(:perform_later).with(account: account).once
|
||||
described_class.perform_now
|
||||
end
|
||||
|
||||
@@ -18,16 +18,17 @@ RSpec.describe Conversations::ResolutionJob do
|
||||
end
|
||||
|
||||
it 'resolves the issue if time of inactivity is more than the auto resolve duration' do
|
||||
account.update(auto_resolve_duration: 10)
|
||||
conversation.update(last_activity_at: 13.days.ago)
|
||||
account.update(auto_resolve_after: 14_400) # 10 days in minutes
|
||||
conversation.update(last_activity_at: 13.days.ago, waiting_since: nil)
|
||||
described_class.perform_now(account: account)
|
||||
expect(conversation.reload.status).to eq('resolved')
|
||||
end
|
||||
|
||||
it 'resolved only a limited number of conversations in a single execution' do
|
||||
stub_const('Limits::BULK_ACTIONS_LIMIT', 2)
|
||||
account.update(auto_resolve_duration: 10)
|
||||
create_list(:conversation, 3, account: account, last_activity_at: 13.days.ago)
|
||||
account.update(auto_resolve_after: 14_400) # 10 days in minutes
|
||||
conversations = create_list(:conversation, 3, account: account, last_activity_at: 13.days.ago)
|
||||
conversations.each { |conversation| conversation.update(waiting_since: nil) }
|
||||
described_class.perform_now(account: account)
|
||||
expect(account.conversations.resolved.count).to eq(Limits::BULK_ACTIONS_LIMIT)
|
||||
end
|
||||
|
||||
@@ -3,9 +3,6 @@
|
||||
require 'rails_helper'
|
||||
|
||||
RSpec.describe Account do
|
||||
it { is_expected.to validate_numericality_of(:auto_resolve_duration).is_greater_than_or_equal_to(1) }
|
||||
it { is_expected.to validate_numericality_of(:auto_resolve_duration).is_less_than_or_equal_to(999) }
|
||||
|
||||
it { is_expected.to have_many(:users).through(:account_users) }
|
||||
it { is_expected.to have_many(:account_users) }
|
||||
it { is_expected.to have_many(:inboxes).dependent(:destroy_async) }
|
||||
@@ -134,4 +131,86 @@ RSpec.describe Account do
|
||||
expect(account.locale_english_name).to eq('portuguese')
|
||||
end
|
||||
end
|
||||
|
||||
describe 'settings' do
|
||||
let(:account) { create(:account) }
|
||||
|
||||
context 'when auto_resolve_after' do
|
||||
it 'validates minimum value' do
|
||||
account.settings = { auto_resolve_after: 4 }
|
||||
expect(account).to be_invalid
|
||||
expect(account.errors.messages).to eq({ auto_resolve_after: ['must be greater than or equal to 10'] })
|
||||
end
|
||||
|
||||
it 'validates maximum value' do
|
||||
account.settings = { auto_resolve_after: 1_439_857 }
|
||||
expect(account).to be_invalid
|
||||
expect(account.errors.messages).to eq({ auto_resolve_after: ['must be less than or equal to 1439856'] })
|
||||
end
|
||||
|
||||
it 'allows valid values' do
|
||||
account.settings = { auto_resolve_after: 15 }
|
||||
expect(account).to be_valid
|
||||
|
||||
account.settings = { auto_resolve_after: 1_439_856 }
|
||||
expect(account).to be_valid
|
||||
end
|
||||
|
||||
it 'allows null values' do
|
||||
account.settings = { auto_resolve_after: nil }
|
||||
expect(account).to be_valid
|
||||
end
|
||||
end
|
||||
|
||||
context 'when auto_resolve_message' do
|
||||
it 'allows string values' do
|
||||
account.settings = { auto_resolve_message: 'This conversation has been resolved automatically.' }
|
||||
expect(account).to be_valid
|
||||
end
|
||||
|
||||
it 'allows empty string' do
|
||||
account.settings = { auto_resolve_message: '' }
|
||||
expect(account).to be_valid
|
||||
end
|
||||
|
||||
it 'allows nil values' do
|
||||
account.settings = { auto_resolve_message: nil }
|
||||
expect(account).to be_valid
|
||||
end
|
||||
end
|
||||
|
||||
context 'when using store_accessor' do
|
||||
it 'correctly gets and sets auto_resolve_after' do
|
||||
account.auto_resolve_after = 30
|
||||
expect(account.auto_resolve_after).to eq(30)
|
||||
expect(account.settings['auto_resolve_after']).to eq(30)
|
||||
end
|
||||
|
||||
it 'correctly gets and sets auto_resolve_message' do
|
||||
message = 'This conversation was automatically resolved'
|
||||
account.auto_resolve_message = message
|
||||
expect(account.auto_resolve_message).to eq(message)
|
||||
expect(account.settings['auto_resolve_message']).to eq(message)
|
||||
end
|
||||
|
||||
it 'handles nil values correctly' do
|
||||
account.auto_resolve_after = nil
|
||||
account.auto_resolve_message = nil
|
||||
expect(account.auto_resolve_after).to be_nil
|
||||
expect(account.auto_resolve_message).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
context 'when using with_auto_resolve scope' do
|
||||
it 'finds accounts with auto_resolve_after set' do
|
||||
account.update(auto_resolve_after: 40 * 24 * 60)
|
||||
expect(described_class.with_auto_resolve).to include(account)
|
||||
end
|
||||
|
||||
it 'does not find accounts without auto_resolve_after' do
|
||||
account.update(auto_resolve_after: nil)
|
||||
expect(described_class.with_auto_resolve).not_to include(account)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -5,9 +5,10 @@ RSpec.describe JsonSchemaValidator, type: :validator do
|
||||
'type' => 'object',
|
||||
'properties' => {
|
||||
'name' => { 'type' => 'string' },
|
||||
'age' => { 'type' => 'integer' },
|
||||
'age' => { 'type' => 'integer', 'minimum' => 18, 'maximum' => 100 },
|
||||
'is_active' => { 'type' => 'boolean' },
|
||||
'tags' => { 'type' => 'array' },
|
||||
'score' => { 'type' => 'number', 'minimum' => 0, 'maximum' => 10 },
|
||||
'address' => {
|
||||
'type' => 'object',
|
||||
'properties' => {
|
||||
@@ -109,4 +110,44 @@ RSpec.describe JsonSchemaValidator, type: :validator do
|
||||
:is_active => ['must be of type boolean'], :tags => ['must be of type array'] })
|
||||
end
|
||||
end
|
||||
|
||||
context 'with value below minimum' do
|
||||
let(:invalid_data) do
|
||||
{
|
||||
'name' => 'John Doe',
|
||||
'age' => 15,
|
||||
'score' => -1,
|
||||
'is_active' => true
|
||||
}
|
||||
end
|
||||
|
||||
it 'fails validation' do
|
||||
model = TestModelForJSONValidation.new(invalid_data)
|
||||
expect(model.valid?).to be false
|
||||
expect(model.errors.messages).to eq({
|
||||
:age => ['must be greater than or equal to 18'],
|
||||
:score => ['must be greater than or equal to 0']
|
||||
})
|
||||
end
|
||||
end
|
||||
|
||||
context 'with value above maximum' do
|
||||
let(:invalid_data) do
|
||||
{
|
||||
'name' => 'John Doe',
|
||||
'age' => 120,
|
||||
'score' => 11,
|
||||
'is_active' => true
|
||||
}
|
||||
end
|
||||
|
||||
it 'fails validation' do
|
||||
model = TestModelForJSONValidation.new(invalid_data)
|
||||
expect(model.valid?).to be false
|
||||
expect(model.errors.messages).to eq({
|
||||
:age => ['must be less than or equal to 100'],
|
||||
:score => ['must be less than or equal to 10']
|
||||
})
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -213,11 +213,18 @@ RSpec.describe Conversation do
|
||||
end
|
||||
|
||||
it 'adds a message for system auto resolution if marked resolved by system' do
|
||||
account.update(auto_resolve_duration: 40)
|
||||
account.update(auto_resolve_after: 40 * 24 * 60)
|
||||
conversation2 = create(:conversation, status: 'open', account: account, assignee: old_assignee)
|
||||
Current.user = nil
|
||||
|
||||
system_resolved_message = "Conversation was marked resolved by system due to #{account.auto_resolve_duration} days of inactivity"
|
||||
message_data = if account.auto_resolve_after >= 1440 && account.auto_resolve_after % 1440 == 0
|
||||
{ key: 'auto_resolved_days', count: account.auto_resolve_after / 1440 }
|
||||
elsif account.auto_resolve_after >= 60 && account.auto_resolve_after % 60 == 0
|
||||
{ key: 'auto_resolved_hours', count: account.auto_resolve_after / 60 }
|
||||
else
|
||||
{ key: 'auto_resolved_minutes', count: account.auto_resolve_after }
|
||||
end
|
||||
system_resolved_message = "Conversation was marked resolved by system due to #{message_data[:count]} days of inactivity"
|
||||
expect { conversation2.update(status: :resolved) }
|
||||
.to have_enqueued_job(Conversations::ActivityMessageJob)
|
||||
.with(conversation2, { account_id: conversation2.account_id, inbox_id: conversation2.inbox_id, message_type: :activity,
|
||||
|
||||
Reference in New Issue
Block a user