From 09313ed326138c7c7254deb741458b5151c5f0e5 Mon Sep 17 00:00:00 2001 From: Billy Zhou Date: Thu, 12 Jul 2018 15:18:39 +0800 Subject: [PATCH] Revert "Revert "Added group by direction feature to Overviews"" This reverts commit d02edbfc3b4e6ee4f5b577ae40e695159b85d2bd. --- .../_application_controller_table.coffee | 4 + .../app/controllers/ticket_overview.coffee | 21 +++- .../javascripts/app/models/overview.coffee | 15 ++- db/migrate/20120101000010_create_ticket.rb | 1 + ...020509_add_group_direction_to_overviews.rb | 8 ++ public/assets/tests/table.js | 33 ++++++ test/browser/admin_overview_test.rb | 102 ++++++++++++++++++ test/browser_test_helper.rb | 52 +++++++++ test/controllers/overviews_controller_test.rb | 37 +++++++ 9 files changed, 269 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20180709020509_add_group_direction_to_overviews.rb diff --git a/app/assets/javascripts/app/controllers/_application_controller_table.coffee b/app/assets/javascripts/app/controllers/_application_controller_table.coffee index d32477773..6240cca77 100644 --- a/app/assets/javascripts/app/controllers/_application_controller_table.coffee +++ b/app/assets/javascripts/app/controllers/_application_controller_table.coffee @@ -113,6 +113,7 @@ class App.ControllerTable extends App.Controller radio: false renderState: undefined groupBy: undefined + groupDirection: undefined shownPerPage: 150 shownPage: 0 @@ -771,6 +772,9 @@ class App.ControllerTable extends App.Controller for key of groupObjects groupsSorted.push key groupsSorted = groupsSorted.sort() + # Reverse the sorted groups depending on the groupDirection + if @groupDirection == 'DESC' + groupsSorted.reverse() # get new order localObjects = [] diff --git a/app/assets/javascripts/app/controllers/ticket_overview.coffee b/app/assets/javascripts/app/controllers/ticket_overview.coffee index 369fea4d5..7a70edd5d 100644 --- a/app/assets/javascripts/app/controllers/ticket_overview.coffee +++ b/app/assets/javascripts/app/controllers/ticket_overview.coffee @@ -969,6 +969,7 @@ class Table extends App.Controller overviewAttributes: @overview.view.s objects: ticketListShow groupBy: @overview.group_by + groupDirection: @overview.group_direction orderBy: @overview.order.by orderDirection: @overview.order.direction ) @@ -1134,6 +1135,7 @@ class Table extends App.Controller objects: ticketListShow checkbox: checkbox groupBy: @overview.group_by + groupDirection: @overview.group_direction orderBy: @overview.order.by orderDirection: @overview.order.direction class: 'table--light' @@ -1529,7 +1531,7 @@ class App.OverviewSettings extends App.ControllerModal }, { name: 'order::direction' - display: 'Direction' + display: 'Order by Direction' tag: 'select' default: @overview.order.direction null: false @@ -1547,7 +1549,18 @@ class App.OverviewSettings extends App.ControllerModal nulloption: true translate: true options: App.Overview.groupByAttributes() - }) + }, + { + name: 'group_direction' + display: 'Group by Direction' + tag: 'select' + default: @overview.group_direction + null: false + translate: true + options: + ASC: 'up' + DESC: 'down' + },) controller = new App.ControllerForm( model: { configure_attributes: @configure_attributes_article } @@ -1572,6 +1585,10 @@ class App.OverviewSettings extends App.ControllerModal @overview.order.direction = params.order.direction @reload_needed = true + if @overview.group_direction isnt params.group_direction + @overview.group_direction = params.group_direction + @reload_needed = true + for key, value of params.view @overview.view[key] = value diff --git a/app/assets/javascripts/app/models/overview.coffee b/app/assets/javascripts/app/models/overview.coffee index f5126170a..4834b94bf 100644 --- a/app/assets/javascripts/app/models/overview.coffee +++ b/app/assets/javascripts/app/models/overview.coffee @@ -1,5 +1,5 @@ class App.Overview extends App.Model - @configure 'Overview', 'name', 'prio', 'condition', 'order', 'group_by', 'view', 'user_ids', 'organization_shared', 'role_ids', 'active' + @configure 'Overview', 'name', 'prio', 'condition', 'order', 'group_by', 'group_direction', 'view', 'user_ids', 'organization_shared', 'role_ids', 'active' @extend Spine.Model.Ajax @url: @apiPath + '/overviews' @configure_attributes = [ @@ -29,7 +29,7 @@ class App.Overview extends App.Model }, { name: 'order::direction' - display: 'Direction' + display: 'Order by Direction' tag: 'select' default: 'down' null: false @@ -53,6 +53,17 @@ class App.Overview extends App.Model group: 'Group' owner: 'Owner' }, + { + name: 'group_direction' + display: 'Group by Direction' + tag: 'select' + default: 'down' + null: false + translate: true + options: + ASC: 'up' + DESC: 'down' + }, { name: 'active', display: 'Active', tag: 'active', default: true }, { name: 'created_by_id', display: 'Created by', relation: 'User', readonly: 1 }, { name: 'created_at', display: 'Created', tag: 'datetime', readonly: 1 }, diff --git a/db/migrate/20120101000010_create_ticket.rb b/db/migrate/20120101000010_create_ticket.rb index 9c16bf3d7..07ee6371d 100644 --- a/db/migrate/20120101000010_create_ticket.rb +++ b/db/migrate/20120101000010_create_ticket.rb @@ -237,6 +237,7 @@ class CreateTicket < ActiveRecord::Migration[4.2] t.column :condition, :text, limit: 500.kilobytes + 1, null: false t.column :order, :string, limit: 2500, null: false t.column :group_by, :string, limit: 250, null: true + t.column :group_direction, :string, limit: 250, null: true t.column :organization_shared, :boolean, null: false, default: false t.column :out_of_office, :boolean, null: false, default: false t.column :view, :string, limit: 1000, null: false diff --git a/db/migrate/20180709020509_add_group_direction_to_overviews.rb b/db/migrate/20180709020509_add_group_direction_to_overviews.rb new file mode 100644 index 000000000..fba072601 --- /dev/null +++ b/db/migrate/20180709020509_add_group_direction_to_overviews.rb @@ -0,0 +1,8 @@ +class AddGroupDirectionToOverviews < ActiveRecord::Migration[5.1] + def change + # return if it's a new setup + return if !Setting.find_by(name: 'system_init_done') + + add_column :overviews, :group_direction, :string, limit: 250, null: true + end +end diff --git a/public/assets/tests/table.js b/public/assets/tests/table.js index 8754aaa8d..4e3164e17 100644 --- a/public/assets/tests/table.js +++ b/public/assets/tests/table.js @@ -355,6 +355,39 @@ test('table test', function() { el.find('tbody > tr:nth-child(5) > td:nth-child(1) label').click() equal(el.find('tbody > tr:nth-child(5) > td:nth-child(1) input').prop('checked'), true, 'check row 5') equal(el.find('tbody > tr:nth-child(5) > td:nth-child(1) input').val(), '1', 'check row 5') + + + $('#table').append('

