From 28b789c47b8b680c4d22bd174f188204928072e3 Mon Sep 17 00:00:00 2001 From: Muhammad Nuzaihan Date: Fri, 13 Apr 2018 22:19:48 +0800 Subject: [PATCH] improves twitter outgoing direct message, fixes #1931 --- .../article_action/email_reply.coffee | 21 +- .../article_action/telegram.coffee | 2 +- .../article_action/twitter_reply.coffee | 18 +- .../ticket_zoom/article_new.coffee | 58 +++--- .../ticket/article/communicate_twitter.rb | 1 + test/integration/twitter_browser_test.rb | 188 ++++++++++++++---- test/integration/twitter_test.rb | 71 +++++++ 7 files changed, 279 insertions(+), 80 deletions(-) diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee index 3228ba3bc..1827adcbc 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee @@ -235,7 +235,15 @@ class EmailReply extends App.Controller articleTypes - @setArticleType: (type, ticket, ui, signaturePosition) -> + @setArticleTypePre: (type, ticket, ui, signaturePosition) -> + + # remove old signature + if type isnt 'email' + ui.$('[data-name=body] [data-signature=true]').remove() + return + + @setArticleTypePost: (type, ticket, ui, signaturePosition) -> + return if type isnt 'email' # detect current signature (use current group_id, if not set, use ticket.group_id) ticketCurrent = App.Ticket.fullLocal(ticket.id) @@ -249,7 +257,7 @@ class EmailReply extends App.Controller signature = App.Signature.find(group.signature_id) # add/replace signature - if signature && signature.body && type is 'email' + if signature && signature.body # if signature has changed, remove it signature_id = ui.$('[data-signature=true]').data('signature-id') @@ -271,15 +279,6 @@ class EmailReply extends App.Controller body.append(signature) ui.$('[data-name=body]').replaceWith(body) - # remove old signature - else - ui.$('[data-name=body] [data-signature=true]').remove() - - if type isnt 'email' - ui.$('[name=to]').val('') - ui.$('[name=cc]').val('') - ui.$('[name=subject]').val('') - @validation: (type, params, ui) -> return true if type isnt 'email' diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/telegram.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/telegram.coffee index edba450f8..90c491ebd 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/telegram.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/telegram.coffee @@ -61,7 +61,7 @@ class TelegramReply } articleTypes - @setArticleType: (type, ticket, ui) -> + @setArticleTypePost: (type, ticket, ui) -> return if type isnt 'telegram personal-message' rawHTML = ui.$('[data-name=body]').html() cleanHTML = App.Utils.htmlRemoveRichtext(rawHTML) diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/twitter_reply.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/twitter_reply.coffee index 1a39dd606..04ee715ff 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/twitter_reply.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/twitter_reply.coffee @@ -116,7 +116,7 @@ class TwitterReply else articleNew.to = article.from - if !articleNew.to + if !articleNew.to && customer && customer.accounts articleNew.to = customer.accounts['twitter'].username || customer.accounts['twitter'].uid App.Event.trigger('ui::ticket::setArticleType', { @@ -169,9 +169,23 @@ class TwitterReply textLength = ui.maxTextLength - App.Utils.textLengthWithUrl(params.body) return false if textLength < 0 + # check if recipient exists + if _.isEmpty(params.to) + new App.ControllerModal( + head: 'Text missing' + buttonCancel: 'Cancel' + buttonCancelClass: 'btn--danger' + buttonSubmit: false + message: 'Need recipient in "To".' + shown: true + small: true + container: ui.el.closest('.content') + ) + return false + true - @setArticleType: (type, ticket, ui) -> + @setArticleTypePost: (type, ticket, ui) -> return if type isnt 'twitter status' && type isnt 'twitter direct-message' rawHTML = ui.$('[data-name=body]').html() cleanHTML = App.Utils.htmlRemoveRichtext(rawHTML) diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee index bc76912be..2688e2efa 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee @@ -51,6 +51,8 @@ class App.TicketZoomArticleNew extends App.Controller @bind('ui::ticket::setArticleType', (data) => return if data.ticket.id.toString() isnt @ticket_id.toString() + @setArticleTypePre(data.type.name, data.signaturePosition) + @openTextarea(null, true) for key, value of data.article if key is 'body' @@ -58,8 +60,7 @@ class App.TicketZoomArticleNew extends App.Controller else @$("[name=\"#{key}\"]").val(value).trigger('change') - # preselect article type - @setArticleType(data.type.name, data.signaturePosition) + @setArticleTypePost(data.type.name, data.signaturePosition) # set focus into field if data.focus @@ -120,11 +121,8 @@ class App.TicketZoomArticleNew extends App.Controller App.Delay.set(a, 500, undefined, 'tags') setPossibleArticleTypes: => - actionConfig = App.Config.get('TicketZoomArticleAction') - keys = _.keys(actionConfig).sort() @articleTypes = [] - for key in keys - config = actionConfig[key] + for config in @actions() if config && config.articleTypes @articleTypes = config.articleTypes(@articleTypes, @ticket, @) @@ -166,7 +164,8 @@ class App.TicketZoomArticleNew extends App.Controller isCustomer: @permissionCheck('ticket.customer') internalSelector: @internalSelector ) - @setArticleType(@type) + @setArticleTypePre(@type) + @setArticleTypePost(@type) new App.WidgetAvatar( el: @$('.js-avatar') @@ -278,10 +277,7 @@ class App.TicketZoomArticleNew extends App.Controller params.internal = false # backend based validation - actionConfig = App.Config.get('TicketZoomArticleAction') - keys = _.keys(actionConfig).sort() - for key in keys - config = actionConfig[key] + for config in @actions() if config && config.params params = config.params(params.type, params, @) @@ -323,10 +319,7 @@ class App.TicketZoomArticleNew extends App.Controller return false # backend based validation - actionConfig = App.Config.get('TicketZoomArticleAction') - keys = _.keys(actionConfig).sort() - for key in keys - config = actionConfig[key] + for config in @actions() if config && config.validation return false if !config.validation(params.type, params, @) @@ -350,10 +343,11 @@ class App.TicketZoomArticleNew extends App.Controller selectArticleType: (event) => event.stopPropagation() articleTypeToSet = $(event.target).closest('.pop-selectable').data('value') - @setArticleType(articleTypeToSet) + @setArticleTypePre(articleTypeToSet) @hideSelectableArticleType() + @setArticleTypePost(articleTypeToSet) - $(window).off 'click.ticket-zoom-select-type' + $(window).off('click.ticket-zoom-select-type') @tokanice() hideSelectableArticleType: => @@ -374,8 +368,14 @@ class App.TicketZoomArticleNew extends App.Controller @$('[name=internal]').val('') - setArticleType: (type, signaturePosition = 'bottom') => + setArticleTypePre: (type, signaturePosition = 'bottom') => wasScrolledToBottom = @isScrolledToBottom() + + # reset old params + if type isnt @type + for key in ['to', 'cc', 'bcc', 'subject', 'in_reply_to'] + @$("[name=#{key}]").val('').trigger('change') + @type = type @$('[name=type]').val(type).trigger('change') @articleNewEdit.attr('data-type', type) @@ -395,13 +395,6 @@ class App.TicketZoomArticleNew extends App.Controller else @setArticleInternal(false) - actionConfig = App.Config.get('TicketZoomArticleAction') - keys = _.keys(actionConfig).sort() - for key in keys - localConfig = actionConfig[key] - if localConfig && localConfig.setArticleType - localConfig.setArticleType(@type, @ticket, @, signaturePosition) - # show/hide attributes/features @maxTextLength = undefined @warningTextLength = undefined @@ -438,6 +431,11 @@ class App.TicketZoomArticleNew extends App.Controller @scrollToBottom() if wasScrolledToBottom + setArticleTypePost: (type, signaturePosition = 'bottom') => + for localConfig in @actions() + if localConfig && localConfig.setArticleTypePost + localConfig.setArticleTypePost(@type, @ticket, @, signaturePosition) + isScrolledToBottom: -> return @el.scrollParent().scrollTop() + @el.scrollParent().height() is @el.scrollParent().prop('scrollHeight') @@ -615,3 +613,13 @@ class App.TicketZoomArticleNew extends App.Controller if element.find('.attachment').length == 0 element.empty() ) + + actions: -> + actionConfig = App.Config.get('TicketZoomArticleAction') + keys = _.keys(actionConfig).sort() + actions = [] + for key in keys + localConfig = actionConfig[key] + if localConfig + actions.push localConfig + actions diff --git a/app/models/observer/ticket/article/communicate_twitter.rb b/app/models/observer/ticket/article/communicate_twitter.rb index b05048c4b..311f48e04 100644 --- a/app/models/observer/ticket/article/communicate_twitter.rb +++ b/app/models/observer/ticket/article/communicate_twitter.rb @@ -23,6 +23,7 @@ class Observer::Ticket::Article::CommunicateTwitter < ActiveRecord::Observer type = Ticket::Article::Type.lookup(id: record.type_id) return if type['name'] !~ /\Atwitter/i + raise Exceptions::UnprocessableEntity, 'twitter to: parameter is missing' if record.to.blank? && type['name'] == 'twitter direct-message' Delayed::Job.enqueue(Observer::Ticket::Article::CommunicateTwitter::BackgroundJob.new(record.id)) end diff --git a/test/integration/twitter_browser_test.rb b/test/integration/twitter_browser_test.rb index c0ea4d20b..52c2a27a0 100644 --- a/test/integration/twitter_browser_test.rb +++ b/test/integration/twitter_browser_test.rb @@ -3,36 +3,7 @@ require 'browser_test_helper' class TwitterBrowserTest < TestCase def test_add_config - - # app config - if !ENV['TWITTER_BT_CONSUMER_KEY'] - raise "ERROR: Need TWITTER_BT_CONSUMER_KEY - hint TWITTER_BT_CONSUMER_KEY='1234'" - end - consumer_key = ENV['TWITTER_BT_CONSUMER_KEY'] - if !ENV['TWITTER_BT_CONSUMER_SECRET'] - raise "ERROR: Need TWITTER_BT_CONSUMER_SECRET - hint TWITTER_BT_CONSUMER_SECRET='1234'" - end - consumer_secret = ENV['TWITTER_BT_CONSUMER_SECRET'] - - if !ENV['TWITTER_BT_USER_LOGIN'] - raise "ERROR: Need TWITTER_BT_USER_LOGIN - hint TWITTER_BT_USER_LOGIN='1234'" - end - twitter_user_login = ENV['TWITTER_BT_USER_LOGIN'] - - if !ENV['TWITTER_BT_USER_PW'] - raise "ERROR: Need TWITTER_BT_USER_PW - hint TWITTER_BT_USER_PW='1234'" - end - twitter_user_pw = ENV['TWITTER_BT_USER_PW'] - - if !ENV['TWITTER_BT_CUSTOMER_TOKEN'] - raise "ERROR: Need TWITTER_BT_CUSTOMER_TOKEN - hint TWITTER_BT_CUSTOMER_TOKEN='1234'" - end - twitter_customer_token = ENV['TWITTER_BT_CUSTOMER_TOKEN'] - - if !ENV['TWITTER_BT_CUSTOMER_TOKEN_SECRET'] - raise "ERROR: Need TWITTER_BT_CUSTOMER_TOKEN_SECRET - hint TWITTER_BT_CUSTOMER_TOKEN_SECRET='1234'" - end - twitter_customer_token_secret = ENV['TWITTER_BT_CUSTOMER_TOKEN_SECRET'] + twitter_config hash = "#sweet#{hash_gen}" @@ -51,7 +22,7 @@ class TwitterBrowserTest < TestCase sleep 2 set( css: '.content.active .modal [name=consumer_key]', - value: consumer_key, + value: twitter_config[:consumer_key], ) set( css: '.content.active .modal [name=consumer_secret]', @@ -66,7 +37,7 @@ class TwitterBrowserTest < TestCase set( css: '.content.active .modal [name=consumer_secret]', - value: consumer_secret, + value: twitter_config[:consumer_secret], ) click(css: '.content.active .modal .js-submit') @@ -95,7 +66,7 @@ class TwitterBrowserTest < TestCase set( css: '.content.active .modal [name=consumer_secret]', - value: consumer_secret, + value: twitter_config[:consumer_secret], ) click(css: '.content.active .modal .js-submit') @@ -115,12 +86,12 @@ class TwitterBrowserTest < TestCase set( css: '#username_or_email', - value: twitter_user_login, + value: twitter_config[:twitter_user_login], no_click: true, #