Skip to content

Commit

Permalink
Email bug fixes august 2023
Browse files Browse the repository at this point in the history
The problem: 
email preview showing full email message in email threads list view:

<img width="1728" alt="Screen Shot 2023-08-10 at 5 05 05 PM" src="https://github.com/restarone/violet_rails/assets/35935196/1942d52a-9df8-4aa2-8989-fe1f9a0f31d1">



fixes,

- rendering overflow in email list view - list items are now a fixed size
- sorting when email threads are updated with new messages - threads with unread messages, or new thread show up at the top 


https://github.com/restarone/violet_rails/assets/35935196/438b3148-708c-4473-b1cb-436685d7e14b
  • Loading branch information
donrestarone authored Aug 14, 2023
1 parent 66a63e4 commit 5fbeb9a
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 21 deletions.
7 changes: 4 additions & 3 deletions app/controllers/mailbox/mailbox_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ class Mailbox::MailboxController < Mailbox::BaseController
def show
params[:q] ||= {}
@message_threads_q = if params[:categories].present?
MessageThread.includes(:categories).for_category(params[:categories]).all.includes(:messages).ransack(params[:q])
MessageThread.joins(:messages).includes(:categories).for_category(params[:categories]).all.includes(:messages).ransack(params[:q])
else
MessageThread.all.includes(:messages).ransack(params[:q])
MessageThread.joins(:messages).includes(:messages).ransack(params[:q])
end

@message_threads_q.sorts = ['created_at desc'] if @message_threads_q.sorts.empty?
@message_threads_q.sorts = ['messages_created_at desc', 'unread DESC NULLS LAST', 'created_at desc'] if @message_threads_q.sorts.empty?

@message_threads = @message_threads_q.result(distinct: true).paginate(page: params[:page], per_page: params[:per_page] || 10)
end
end
1 change: 1 addition & 0 deletions app/controllers/mailbox/message_threads_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ class Mailbox::MessageThreadsController < Mailbox::BaseController
before_action :load_thread, except: [:new, :create]

