From 7933bfc546c65f166475b626a28ec0e6bc96116d Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Thu, 29 Aug 2019 07:09:50 -0700 Subject: [PATCH 1/4] Track from_email and to_email Clean up logic with email lookup Show sender email --- .../sync/zendesk/ticket_comments_to_notes.rb | 33 +++++++++++++------ app/models/publisher_note.rb | 2 ++ app/views/admin/publishers/_note.html.slim | 2 +- ...845_add_sender_email_to_publisher_notes.rb | 6 ++++ db/schema.rb | 4 ++- 5 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 db/migrate/20190903235845_add_sender_email_to_publisher_notes.rb diff --git a/app/jobs/sync/zendesk/ticket_comments_to_notes.rb b/app/jobs/sync/zendesk/ticket_comments_to_notes.rb index 069bd30715..4c636970a3 100644 --- a/app/jobs/sync/zendesk/ticket_comments_to_notes.rb +++ b/app/jobs/sync/zendesk/ticket_comments_to_notes.rb @@ -30,27 +30,40 @@ def perform(zendesk_ticket_id, page_number = 0) publisher_notes = [] publisher = nil - admin_id = Publisher.find_by(email: Rails.application.secrets[:zendesk_admin_email]).id + admin = Publisher.find_by(email: Rails.application.secrets[:zendesk_admin_email]) zendesk_comments = client.ticket.find(id: zendesk_ticket_id).comments for index in (0...zendesk_comments.count) + from_email = nil + to_email = nil + + # The first email should be the publisher's email. Would be surprising otherwise comment = zendesk_comments[index] - publisher_email = comment&.via&.source&.from&.address + from_email = comment&.via&.source&.from&.address + to_email = comment&.via&.source&.to&.address - if publisher_email.present? && publisher.nil? - publisher = Publisher.find_by(email: publisher_email) + if (from_email.present? || to_email.present?) && publisher.nil? + publisher = Publisher.find_by(email: from_email) + publisher = Publisher.find_by(email: to_email) if publisher.nil? end - publisher_notes << PublisherNote.new(note: comment.plain_body, zendesk_ticket_id: zendesk_ticket_id, zendesk_comment_id: comment.id, created_at: comment.created_at) + + publisher_note = PublisherNote.find_or_initialize_by(zendesk_ticket_id: zendesk_ticket_id, zendesk_comment_id: comment.id) + publisher_note.created_at = comment.created_at + publisher_note.created_by_id = admin.id + publisher_note.note = comment.plain_body + publisher_note.zendesk_from_email = from_email + publisher_note.zendesk_to_email = to_email + publisher_notes << publisher_note end - return if publisher.nil? + # Don't lock this in a transaction as we might need to parse over to update the ticket - publisher_notes.each do |publisher_note| + publisher_notes.each do |pn| begin - publisher_note.publisher_id = publisher.id - publisher_note.created_by_id = admin_id - publisher_note.save + pn.publisher_id = publisher.id + pn.save rescue ActiveRecord::RecordNotUnique end end + Rails.logger.info "Done with zendesk ticket #{zendesk_ticket_id}" end end diff --git a/app/models/publisher_note.rb b/app/models/publisher_note.rb index 4100383e67..49b6b95559 100644 --- a/app/models/publisher_note.rb +++ b/app/models/publisher_note.rb @@ -8,4 +8,6 @@ class PublisherNote < ApplicationRecord has_many :comments, class_name: "PublisherNote", foreign_key: "thread_id" validates :note, presence: true, allow_blank: false + + UNKNOWN_PUBLISHER_ID = "00000000-0000-0000-0000-000000000000".freeze end diff --git a/app/views/admin/publishers/_note.html.slim b/app/views/admin/publishers/_note.html.slim index 8b924b4475..d3f7116b04 100644 --- a/app/views/admin/publishers/_note.html.slim +++ b/app/views/admin/publishers/_note.html.slim @@ -11,7 +11,7 @@ - unless condense .note-header / The name of the person - strong= note.created_by.name + strong= note.zendesk_from_email.present? ? note.zendesk_from_email : note.created_by.name - if note.zendesk_ticket_id.present? .text-muted.mx-2 Note ported from Zendesk diff --git a/db/migrate/20190903235845_add_sender_email_to_publisher_notes.rb b/db/migrate/20190903235845_add_sender_email_to_publisher_notes.rb new file mode 100644 index 0000000000..e0764dcd07 --- /dev/null +++ b/db/migrate/20190903235845_add_sender_email_to_publisher_notes.rb @@ -0,0 +1,6 @@ +class AddSenderEmailToPublisherNotes < ActiveRecord::Migration[5.2] + def change + add_column :publisher_notes, :zendesk_to_email, :string, index: true + add_column :publisher_notes, :zendesk_from_email, :string, index: true + end +end diff --git a/db/schema.rb b/db/schema.rb index a36704857c..0e0a8c2169 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -338,9 +338,11 @@ t.datetime "created_at", null: false t.datetime "updated_at", null: false t.uuid "created_by_id", null: false + t.uuid "thread_id" t.bigint "zendesk_ticket_id" t.bigint "zendesk_comment_id" - t.uuid "thread_id" + t.string "zendesk_to_email" + t.string "zendesk_from_email" t.index ["created_by_id"], name: "index_publisher_notes_on_created_by_id" t.index ["publisher_id"], name: "index_publisher_notes_on_publisher_id" t.index ["thread_id"], name: "index_publisher_notes_on_thread_id" From 71bf6a537c887b40f9c75e3a516b6e79fdab5cff Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Fri, 11 Oct 2019 17:40:51 -0700 Subject: [PATCH 2/4] =?UTF-8?q?Always=20set=20default=20locale=20on=20admi?= =?UTF-8?q?n.=20Also=20fixes=20an=20error=20for=20invalid=20l=E2=80=A6=20(?= =?UTF-8?q?#2306)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Always set default locale on admin. Also fixes an error for invalid locales coming in. Closes https://github.com/brave-intl/publishers/issues/2294 * Only switch on locale checks for get requests --- app/controllers/application_controller.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 825e428b5e..0568946386 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -24,14 +24,15 @@ class ApplicationController < ActionController::Base around_action :switch_locale def switch_locale(&action) locale = nil + return I18n.with_locale(I18n.default_locale, &action) if params['controller'].split("/")[0] == 'admin' locale = params[:locale] if params[:locale].present? - if locale.nil? && extract_locale_from_accept_language_header == 'ja' + if locale.nil? && extract_locale_from_accept_language_header == 'ja' && request.get? new_query = URI(request.original_url).query.present? ? "&locale=ja" : "?locale=ja" redirect_to(request.original_url + new_query) and return end - locale = I18n.default_locale if locale.nil? || !locale.in?(I18n.available_locales) + locale = I18n.default_locale if locale.nil? || !locale.to_sym.in?(I18n.available_locales) I18n.with_locale(locale, &action) end From ac3a0a72b9f2833e1b496d46d6cb23ffc522ad27 Mon Sep 17 00:00:00 2001 From: Cory McDonald Date: Mon, 14 Oct 2019 10:28:14 -0500 Subject: [PATCH 3/4] Fail gracefully when we don't have access to uphold transactions (#2304) --- app/services/publisher_statement_getter.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/services/publisher_statement_getter.rb b/app/services/publisher_statement_getter.rb index b76ae8fee1..fbeddb2524 100644 --- a/app/services/publisher_statement_getter.rb +++ b/app/services/publisher_statement_getter.rb @@ -87,6 +87,9 @@ def get_uphold_transactions end uphold + rescue Faraday::ClientError + Rails.logger.info("Couldn't access publisher #{@publisher.id} Uphold Transaction History") + [] end def channel_name(identifier) From 9968bc20bc99c6ef1a6ac2f49a49c7a7aec9d754 Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Tue, 15 Oct 2019 10:44:48 -0700 Subject: [PATCH 4/4] Don't cause redirect loop by checking against request.fullpath for suspended publishers (#2315) Closes #2313 --- app/controllers/application_controller.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 0568946386..40b82a1be8 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -17,6 +17,7 @@ class ApplicationController < ActionController::Base newrelic_ignore_enduser + rescue_from Ability::AdminNotOnIPWhitelistError do |e| render file: "admin/errors/whitelist.html", layout: false end @@ -55,7 +56,11 @@ def user_for_paper_trail def redirect_if_suspended # Redirect to suspended page if they're logged in - redirect_to(suspended_error_publishers_path) and return if current_publisher.present? && current_publisher.suspended? + redirect_to(suspended_error_publishers_path) and return if current_publisher&.suspended? && !request.fullpath.split("?")[0].in?(valid_suspended_paths) + end + + def valid_suspended_paths + [suspended_error_publishers_path.split("?")[0], log_out_publishers_path.split("?")[0]] end def current_ability