mirror of
https://github.com/Freika/dawarich.git
synced 2026-01-10 01:01:39 -05:00
Merge pull request #1924 from Freika/fix/family-invitation-email
Move sending family invitation email to a background job
This commit is contained in:
Evgenii Burmakin
GitHub
commit
a8e9df6f1a
11 changed files with 226 additions and 117 deletions
|
|
@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
|
|||
|
||||
- Taiwan flag is now shown on its own instead of in combination with China flag.
|
||||
- On the registration page and other user forms, if something goes wrong, error messages are now shown to the user.
|
||||
- Leaving family, deleting family and cancelling invitations now prompt confirmation dialog to prevent accidental actions.
|
||||
- Each pending family invitation now also contain a link to share with the invitee.
|
||||
|
||||
## Changed
|
||||
|
||||
|
|
|
|||
6
Gemfile
6
Gemfile
|
|
@ -5,14 +5,14 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" }
|
|||
|
||||
ruby File.read('.ruby-version').strip
|
||||
|
||||
gem 'activerecord-postgis-adapter'
|
||||
gem 'activerecord-postgis-adapter', '~> 11.0'
|
||||
# https://meta.discourse.org/t/cant-rebuild-due-to-aws-sdk-gem-bump-and-new-aws-data-integrity-protections/354217/40
|
||||
gem 'aws-sdk-core', '~> 3.215.1', require: false
|
||||
gem 'aws-sdk-kms', '~> 1.96.0', require: false
|
||||
gem 'aws-sdk-s3', '~> 1.177.0', require: false
|
||||
gem 'bootsnap', require: false
|
||||
gem 'chartkick'
|
||||
gem 'data_migrate', '>= 11.3.1'
|
||||
gem 'data_migrate'
|
||||
gem 'devise'
|
||||
gem 'geocoder', github: 'Freika/geocoder', branch: 'master'
|
||||
gem 'gpx'
|
||||
|
|
@ -34,7 +34,7 @@ gem 'rails_icons'
|
|||
gem 'redis'
|
||||
gem 'rexml'
|
||||
gem 'rgeo'
|
||||
gem 'rgeo-activerecord', '>= 8.1.0'
|
||||
gem 'rgeo-activerecord', '~> 8.0.0'
|
||||
gem 'rgeo-geojson'
|
||||
gem 'rqrcode', '~> 3.0'
|
||||
gem 'rswag-api'
|
||||
|
|
|
|||
134
Gemfile.lock
134
Gemfile.lock
|
|
@ -10,29 +10,29 @@ GIT
|
|||
GEM
|
||||
remote: https://rubygems.org/
|
||||
specs:
|
||||
actioncable (8.0.4)
|
||||
actionpack (= 8.0.4)
|
||||
activesupport (= 8.0.4)
|
||||
actioncable (8.0.3)
|
||||
actionpack (= 8.0.3)
|
||||
activesupport (= 8.0.3)
|
||||
nio4r (~> 2.0)
|
||||
websocket-driver (>= 0.6.1)
|
||||
zeitwerk (~> 2.6)
|
||||
actionmailbox (8.0.4)
|
||||
actionpack (= 8.0.4)
|
||||
activejob (= 8.0.4)
|
||||
activerecord (= 8.0.4)
|
||||
activestorage (= 8.0.4)
|
||||
activesupport (= 8.0.4)
|
||||
actionmailbox (8.0.3)
|
||||
actionpack (= 8.0.3)
|
||||
activejob (= 8.0.3)
|
||||
activerecord (= 8.0.3)
|
||||
activestorage (= 8.0.3)
|
||||
activesupport (= 8.0.3)
|
||||
mail (>= 2.8.0)
|
||||
actionmailer (8.0.4)
|
||||
actionpack (= 8.0.4)
|
||||
actionview (= 8.0.4)
|
||||
activejob (= 8.0.4)
|
||||
activesupport (= 8.0.4)
|
||||
actionmailer (8.0.3)
|
||||
actionpack (= 8.0.3)
|
||||
actionview (= 8.0.3)
|
||||
activejob (= 8.0.3)
|
||||
activesupport (= 8.0.3)
|
||||
mail (>= 2.8.0)
|
||||
rails-dom-testing (~> 2.2)
|
||||
actionpack (8.0.4)
|
||||
actionview (= 8.0.4)
|
||||
activesupport (= 8.0.4)
|
||||
actionpack (8.0.3)
|
||||
actionview (= 8.0.3)
|
||||
activesupport (= 8.0.3)
|
||||
nokogiri (>= 1.8.5)
|
||||
rack (>= 2.2.4)
|
||||
rack-session (>= 1.0.1)
|
||||
|
|
@ -40,38 +40,38 @@ GEM
|
|||
rails-dom-testing (~> 2.2)
|
||||
rails-html-sanitizer (~> 1.6)
|
||||
useragent (~> 0.16)
|
||||
actiontext (8.0.4)
|
||||
actionpack (= 8.0.4)
|
||||
activerecord (= 8.0.4)
|
||||
activestorage (= 8.0.4)
|
||||
activesupport (= 8.0.4)
|
||||
actiontext (8.0.3)
|
||||
actionpack (= 8.0.3)
|
||||
activerecord (= 8.0.3)
|
||||
activestorage (= 8.0.3)
|
||||
activesupport (= 8.0.3)
|
||||
globalid (>= 0.6.0)
|
||||
nokogiri (>= 1.8.5)
|
||||
actionview (8.0.4)
|
||||
activesupport (= 8.0.4)
|
||||
actionview (8.0.3)
|
||||
activesupport (= 8.0.3)
|
||||
builder (~> 3.1)
|
||||
erubi (~> 1.11)
|
||||
rails-dom-testing (~> 2.2)
|
||||
rails-html-sanitizer (~> 1.6)
|
||||
activejob (8.0.4)
|
||||
activesupport (= 8.0.4)
|
||||
activejob (8.0.3)
|
||||
activesupport (= 8.0.3)
|
||||
globalid (>= 0.3.6)
|
||||
activemodel (8.0.4)
|
||||
activesupport (= 8.0.4)
|
||||
activerecord (8.0.4)
|
||||
activemodel (= 8.0.4)
|
||||
activesupport (= 8.0.4)
|
||||
activemodel (8.0.3)
|
||||
activesupport (= 8.0.3)
|
||||
activerecord (8.0.3)
|
||||
activemodel (= 8.0.3)
|
||||
activesupport (= 8.0.3)
|
||||
timeout (>= 0.4.0)
|
||||
activerecord-postgis-adapter (11.0.0)
|
||||
activerecord (~> 8.0.0)
|
||||
rgeo-activerecord (~> 8.0.0)
|
||||
activestorage (8.0.4)
|
||||
actionpack (= 8.0.4)
|
||||
activejob (= 8.0.4)
|
||||
activerecord (= 8.0.4)
|
||||
activesupport (= 8.0.4)
|
||||
activestorage (8.0.3)
|
||||
actionpack (= 8.0.3)
|
||||
activejob (= 8.0.3)
|
||||
activerecord (= 8.0.3)
|
||||
activesupport (= 8.0.3)
|
||||
marcel (~> 1.0)
|
||||
activesupport (8.0.4)
|
||||
activesupport (8.0.3)
|
||||
base64
|
||||
benchmark (>= 0.3)
|
||||
bigdecimal
|
||||
|
|
@ -106,7 +106,7 @@ GEM
|
|||
aws-eventstream (~> 1, >= 1.0.2)
|
||||
base64 (0.3.0)
|
||||
bcrypt (3.1.20)
|
||||
benchmark (0.5.0)
|
||||
benchmark (0.4.1)
|
||||
bigdecimal (3.3.1)
|
||||
bootsnap (1.18.6)
|
||||
msgpack (~> 1.2)
|
||||
|
|
@ -139,7 +139,7 @@ GEM
|
|||
tzinfo
|
||||
unicode (>= 0.4.4.5)
|
||||
csv (3.3.4)
|
||||
data_migrate (11.3.0)
|
||||
data_migrate (11.3.1)
|
||||
activerecord (>= 6.1)
|
||||
railties (>= 6.1)
|
||||
database_consistency (2.0.6)
|
||||
|
|
@ -328,20 +328,20 @@ GEM
|
|||
rack (>= 1.3)
|
||||
rackup (2.2.1)
|
||||
rack (>= 3)
|
||||
rails (8.0.4)
|
||||
actioncable (= 8.0.4)
|
||||
actionmailbox (= 8.0.4)
|
||||
actionmailer (= 8.0.4)
|
||||
actionpack (= 8.0.4)
|
||||
actiontext (= 8.0.4)
|
||||
actionview (= 8.0.4)
|
||||
activejob (= 8.0.4)
|
||||
activemodel (= 8.0.4)
|
||||
activerecord (= 8.0.4)
|
||||
activestorage (= 8.0.4)
|
||||
activesupport (= 8.0.4)
|
||||
rails (8.0.3)
|
||||
actioncable (= 8.0.3)
|
||||
actionmailbox (= 8.0.3)
|
||||
actionmailer (= 8.0.3)
|
||||
actionpack (= 8.0.3)
|
||||
actiontext (= 8.0.3)
|
||||
actionview (= 8.0.3)
|
||||
activejob (= 8.0.3)
|
||||
activemodel (= 8.0.3)
|
||||
activerecord (= 8.0.3)
|
||||
activestorage (= 8.0.3)
|
||||
activesupport (= 8.0.3)
|
||||
bundler (>= 1.15.0)
|
||||
railties (= 8.0.4)
|
||||
railties (= 8.0.3)
|
||||
rails-dom-testing (2.3.0)
|
||||
activesupport (>= 5.0.0)
|
||||
minitest
|
||||
|
|
@ -352,9 +352,9 @@ GEM
|
|||
rails_icons (1.4.0)
|
||||
nokogiri (~> 1.16, >= 1.16.4)
|
||||
rails (> 6.1)
|
||||
railties (8.0.4)
|
||||
actionpack (= 8.0.4)
|
||||
activesupport (= 8.0.4)
|
||||
railties (8.0.3)
|
||||
actionpack (= 8.0.3)
|
||||
activesupport (= 8.0.3)
|
||||
irb (~> 1.13)
|
||||
rackup (>= 1.0.0)
|
||||
rake (>= 12.2)
|
||||
|
|
@ -408,17 +408,17 @@ GEM
|
|||
rspec-mocks (~> 3.13)
|
||||
rspec-support (~> 3.13)
|
||||
rspec-support (3.13.3)
|
||||
rswag-api (2.16.0)
|
||||
activesupport (>= 5.2, < 8.1)
|
||||
railties (>= 5.2, < 8.1)
|
||||
rswag-specs (2.16.0)
|
||||
activesupport (>= 5.2, < 8.1)
|
||||
json-schema (>= 2.2, < 6.0)
|
||||
railties (>= 5.2, < 8.1)
|
||||
rswag-api (2.17.0)
|
||||
activesupport (>= 5.2, < 8.2)
|
||||
railties (>= 5.2, < 8.2)
|
||||
rswag-specs (2.17.0)
|
||||
activesupport (>= 5.2, < 8.2)
|
||||
json-schema (>= 2.2, < 7.0)
|
||||
railties (>= 5.2, < 8.2)
|
||||
rspec-core (>= 2.14)
|
||||
rswag-ui (2.16.0)
|
||||
actionpack (>= 5.2, < 8.1)
|
||||
railties (>= 5.2, < 8.1)
|
||||
rswag-ui (2.17.0)
|
||||
actionpack (>= 5.2, < 8.2)
|
||||
railties (>= 5.2, < 8.2)
|
||||
rubocop (1.81.1)
|
||||
json (~> 2.3)
|
||||
language_server-protocol (~> 3.17.0.2)
|
||||
|
|
@ -540,7 +540,7 @@ PLATFORMS
|
|||
x86_64-linux
|
||||
|
||||
DEPENDENCIES
|
||||
activerecord-postgis-adapter
|
||||
activerecord-postgis-adapter (~> 11.0)
|
||||
aws-sdk-core (~> 3.215.1)
|
||||
aws-sdk-kms (~> 1.96.0)
|
||||
aws-sdk-s3 (~> 1.177.0)
|
||||
|
|
@ -580,7 +580,7 @@ DEPENDENCIES
|
|||
redis
|
||||
rexml
|
||||
rgeo
|
||||
rgeo-activerecord
|
||||
rgeo-activerecord (~> 8.0.0)
|
||||
rgeo-geojson
|
||||
rqrcode (~> 3.0)
|
||||
rspec-rails (>= 8.0.1)
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
|
|
@ -13,9 +13,10 @@ class Family::Invitations::CleanupJob < ApplicationJob
|
|||
Rails.logger.info "Updated #{expired_count} expired family invitations"
|
||||
|
||||
cleanup_threshold = 30.days.ago
|
||||
deleted_count = Family::Invitation.where(status: [:expired, :cancelled])
|
||||
.where('updated_at < ?', cleanup_threshold)
|
||||
.delete_all
|
||||
deleted_count =
|
||||
Family::Invitation.where(status: %i[expired cancelled])
|
||||
.where('updated_at < ?', cleanup_threshold)
|
||||
.delete_all
|
||||
|
||||
Rails.logger.info "Deleted #{deleted_count} old family invitations"
|
||||
|
||||
|
|
|
|||
13
app/jobs/family/invitations/sending_job.rb
Normal file
13
app/jobs/family/invitations/sending_job.rb
Normal file
|
|
@ -0,0 +1,13 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class Family::Invitations::SendingJob < ApplicationJob
|
||||
queue_as :families
|
||||
|
||||
def perform(invitation_id)
|
||||
invitation = Family::Invitation.find_by(id: invitation_id)
|
||||
|
||||
return unless invitation&.pending?
|
||||
|
||||
FamilyMailer.invitation(invitation).deliver_now
|
||||
end
|
||||
end
|
||||
|
|
@ -19,8 +19,8 @@ module Families
|
|||
return false unless invite_sendable?
|
||||
|
||||
ActiveRecord::Base.transaction do
|
||||
create_invitation
|
||||
send_invitation_email
|
||||
invitation = create_invitation
|
||||
send_invitation_email(invitation)
|
||||
send_notification
|
||||
end
|
||||
|
||||
|
|
@ -80,16 +80,18 @@ module Families
|
|||
)
|
||||
end
|
||||
|
||||
def send_invitation_email
|
||||
# Send email in background with retry logic
|
||||
FamilyMailer.invitation(@invitation).deliver_later(
|
||||
queue: :mailer,
|
||||
retry: 3,
|
||||
wait: 30.seconds
|
||||
)
|
||||
def send_invitation_email(invitation)
|
||||
Family::Invitations::SendingJob.perform_later(invitation.id)
|
||||
end
|
||||
|
||||
def send_notification
|
||||
message =
|
||||
if DawarichSettings.self_hosted?
|
||||
"Family invitation sent to #{email} if SMTP is configured properly. If you're not using SMTP, copy the invitation link from the family page and share it manually."
|
||||
else
|
||||
"Family invitation sent to #{email}"
|
||||
end
|
||||
|
||||
Notification.create!(
|
||||
user: invited_by,
|
||||
kind: :info,
|
||||
|
|
|
|||
|
|
@ -88,7 +88,7 @@
|
|||
<% if policy(@family).destroy? %>
|
||||
<%= link_to family_path,
|
||||
method: :delete,
|
||||
data: { turbo_confirm: 'Are you sure you want to delete this family? This action cannot be undone.' },
|
||||
data: { turbo_confirm: 'Are you sure you want to delete this family? This action cannot be undone.', turbo_method: :delete },
|
||||
class: "btn btn-outline btn-error" do %>
|
||||
<%= icon 'trash-2', class: "inline-block w-4" %>
|
||||
Delete Family
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@
|
|||
<% if !current_user.family_owner? && current_user.family_membership %>
|
||||
<%= link_to family_member_path(current_user.family_membership),
|
||||
method: :delete,
|
||||
data: { turbo_confirm: 'Are you sure you want to leave this family?' },
|
||||
data: { turbo_confirm: 'Are you sure you want to leave this family?', turbo_method: :delete },
|
||||
class: "btn btn-outline btm-sm btn-warning" do %>
|
||||
Leave Family
|
||||
<% end %>
|
||||
|
|
@ -35,7 +35,7 @@
|
|||
<% if policy(@family).destroy? %>
|
||||
<%= link_to family_path,
|
||||
method: :delete,
|
||||
data: { turbo_confirm: 'Are you sure you want to delete this family? This action cannot be undone.' },
|
||||
data: { turbo_confirm: 'Are you sure you want to delete this family? This action cannot be undone.', turbo_method: :delete },
|
||||
class: "btn btn-outline btm-sm btn-error" do %>
|
||||
<%= icon 'trash-2', class: "inline-block w-4" %>
|
||||
Delete
|
||||
|
|
@ -175,38 +175,46 @@
|
|||
<% if @pending_invitations.any? %>
|
||||
<div class="space-y-3 mb-4">
|
||||
<% @pending_invitations.each do |invitation| %>
|
||||
<div class="flex items-center justify-between p-3 bg-base-100 rounded-lg">
|
||||
<div class="flex-grow">
|
||||
<div class="font-medium text-base-content"><%= invitation.email %></div>
|
||||
<div class="text-sm text-base-content opacity-60">
|
||||
<%= t('families.show.invited_on', default: 'Invited') %>
|
||||
<%= invitation.created_at.strftime('%b %d, %Y') %>
|
||||
</div>
|
||||
<div class="text-xs text-base-content opacity-50">
|
||||
<%= t('families.show.expires_on', default: 'Expires') %>
|
||||
<%= invitation.expires_at.strftime('%b %d, %Y at %I:%M %p') %>
|
||||
</div>
|
||||
<div class="mt-2">
|
||||
<button data-controller="clipboard"
|
||||
data-clipboard-text-value="<%= public_invitation_url(invitation.token) %>"
|
||||
data-action="click->clipboard#copy"
|
||||
class="btn btn-outline btn-info btn-xs"
|
||||
title="Copy invitation link">
|
||||
<%= icon 'copy', class: "inline-block w-3" %>
|
||||
Copy Invitation Link
|
||||
</button>
|
||||
<div class="p-3 bg-base-100 rounded-lg">
|
||||
<div class="flex items-center justify-between">
|
||||
<div class="flex-grow">
|
||||
<div class="font-medium text-base-content"><%= invitation.email %></div>
|
||||
<div class="text-sm text-base-content opacity-60">
|
||||
<%= t('families.show.invited_on', default: 'Invited') %>
|
||||
<%= invitation.created_at.strftime('%b %d, %Y') %>
|
||||
</div>
|
||||
<div class="text-xs text-base-content opacity-50">
|
||||
<%= t('families.show.expires_on', default: 'Expires') %>
|
||||
<%= invitation.expires_at.strftime('%b %d, %Y at %I:%M %p') %>
|
||||
</div>
|
||||
</div>
|
||||
<% if policy(@family).manage_invitations? %>
|
||||
<div class="ml-3">
|
||||
<%= button_to family_invitation_path(invitation.token),
|
||||
method: :delete,
|
||||
form: { data: { turbo_confirm: 'Are you sure you want to cancel this invitation?', turbo_method: :delete } },
|
||||
class: "btn btn-outline btn-warning btn-sm" do %>
|
||||
Cancel
|
||||
<% end %>
|
||||
</div>
|
||||
<% end %>
|
||||
</div>
|
||||
<div class="flex items-center gap-2 mt-3">
|
||||
<input type="text"
|
||||
readonly
|
||||
value="<%= public_invitation_url(invitation.token) %>"
|
||||
class="input input-bordered input-sm flex-grow"
|
||||
onclick="this.select();"
|
||||
/>
|
||||
<button data-controller="clipboard"
|
||||
data-clipboard-text-value="<%= public_invitation_url(invitation.token) %>"
|
||||
data-action="click->clipboard#copy"
|
||||
class="btn btn-outline btn-info btn-sm ml-auto"
|
||||
title="Copy invitation link">
|
||||
<%= icon 'copy', class: "inline-block w-3" %>
|
||||
Copy Invitation Link
|
||||
</button>
|
||||
</div>
|
||||
<% if policy(@family).manage_invitations? %>
|
||||
<div class="ml-3">
|
||||
<%= link_to family_invitation_path(invitation.token),
|
||||
method: :delete,
|
||||
data: { turbo_confirm: 'Are you sure you want to cancel this invitation?' },
|
||||
class: "btn btn-outline btn-warning btn-sm" do %>
|
||||
Cancel
|
||||
<% end %>
|
||||
</div>
|
||||
<% end %>
|
||||
</div>
|
||||
<% end %>
|
||||
</div>
|
||||
|
|
|
|||
78
spec/jobs/family/invitations/sending_job_spec.rb
Normal file
78
spec/jobs/family/invitations/sending_job_spec.rb
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require 'rails_helper'
|
||||
|
||||
RSpec.describe Family::Invitations::SendingJob, type: :job do
|
||||
let(:user) { create(:user) }
|
||||
let(:family) { create(:family, creator: user) }
|
||||
let(:invitation) { create(:family_invitation, family: family, invited_by: user, status: :pending) }
|
||||
|
||||
describe '#perform' do
|
||||
context 'when invitation exists and is pending' do
|
||||
it 'sends the invitation email' do
|
||||
mailer_double = double('mailer')
|
||||
expect(FamilyMailer).to receive(:invitation).with(invitation).and_return(mailer_double)
|
||||
expect(mailer_double).to receive(:deliver_now)
|
||||
|
||||
described_class.perform_now(invitation.id)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when invitation does not exist' do
|
||||
it 'does not raise an error' do
|
||||
expect do
|
||||
described_class.perform_now(999_999)
|
||||
end.not_to raise_error
|
||||
end
|
||||
|
||||
it 'does not send any email' do
|
||||
expect(FamilyMailer).not_to receive(:invitation)
|
||||
|
||||
described_class.perform_now(999_999)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when invitation is not pending' do
|
||||
let(:accepted_invitation) do
|
||||
create(:family_invitation, family: family, invited_by: user, status: :accepted)
|
||||
end
|
||||
|
||||
it 'does not send the invitation email' do
|
||||
expect(FamilyMailer).not_to receive(:invitation)
|
||||
|
||||
described_class.perform_now(accepted_invitation.id)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when invitation is cancelled' do
|
||||
let(:cancelled_invitation) do
|
||||
create(:family_invitation, family: family, invited_by: user, status: :cancelled)
|
||||
end
|
||||
|
||||
it 'does not send the invitation email' do
|
||||
expect(FamilyMailer).not_to receive(:invitation)
|
||||
|
||||
described_class.perform_now(cancelled_invitation.id)
|
||||
end
|
||||
end
|
||||
|
||||
context 'integration test' do
|
||||
before do
|
||||
ActionMailer::Base.deliveries.clear
|
||||
# Set a from address for the mailer to avoid SMTP errors
|
||||
allow(ActionMailer::Base).to receive(:default).and_return(from: 'noreply@dawarich.app')
|
||||
end
|
||||
|
||||
it 'actually calls the mailer' do
|
||||
mailer = instance_double(ActionMailer::MessageDelivery)
|
||||
allow(FamilyMailer).to receive(:invitation).and_return(mailer)
|
||||
allow(mailer).to receive(:deliver_now)
|
||||
|
||||
described_class.perform_now(invitation.id)
|
||||
|
||||
expect(FamilyMailer).to have_received(:invitation).with(invitation)
|
||||
expect(mailer).to have_received(:deliver_now)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -21,6 +21,11 @@ RSpec.describe Families::Invite do
|
|||
expect(invitation.invited_by).to eq(owner)
|
||||
end
|
||||
|
||||
it 'enqueues invitation sending job' do
|
||||
expect(Family::Invitations::SendingJob).to receive(:perform_later).with(an_instance_of(Integer))
|
||||
service.call
|
||||
end
|
||||
|
||||
it 'sends invitation email' do
|
||||
expect(FamilyMailer).to receive(:invitation).and_call_original
|
||||
expect_any_instance_of(ActionMailer::MessageDelivery).to receive(:deliver_later)
|
||||
|
|
|
|||
Loading…
Reference in a new issue