From 358171062e72b613296301c3c2185cb59b3a3098 Mon Sep 17 00:00:00 2001 From: Tejaswini Chile Date: Wed, 10 Nov 2021 14:11:00 +0530 Subject: [PATCH] Feat: Add Null values at the last while sorting (#3292) * Add Null values at the last while sorting * Add contacts at last with special character in it * Optimize SQL order and direction --- .../api/v1/accounts/contacts_controller.rb | 14 ++--- .../contacts/components/ContactsTable.vue | 9 ++- app/models/contact.rb | 59 +++++++++++++++++++ .../v1/accounts/contacts_controller_spec.rb | 49 ++++++++++++++- 4 files changed, 120 insertions(+), 11 deletions(-) diff --git a/app/controllers/api/v1/accounts/contacts_controller.rb b/app/controllers/api/v1/accounts/contacts_controller.rb index 090827d8f..b795a8417 100644 --- a/app/controllers/api/v1/accounts/contacts_controller.rb +++ b/app/controllers/api/v1/accounts/contacts_controller.rb @@ -1,10 +1,12 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController include Sift - sort_on :email, type: :string - sort_on :name, type: :string + sort_on :name, internal_name: :order_on_name, type: :scope, scope_params: [:direction] sort_on :phone_number, type: :string - sort_on :last_activity_at, type: :datetime + sort_on :last_activity_at, internal_name: :order_on_last_activity_at, type: :scope, scope_params: [:direction] + sort_on :company, internal_name: :order_on_company_name, type: :scope, scope_params: [:direction] + sort_on :city, internal_name: :order_on_city, type: :scope, scope_params: [:direction] + sort_on :country, internal_name: :order_on_country_name, type: :scope, scope_params: [:direction] RESULTS_PER_PAGE = 15 @@ -98,10 +100,8 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController def resolved_contacts return @resolved_contacts if @resolved_contacts - @resolved_contacts = Current.account.contacts - .where.not(email: [nil, '']) - .or(Current.account.contacts.where.not(phone_number: [nil, ''])) - .or(Current.account.contacts.where.not(identifier: [nil, ''])) + @resolved_contacts = Current.account.contacts.resolved_contacts + @resolved_contacts = @resolved_contacts.tagged_with(params[:labels], any: true) if params[:labels].present? @resolved_contacts end diff --git a/app/javascript/dashboard/routes/dashboard/contacts/components/ContactsTable.vue b/app/javascript/dashboard/routes/dashboard/contacts/components/ContactsTable.vue index 4612c99be..f3d27032a 100644 --- a/app/javascript/dashboard/routes/dashboard/contacts/components/ContactsTable.vue +++ b/app/javascript/dashboard/routes/dashboard/contacts/components/ContactsTable.vue @@ -114,7 +114,7 @@ export default { title: this.$t('CONTACTS_PAGE.LIST.TABLE_HEADER.NAME'), fixed: 'left', align: 'left', - sortBy: this.sortConfig.name || '', + sortBy: this.sortConfig.name || undefined, width: 300, renderBodyCell: ({ row }) => ( { if (row.email) @@ -171,19 +171,21 @@ export default { { field: 'phone_number', key: 'phone_number', - sortBy: this.sortConfig.phone_number || '', + sortBy: this.sortConfig.phone_number || undefined, title: this.$t('CONTACTS_PAGE.LIST.TABLE_HEADER.PHONE_NUMBER'), align: 'left', }, { field: 'company', key: 'company', + sortBy: this.sortConfig.company_name || undefined, title: this.$t('CONTACTS_PAGE.LIST.TABLE_HEADER.COMPANY'), align: 'left', }, { field: 'city', key: 'city', + sortBy: this.sortConfig.city || undefined, title: this.$t('CONTACTS_PAGE.LIST.TABLE_HEADER.CITY'), align: 'left', }, @@ -192,6 +194,7 @@ export default { key: 'country', title: this.$t('CONTACTS_PAGE.LIST.TABLE_HEADER.COUNTRY'), align: 'left', + sortBy: this.sortConfig.country || undefined, renderBodyCell: ({ row }) => { if (row.country) { return ( diff --git a/app/models/contact.rb b/app/models/contact.rb index 267dfab72..ec0b27d70 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -50,6 +50,59 @@ class Contact < ApplicationRecord after_update_commit :dispatch_update_event after_destroy_commit :dispatch_destroy_event + scope :order_on_last_activity_at, lambda { |direction| + order( + Arel::Nodes::SqlLiteral.new( + sanitize_sql_for_order("\"contacts\".\"last_activity_at\" #{direction} + NULLS LAST") + ) + ) + } + scope :order_on_company_name, lambda { |direction| + order( + Arel::Nodes::SqlLiteral.new( + sanitize_sql_for_order( + "\"contacts\".\"additional_attributes\"->>'company_name' #{direction} + NULLS LAST" + ) + ) + ) + } + scope :order_on_city, lambda { |direction| + order( + Arel::Nodes::SqlLiteral.new( + sanitize_sql_for_order( + "\"contacts\".\"additional_attributes\"->>'city' #{direction} + NULLS LAST" + ) + ) + ) + } + scope :order_on_country_name, lambda { |direction| + order( + Arel::Nodes::SqlLiteral.new( + sanitize_sql_for_order( + "\"contacts\".\"additional_attributes\"->>'country' #{direction} + NULLS LAST" + ) + ) + ) + } + + scope :order_on_name, lambda { |direction| + order( + Arel::Nodes::SqlLiteral.new( + sanitize_sql_for_order( + "CASE + WHEN \"contacts\".\"name\" ~~* '^+\d*' THEN 'z' + WHEN \"contacts\".\"name\" ~~* '^\b*' THEN 'z' + ELSE LOWER(\"contacts\".\"name\") + END #{direction}" + ) + ) + ) + } + def get_source_id(inbox_id) contact_inboxes.find_by!(inbox_id: inbox_id).source_id end @@ -79,6 +132,12 @@ class Contact < ApplicationRecord } end + def self.resolved_contacts + where.not(email: [nil, '']).or( + Current.account.contacts.where.not(phone_number: [nil, '']) + ).or(Current.account.contacts.where.not(identifier: [nil, ''])) + end + private def ip_lookup diff --git a/spec/controllers/api/v1/accounts/contacts_controller_spec.rb b/spec/controllers/api/v1/accounts/contacts_controller_spec.rb index 14f83d35b..fcff1ac2a 100644 --- a/spec/controllers/api/v1/accounts/contacts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/contacts_controller_spec.rb @@ -14,7 +14,19 @@ RSpec.describe 'Contacts API', type: :request do context 'when it is an authenticated user' do let(:admin) { create(:user, account: account, role: :administrator) } - let!(:contact) { create(:contact, :with_email, account: account) } + let!(:contact) { create(:contact, :with_email, account: account, additional_attributes: { company_name: 'Company 1', country_code: 'IN' }) } + let!(:contact_1) do + create(:contact, :with_email, account: account, additional_attributes: { company_name: 'Test Company 1', country_code: 'CA' }) + end + let(:contact_2) do + create(:contact, :with_email, account: account, additional_attributes: { company_name: 'Marvel Company', country_code: 'AL' }) + end + let(:contact_3) do + create(:contact, :with_email, account: account, additional_attributes: { company_name: nil, country_code: nil }) + end + let!(:contact_4) do + create(:contact, :with_email, account: account, additional_attributes: { company_name: nil, country_code: nil }) + end let!(:contact_inbox) { create(:contact_inbox, contact: contact) } it 'returns all resolved contacts along with contact inboxes' do @@ -40,6 +52,41 @@ RSpec.describe 'Contacts API', type: :request do expect(response_body['payload'].first['contact_inboxes'].blank?).to eq(true) end + it 'returns all contacts with company name desc order' do + get "/api/v1/accounts/#{account.id}/contacts?include_contact_inboxes=false&sort=-company", + headers: admin.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + response_body = JSON.parse(response.body) + expect(response_body['payload'].last['id']).to eq(contact_4.id) + expect(response_body['payload'].last['email']).to eq(contact_4.email) + end + + it 'returns all contacts with company name asc order with null values at last' do + get "/api/v1/accounts/#{account.id}/contacts?include_contact_inboxes=false&sort=-company", + headers: admin.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + response_body = JSON.parse(response.body) + expect(response_body['payload'].first['email']).to eq(contact_1.email) + expect(response_body['payload'].first['id']).to eq(contact_1.id) + expect(response_body['payload'].last['email']).to eq(contact_4.email) + end + + it 'returns all contacts with country name desc order with null values at last' do + get "/api/v1/accounts/#{account.id}/contacts?include_contact_inboxes=false&sort=country", + headers: admin.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + response_body = JSON.parse(response.body) + expect(response_body['payload'].first['email']).to eq(contact.email) + expect(response_body['payload'].first['id']).to eq(contact.id) + expect(response_body['payload'].last['email']).to eq(contact_4.email) + end + it 'returns includes conversations count and last seen at' do create(:conversation, contact: contact, account: account, inbox: contact_inbox.inbox, contact_last_seen_at: Time.now.utc) get "/api/v1/accounts/#{account.id}/contacts",