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

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
+    @resolved_contacts = Current.account.contacts.resolved_contacts
-                                .where.not(email: [nil, ''])
+
-                                .or(Current.account.contacts.where.not(phone_number: [nil, '']))
-                                .or(Current.account.contacts.where.not(identifier: [nil, '']))
     @resolved_contacts = @resolved_contacts.tagged_with(params[:labels], any: true) if params[:labels].present?
     @resolved_contacts
   end

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 }) => (
             <woot-button
@@ -150,7 +150,7 @@ export default {
           key: 'email',
           title: this.$t('CONTACTS_PAGE.LIST.TABLE_HEADER.EMAIL_ADDRESS'),
           align: 'left',
-          sortBy: this.sortConfig.email || '',
+          sortBy: this.sortConfig.email || undefined,
           width: 240,
           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 (

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

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",