table Group By Direction DESC

') + el = $('#table6') + var clickCheckbox = function (id, checked, e) { + console.log('clickCheckbox', id, checked, e.target) + }; + new App.ControllerTable({ + el: el, + overview: ['number', 'title', 'owner', 'customer', 'priority', 'group', 'state', 'created_at'], + model: App.Ticket, + objects: App.Ticket.search({sortBy:'created_at', order: 'DESC'}), + groupBy: 'priority', + groupDirection: 'DESC', + }) + equal(el.find('tbody > tr:nth-child(1) > td:nth-child(1)').text().trim(), '2 normal', 'check row 1') + equal(el.find('tbody > tr:nth-child(3) > td:nth-child(1)').text().trim(), '1 niedrig', 'check row 3') + + $('#table').append('

table Group By Direction ASC

') + el = $('#table7') + var clickCheckbox = function (id, checked, e) { + console.log('clickCheckbox', id, checked, e.target) + }; + new App.ControllerTable({ + el: el, + overview: ['number', 'title', 'owner', 'customer', 'priority', 'group', 'state', 'created_at'], + model: App.Ticket, + objects: App.Ticket.search({sortBy:'created_at', order: 'DESC'}), + groupBy: 'priority', + groupDirection: 'ASC', + }) + equal(el.find('tbody > tr:nth-child(1) > td:nth-child(1)').text().trim(), '1 niedrig', 'check row 1') + equal(el.find('tbody > tr:nth-child(4) > td:nth-child(1)').text().trim(), '2 normal', 'check row 4') }); test('table test 2.1', function() { diff --git a/test/browser/admin_overview_test.rb b/test/browser/admin_overview_test.rb index 87b6791f5..e9cfe0b9c 100644 --- a/test/browser/admin_overview_test.rb +++ b/test/browser/admin_overview_test.rb @@ -38,4 +38,106 @@ class AdminOverviewTest < TestCase ) end + def test_overview_group_by_direction + name = "overview_#{rand(99_999_999)}" + ticket_titles = (1..3).map { |i| "Priority #{i} ticket" } + + @browser = instance = browser_instance + login( + username: 'master@example.com', + password: 'test', + url: browser_url, + ) + tasks_close_all() + + ticket_create( + data: { + customer: 'nico', + group: 'Users', + title: 'Priority 1 ticket', + body: 'some body 123äöü', + priority: '1 low', + }, + ) + + ticket_create( + data: { + customer: 'nico', + group: 'Users', + title: 'Priority 2 ticket', + body: 'some body 123äöü', + priority: '2 normal', + }, + ) + + ticket_create( + data: { + customer: 'nico', + group: 'Users', + title: 'Priority 3 ticket', + body: 'some body 123äöü', + priority: '3 high', + }, + ) + + # Add new overview to sort groups from high to low + overview_create( + data: { + name: name, + roles: ['Agent'], + selector: { + 'State' => 'open', + }, + 'order::direction' => 'down', + group_by: 'Priority', + group_direction: 'down', + } + ) + + click( + browser: instance, + css: 'a[href="#ticket/view"]', + mute_log: true, + ) + click( + browser: instance, + css: "div.overview-header a[href='#ticket/view/#{name}']", + mute_log: true, + ) + + # Sort the tickets according to their onscreen Y location + tickets_low_to_high = ticket_titles.map do |title| + [title, + get_location( css: "td[title='#{title}']").y] + end + tickets_low_to_high = tickets_low_to_high.sort_by { |x| -x[1] }.map { |x| x[0] } + assert_equal(ticket_titles, tickets_low_to_high) + + # Update overview to sort groups from low to high + overview_update( + data: { + name: name, + group_direction: 'up', + } + ) + + click( + browser: instance, + css: 'a[href="#ticket/view"]', + mute_log: true, + ) + click( + browser: instance, + css: "div.overview-header a[href='#ticket/view/#{name}']", + mute_log: true, + ) + + # Sort the tickets according to their onscreen Y location + tickets_high_to_low = ticket_titles.map do |title| + [title, + get_location( css: "td[title='#{title}']").y] + end + tickets_high_to_low = tickets_high_to_low.sort_by { |x| x[1] }.map { |x| x[0] } + assert_equal(ticket_titles, tickets_high_to_low) + end end diff --git a/test/browser_test_helper.rb b/test/browser_test_helper.rb index 75b22fafe..12705ddc6 100644 --- a/test/browser_test_helper.rb +++ b/test/browser_test_helper.rb @@ -1018,6 +1018,40 @@ class TestCase < Test::Unit::TestCase =begin +Get the on-screen pixel coordinates of a given DOM element. Can be used to compare +the relative location of table rows before and after sort, for example. + +Returns a Selenium::WebDriver::Point object. Use result.x and result.y to access +its X and Y coordinates respectively. + + get_location( + browser: browser1, + css: '.some_class', + ) + +=end + + def get_location(params) + switch_window_focus(params) + log('exists', params) + + instance = params[:browser] || @browser + if params[:css] + query = { css: params[:css] } + end + if params[:xpath] + query = { xpath: params[:xpath] } + end + if !instance.find_elements(query)[0] + screenshot(browser: instance, comment: 'exists_failed') + raise "#{query} dosn't exist, but should" + end + + instance.find_elements(query)[0].location + end + +=begin + set type of task (closeTab, closeNextInOverview, stayOnTab) task_type( @@ -1735,6 +1769,15 @@ wait untill text in selector disabppears ) end + if data[:group_direction] + select( + browser: instance, + css: '.modal select[name="group_direction"]', + value: data[:group_direction], + mute_log: true, + ) + end + instance.find_elements(css: '.modal button.js-submit')[0].click modal_disappear(browser: instance) 11.times do @@ -1839,6 +1882,15 @@ wait untill text in selector disabppears ) end + if data[:group_direction] + select( + browser: instance, + css: '.modal select[name="group_direction"]', + value: data[:group_direction], + mute_log: true, + ) + end + instance.find_elements(css: '.modal button.js-submit')[0].click modal_disappear(browser: instance) 11.times do diff --git a/test/controllers/overviews_controller_test.rb b/test/controllers/overviews_controller_test.rb index d2bcb36c0..639a04ec8 100644 --- a/test/controllers/overviews_controller_test.rb +++ b/test/controllers/overviews_controller_test.rb @@ -181,4 +181,41 @@ class OverviewsControllerTest < ActionDispatch::IntegrationTest assert_equal(1, overview2.prio) end + test 'create an overview with group_by direction' do + credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-admin', 'adminpw') + + params = { + name: 'Overview2', + link: 'my_overview', + roles: Role.where(name: 'Agent').pluck(:name), + condition: { + 'ticket.state_id' => { + operator: 'is', + value: [1, 2, 3], + }, + }, + order: { + by: 'created_at', + direction: 'DESC', + }, + group_by: 'priority', + group_direction: 'ASC', + view: { + d: %w[title customer state created_at], + s: %w[number title customer state created_at], + m: %w[number title customer state created_at], + view_mode_default: 's', + }, + } + + post '/api/v1/overviews', params: params.to_json, headers: @headers.merge('Authorization' => credentials) + assert_response(201) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert_equal('Overview2', result['name']) + assert_equal('my_overview', result['link']) + assert_equal('priority', result['group_by']) + assert_equal('ASC', result['group_direction']) + end + end