def show
if @message_thread.unread then @message_thread.update(unread: false) end
ahoy.track(
"subdomain-email-visit",
{visit_id: current_visit.id, message_thread_id: @message_thread.id, user_id: current_user.id}
Expand Down
4 changes: 4 additions & 0 deletions app/helpers/content_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ def render_api_namespace_resource(api_namespace_slug, options = {})
render body: Rails.root.join('public', '404.html').read.html_safe, status: :not_found, layout: false
end

def render_trix_preview(action_text_richtext)
action_text_richtext.to_plain_text[0, 250]
end

private

def cms_dynamic_snippet_render(identifier, cms_site = @cms_site, context = {})
Expand Down
1 change: 1 addition & 0 deletions app/mailboxes/e_mailbox.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def process
from: mail.from.join(', '),
attachments: (attachments + multipart_attached).map{ |a| a[:blob] }
)
message_thread.update(unread: true)
ApiNamespace::Plugin::V1::SubdomainEventsService.new(message).track_event
end
end
Expand Down
18 changes: 1 addition & 17 deletions app/views/mailbox/mailbox/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,4 @@
= Subdomain.current.mailing_address
= render partial: 'mailbox/mailbox/search_filters'
= render partial: 'mailbox/mailbox/pagination', locals: { data: @message_threads }
- @message_threads.each do |message_thread|
= link_to mailbox_message_thread_path(id: message_thread.id), class: 'text-reset text-decoration-none' do
.card.my-3
.card-body
.card-title.d-flex.justify-content-between
= message_thread.subject
.item-categories
= render "comfy/admin/cms/categories/categories", object: message_thread
.card-subtitle.mb-2.text-muted
%strong
%span= message_thread.recipients.join(', ')
%div
%small= "#{distance_of_time_in_words(Time.now, message_thread.updated_at)} ago (#{message_thread.updated_at})"
- if message_thread.messages.any?
.card-text.bg-light.p-2
= message_thread.messages.first.content
= render partial: 'mailbox/mailbox/pagination', locals: { data: @message_threads }
= render partial: 'mailbox/message_threads/index', locals: { data: @message_threads }
21 changes: 21 additions & 0 deletions app/views/mailbox/message_threads/_index.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
- @message_threads.each do |message_thread|
= link_to mailbox_message_thread_path(id: message_thread.id), class: 'text-reset text-decoration-none' do
.card.my-3
%div{class: message_thread.unread? ? "card-body card-header border-secondary" : "card-body"}
.card-title.d-flex.justify-content-between
= message_thread.subject
.item-categories
= render "comfy/admin/cms/categories/categories", object: message_thread
.card-subtitle.mb-2.text-muted
%strong
%span= message_thread.recipients.join(', ')
- last_message = message_thread.messages.first
%div
- if message_thread.messages.any?
%small= "#{distance_of_time_in_words(Time.now, last_message.created_at)} ago (#{last_message.created_at})"
- else
%small= "#{distance_of_time_in_words(Time.now, message_thread.updated_at)} ago (#{message_thread.updated_at})"
- if message_thread.messages.any?
%p.card-text.bg-light.p-2{ :style => "white-space: nowrap; text-overflow: ellipsis; overflow-y: hidden;" }
= render_trix_preview(last_message.content)
= render partial: 'mailbox/mailbox/pagination', locals: { data: @message_threads }
2 changes: 1 addition & 1 deletion db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def maybe_image
# ------
3.times do
recipients = [Faker::Internet.email, Faker::Internet.email]
email_thread = MessageThread.create!(recipients: recipients)
email_thread = MessageThread.create!(recipients: recipients, subject: Faker::Movie.quote)
3.times do
email_thread.messages.create(content: "
<h5>#{Faker::Movie.quote}</h5>
Expand Down
63 changes: 63 additions & 0 deletions test/controllers/mailbox/mailbox_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -205,4 +205,67 @@ class Mailbox::MailboxControllerTest < ActionDispatch::IntegrationTest
assert_includes categorized_message_thread_ids, message_thread.id
end
end

test "sort order of email threads" do
other_user = users(:one)

message_thread_1 = MessageThread.first
message_1 = message_thread_1.messages.create!(content: '<div>thread 1 message 1</div>')

message_thread_2 = MessageThread.create!(unread: true, subject: 'test', recipients: [other_user.email])
message_2 = message_thread_2.messages.create!(content: '<div>thread 2 message 2</div>')

message_thread_3 = MessageThread.create!(unread: true, subject: 'new email', recipients: [other_user.email])
message_3 = message_thread_3.messages.create!(content: '<div>thread 3 message 3</div>')

sign_in(@user)
get mailbox_url

assert_response :success

message_threads = @controller.view_assigns['message_threads']

# thread with last message should come first
assert_equal [message_thread_3.id, message_thread_2.id, message_thread_1.id], message_threads.pluck(:id)

message_4 = message_thread_2.messages.create!(content: '<div>thread 2 message 4</div>')

get mailbox_url

assert_response :success

message_threads = @controller.view_assigns['message_threads']

# thread with last message should come first
assert_equal [message_thread_2.id, message_thread_3.id, message_thread_1.id], message_threads.pluck(:id)
end

test "sort order of email threads, sort params present" do
other_user = users(:one)

message_thread_1 = MessageThread.first
message_1 = message_thread_1.messages.create!(content: '<div>thread 1 message 1</div>')

message_thread_2 = MessageThread.create!(unread: true, subject: 'test', recipients: [other_user.email])
message_2 = message_thread_2.messages.create!(content: '<div>thread 2 message 2</div>')

message_thread_3 = MessageThread.create!(unread: true, subject: 'new email', recipients: [other_user.email])
message_3 = message_thread_3.messages.create!(content: '<div>thread 3 message 3</div>')

sign_in(@user)
get mailbox_url, params: { q: { s: 'id asc'}}

message_threads = @controller.view_assigns['message_threads']

# should be sorted as specified in params
assert_equal [message_thread_1.id, message_thread_2.id, message_thread_3.id], message_threads.pluck(:id)

message_4 = message_thread_2.messages.create!(content: '<div>thread 2 message 4</div>')

get mailbox_url, params: { q: { s: 'created_at desc'}}

message_threads = @controller.view_assigns['message_threads']

assert_equal [message_thread_3.id, message_thread_2.id, message_thread_1.id], message_threads.pluck(:id)
end
end
4 changes: 4 additions & 0 deletions test/controllers/mailbox/message_threads_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -221,4 +221,8 @@ class Mailbox::MessageThreadsControllerTest < ActionDispatch::IntegrationTest
end
end
end

test 'viewing unread thread sets unread:true' do
# todo test https://github.com/restarone/violet_rails/blob/9476c661537a1688a81c95802d5f49a6617f0678/app/controllers/mailbox/message_threads_controller.rb
end
end
4 changes: 4 additions & 0 deletions test/mailboxes/e_mailbox_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -374,4 +374,8 @@ class EMailboxTest < ActionMailbox::TestCase
end
end
end

test "new message in thread sets thread unread: true" do
# todo test https://github.com/restarone/violet_rails/blob/57739a34ea8927ba222a42d372908a82e35de8cf/app/mailboxes/e_mailbox.rb
end
end

0 comments on commit 5fbeb9a

Please sign in to comment.