From e711ff25fe279a658df181effeb6cab116b20906 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Tue, 7 Oct 2025 18:38:06 +0200 Subject: [PATCH] Refactor family invitations and memberships into separate models and controllers --- Procfile | 1 + app.json | 5 -- .../family/invitations_controller.rb | 37 +------- .../family/memberships_controller.rb | 35 +++++++- .../users/registrations_controller.rb | 2 +- app/controllers/users/sessions_controller.rb | 4 +- app/jobs/family/invitations/cleanup_job.rb | 12 +-- app/mailers/family_mailer.rb | 4 +- app/models/concerns/user_family.rb | 59 +++++-------- app/models/family.rb | 4 +- app/models/family/invitation.rb | 46 ++++++++++ app/models/family/membership.rb | 23 +++++ app/policies/family/invitation_policy.rb | 22 +++++ app/policies/family/membership_policy.rb | 23 +++++ app/services/families/accept_invitation.rb | 2 +- app/services/families/create.rb | 2 +- app/services/families/invite.rb | 2 +- app/views/family/invitations/show.html.erb | 2 +- app/views/family_mailer/invitation.html.erb | 4 +- app/views/family_mailer/invitation.text.erb | 2 +- app/views/shared/_navbar.html.erb | 6 +- .../stats/_reverse_geocoding_stats.html.erb | 27 +++--- config/routes.rb | 7 +- spec/factories/family_invitations.rb | 2 +- spec/factories/family_memberships.rb | 2 +- .../invitation_spec.rb} | 12 +-- .../membership_spec.rb} | 2 +- spec/models/family_spec.rb | 8 +- spec/models/user_family_spec.rb | 6 +- spec/requests/families_spec.rb | 2 +- spec/requests/family/invitations_spec.rb | 83 +----------------- spec/requests/family/memberships_spec.rb | 85 ++++++++++++++++++- spec/requests/family_workflows_spec.rb | 12 +-- .../families/accept_invitation_spec.rb | 14 +-- spec/services/families/create_spec.rb | 6 +- spec/services/families/invite_spec.rb | 10 +-- .../families/memberships/destroy_spec.rb | 6 +- 37 files changed, 336 insertions(+), 245 deletions(-) create mode 100644 app/models/family/invitation.rb create mode 100644 app/models/family/membership.rb create mode 100644 app/policies/family/invitation_policy.rb create mode 100644 app/policies/family/membership_policy.rb rename spec/models/{family_invitation_spec.rb => family/invitation_spec.rb} (93%) rename spec/models/{family_membership_spec.rb => family/membership_spec.rb} (97%) diff --git a/Procfile b/Procfile index fd4fe014..3eb630b7 100644 --- a/Procfile +++ b/Procfile @@ -1,2 +1,3 @@ +release: bundle exec rails db:migrate web: bundle exec puma -C config/puma.rb worker: bundle exec sidekiq -C config/sidekiq.yml diff --git a/app.json b/app.json index 9c425d4e..fcf27c70 100644 --- a/app.json +++ b/app.json @@ -5,11 +5,6 @@ { "url": "https://github.com/heroku/heroku-buildpack-nodejs.git" }, { "url": "https://github.com/heroku/heroku-buildpack-ruby.git" } ], - "scripts": { - "dokku": { - "predeploy": "bundle exec rails db:migrate" - } - }, "healthchecks": { "web": [ { diff --git a/app/controllers/family/invitations_controller.rb b/app/controllers/family/invitations_controller.rb index 4d4f13e3..040451c1 100644 --- a/app/controllers/family/invitations_controller.rb +++ b/app/controllers/family/invitations_controller.rb @@ -3,9 +3,8 @@ class Family::InvitationsController < ApplicationController before_action :authenticate_user!, except: %i[show] before_action :ensure_family_feature_enabled!, except: %i[show] - before_action :set_family, except: %i[show accept] + before_action :set_family, except: %i[show] before_action :set_invitation_by_id_and_family, only: %i[destroy] - before_action :set_invitation_by_id, only: %i[accept] def index authorize @family, :show? @@ -14,7 +13,7 @@ class Family::InvitationsController < ApplicationController end def show - @invitation = FamilyInvitation.find_by!(token: params[:token]) + @invitation = Family::Invitation.find_by!(token: params[:token]) if @invitation.expired? redirect_to root_path, alert: 'This invitation has expired.' and return @@ -41,34 +40,6 @@ class Family::InvitationsController < ApplicationController end end - def accept - unless @invitation.pending? - redirect_to root_path, alert: 'This invitation has already been processed' and return - end - - if @invitation.expired? - redirect_to root_path, alert: 'This invitation is no longer valid or has expired' and return - end - - if @invitation.email != current_user.email - redirect_to root_path, alert: 'This invitation is not for your email address' and return - end - - service = Families::AcceptInvitation.new( - invitation: @invitation, - user: current_user - ) - - if service.call - redirect_to family_path, notice: 'Welcome to the family!' - else - redirect_to root_path, alert: service.error_message || 'Unable to accept invitation' - end - rescue StandardError => e - Rails.logger.error "Error accepting family invitation: #{e.message}" - redirect_to root_path, alert: 'An unexpected error occurred. Please try again later' - end - def destroy authorize @family, :manage_invitations? @@ -92,10 +63,6 @@ class Family::InvitationsController < ApplicationController redirect_to new_family_path, alert: 'You are not in a family' and return unless @family end - def set_invitation_by_id - @invitation = FamilyInvitation.find_by!(token: params[:id]) - end - def set_invitation_by_id_and_family # For authenticated nested routes: /families/:family_id/invitations/:id # The :id param contains the token value diff --git a/app/controllers/family/memberships_controller.rb b/app/controllers/family/memberships_controller.rb index 65784f07..34750f42 100644 --- a/app/controllers/family/memberships_controller.rb +++ b/app/controllers/family/memberships_controller.rb @@ -3,8 +3,37 @@ class Family::MembershipsController < ApplicationController before_action :authenticate_user! before_action :ensure_family_feature_enabled! - before_action :set_family + before_action :set_family, except: %i[create] before_action :set_membership, only: %i[destroy] + before_action :set_invitation, only: %i[create] + + def create + unless @invitation.pending? + redirect_to root_path, alert: 'This invitation has already been processed' and return + end + + if @invitation.expired? + redirect_to root_path, alert: 'This invitation is no longer valid or has expired' and return + end + + if @invitation.email != current_user.email + redirect_to root_path, alert: 'This invitation is not for your email address' and return + end + + service = Families::AcceptInvitation.new( + invitation: @invitation, + user: current_user + ) + + if service.call + redirect_to family_path, notice: 'Welcome to the family!' + else + redirect_to root_path, alert: service.error_message || 'Unable to accept invitation' + end + rescue StandardError => e + Rails.logger.error "Error accepting family invitation: #{e.message}" + redirect_to root_path, alert: 'An unexpected error occurred. Please try again later' + end def destroy authorize @membership @@ -34,4 +63,8 @@ class Family::MembershipsController < ApplicationController def set_membership @membership = @family.family_memberships.find(params[:id]) end + + def set_invitation + @invitation = Family::Invitation.find_by!(token: params[:token]) + end end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index 8193d199..fd6a448c 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -49,7 +49,7 @@ class Users::RegistrationsController < Devise::RegistrationsController def set_invitation return unless invitation_token.present? - @invitation = FamilyInvitation.find_by(token: invitation_token) + @invitation = Family::Invitation.find_by(token: invitation_token) end def self_hosted_mode? diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 58e5984a..151bddc5 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -11,7 +11,7 @@ class Users::SessionsController < Devise::SessionsController def after_sign_in_path_for(resource) if invitation_token.present? - invitation = FamilyInvitation.find_by(token: invitation_token) + invitation = Family::Invitation.find_by(token: invitation_token) if invitation&.can_be_accepted? return family_invitation_path(invitation.token) @@ -26,7 +26,7 @@ class Users::SessionsController < Devise::SessionsController def load_invitation_context return unless invitation_token.present? - @invitation = FamilyInvitation.find_by(token: invitation_token) + @invitation = Family::Invitation.find_by(token: invitation_token) end def invitation_token diff --git a/app/jobs/family/invitations/cleanup_job.rb b/app/jobs/family/invitations/cleanup_job.rb index b55d39c7..b5076472 100644 --- a/app/jobs/family/invitations/cleanup_job.rb +++ b/app/jobs/family/invitations/cleanup_job.rb @@ -6,16 +6,16 @@ class FamilyInvitationsCleanupJob < ApplicationJob def perform Rails.logger.info 'Starting family invitations cleanup' - expired_count = FamilyInvitation.where(status: :pending) - .where('expires_at < ?', Time.current) - .update_all(status: :expired) + expired_count = Family::Invitation.where(status: :pending) + .where('expires_at < ?', Time.current) + .update_all(status: :expired) Rails.logger.info "Updated #{expired_count} expired family invitations" cleanup_threshold = 30.days.ago - deleted_count = FamilyInvitation.where(status: [:expired, :cancelled]) - .where('updated_at < ?', cleanup_threshold) - .delete_all + deleted_count = Family::Invitation.where(status: [:expired, :cancelled]) + .where('updated_at < ?', cleanup_threshold) + .delete_all Rails.logger.info "Deleted #{deleted_count} old family invitations" diff --git a/app/mailers/family_mailer.rb b/app/mailers/family_mailer.rb index 46c57c53..e86c68ea 100644 --- a/app/mailers/family_mailer.rb +++ b/app/mailers/family_mailer.rb @@ -6,10 +6,10 @@ class FamilyMailer < ApplicationMailer @family = invitation.family @invited_by = invitation.invited_by @accept_url = family_invitation_url(@invitation.token) - pp @accept_url + mail( to: @invitation.email, - subject: "You've been invited to join #{@family.name} on Dawarich" + subject: "🎉 You've been invited to join #{@family.name} on Dawarich!" ) end end diff --git a/app/models/concerns/user_family.rb b/app/models/concerns/user_family.rb index de1f56ef..53119792 100644 --- a/app/models/concerns/user_family.rb +++ b/app/models/concerns/user_family.rb @@ -4,11 +4,10 @@ module UserFamily extend ActiveSupport::Concern included do - # Family associations - has_one :family_membership, dependent: :destroy + has_one :family_membership, dependent: :destroy, class_name: 'Family::Membership' has_one :family, through: :family_membership has_one :created_family, class_name: 'Family', foreign_key: 'creator_id', inverse_of: :creator, dependent: :destroy - has_many :sent_family_invitations, class_name: 'FamilyInvitation', foreign_key: 'invited_by_id', + has_many :sent_family_invitations, class_name: 'Family::Invitation', foreign_key: 'invited_by_id', inverse_of: :invited_by, dependent: :destroy before_destroy :check_family_ownership @@ -30,25 +29,14 @@ module UserFamily end def family_sharing_enabled? - # User must be in a family and have explicitly enabled location sharing return false unless in_family? sharing_settings = settings.dig('family', 'location_sharing') - return false if sharing_settings.blank? + return false unless sharing_settings.is_a?(Hash) + return false unless sharing_settings['enabled'] == true - # If it's a boolean (legacy support), return it - return sharing_settings if [true, false].include?(sharing_settings) - - # If it's time-limited sharing, check if it's still active - if sharing_settings.is_a?(Hash) - return false unless sharing_settings['enabled'] == true - - # Check if sharing has an expiration - expires_at = sharing_settings['expires_at'] - return expires_at.blank? || Time.parse(expires_at) > Time.current - end - - false + expires_at = sharing_settings['expires_at'] + expires_at.blank? || Time.parse(expires_at).future? end def update_family_location_sharing!(enabled, duration: nil) @@ -60,21 +48,14 @@ module UserFamily if enabled sharing_config = { 'enabled' => true } - # Add expiration if duration is specified if duration.present? expiration_time = case duration - when '1h' - 1.hour.from_now - when '6h' - 6.hours.from_now - when '12h' - 12.hours.from_now - when '24h' - 24.hours.from_now - when 'permanent' - nil # No expiration - else - duration.to_i.hours.from_now if duration.to_i > 0 + when '1h' then 1.hour.from_now + when '6h' then 6.hours.from_now + when '12h' then 12.hours.from_now + when '24h' then 24.hours.from_now + when 'permanent' then nil + else duration.to_i.hours.from_now if duration.to_i > 0 end sharing_config['expires_at'] = expiration_time.iso8601 if expiration_time @@ -106,21 +87,21 @@ module UserFamily def latest_location_for_family return nil unless family_sharing_enabled? - # Use select to only fetch needed columns and limit to 1 for efficiency - latest_point = points.select(:latitude, :longitude, :timestamp) - .order(timestamp: :desc) - .limit(1) - .first + latest_point = + points.select(:lonlat, :timestamp) + .order(timestamp: :desc) + .limit(1) + .first return nil unless latest_point { user_id: id, email: email, - latitude: latest_point.latitude, - longitude: latest_point.longitude, + latitude: latest_point.lat, + longitude: latest_point.lon, timestamp: latest_point.timestamp, - updated_at: Time.at(latest_point.timestamp) + updated_at: Time.zone.at(latest_point.timestamp) } end diff --git a/app/models/family.rb b/app/models/family.rb index ab12339d..421fce79 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true class Family < ApplicationRecord - has_many :family_memberships, dependent: :destroy + has_many :family_memberships, dependent: :destroy, class_name: 'Family::Membership' has_many :members, through: :family_memberships, source: :user - has_many :family_invitations, dependent: :destroy + has_many :family_invitations, dependent: :destroy, class_name: 'Family::Invitation' belongs_to :creator, class_name: 'User' validates :name, presence: true, length: { maximum: 50 } diff --git a/app/models/family/invitation.rb b/app/models/family/invitation.rb new file mode 100644 index 00000000..68fb4c6d --- /dev/null +++ b/app/models/family/invitation.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +class Family::Invitation < ApplicationRecord + self.table_name = 'family_invitations' + + EXPIRY_DAYS = 7 + + belongs_to :family + belongs_to :invited_by, class_name: 'User' + + validates :email, presence: true, format: { with: URI::MailTo::EMAIL_REGEXP } + validates :token, presence: true, uniqueness: true + validates :expires_at, :status, presence: true + + enum :status, { pending: 0, accepted: 1, expired: 2, cancelled: 3 } + + scope :active, -> { where(status: :pending).where('expires_at > ?', Time.current) } + + before_validation :generate_token, :set_expiry, on: :create + + after_create :clear_family_cache + after_update :clear_family_cache, if: :saved_change_to_status? + after_destroy :clear_family_cache + + def expired? + expires_at.past? + end + + def can_be_accepted? + pending? && !expired? + end + + private + + def generate_token + self.token = SecureRandom.urlsafe_base64(32) if token.blank? + end + + def set_expiry + self.expires_at = EXPIRY_DAYS.days.from_now if expires_at.blank? + end + + def clear_family_cache + family&.clear_member_cache! + end +end diff --git a/app/models/family/membership.rb b/app/models/family/membership.rb new file mode 100644 index 00000000..63ef5e1f --- /dev/null +++ b/app/models/family/membership.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class Family::Membership < ApplicationRecord + self.table_name = 'family_memberships' + + belongs_to :family + belongs_to :user + + validates :user_id, presence: true, uniqueness: true + validates :role, presence: true + + enum :role, { owner: 0, member: 1 } + + after_create :clear_family_cache + after_update :clear_family_cache + after_destroy :clear_family_cache + + private + + def clear_family_cache + family&.clear_member_cache! + end +end diff --git a/app/policies/family/invitation_policy.rb b/app/policies/family/invitation_policy.rb new file mode 100644 index 00000000..b65d6ae8 --- /dev/null +++ b/app/policies/family/invitation_policy.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class Family::InvitationPolicy < ApplicationPolicy + def show? + # Public endpoint for invitation acceptance - no authentication required + true + end + + def create? + user.family == record.family && user.family_owner? + end + + def accept? + # Users can accept invitations sent to their email + user.email == record.email + end + + def destroy? + # Only family owners can cancel invitations + user.family == record.family && user.family_owner? + end +end diff --git a/app/policies/family/membership_policy.rb b/app/policies/family/membership_policy.rb new file mode 100644 index 00000000..4a80b125 --- /dev/null +++ b/app/policies/family/membership_policy.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class Family::MembershipPolicy < ApplicationPolicy + def show? + user.family == record.family + end + + def update? + # Users can update their own settings + return true if user == record.user + + # Family owners can update any member's settings + user.family == record.family && user.family_owner? + end + + def destroy? + # Users can remove themselves (handled by family leave logic) + return true if user == record.user + + # Family owners can remove other members + user.family == record.family && user.family_owner? + end +end diff --git a/app/services/families/accept_invitation.rb b/app/services/families/accept_invitation.rb index 8ea37afb..4869d9fe 100644 --- a/app/services/families/accept_invitation.rb +++ b/app/services/families/accept_invitation.rb @@ -65,7 +65,7 @@ module Families end def create_membership - FamilyMembership.create!( + Family::Membership.create!( family: invitation.family, user: user, role: :member diff --git a/app/services/families/create.rb b/app/services/families/create.rb index e736462b..c958dc32 100644 --- a/app/services/families/create.rb +++ b/app/services/families/create.rb @@ -81,7 +81,7 @@ module Families end def create_owner_membership - FamilyMembership.create!( + Family::Membership.create!( family: family, user: user, role: :owner diff --git a/app/services/families/invite.rb b/app/services/families/invite.rb index a8956666..322109cf 100644 --- a/app/services/families/invite.rb +++ b/app/services/families/invite.rb @@ -73,7 +73,7 @@ module Families end def create_invitation - @invitation = FamilyInvitation.create!( + @invitation = Family::Invitation.create!( family: family, email: email, invited_by: invited_by diff --git a/app/views/family/invitations/show.html.erb b/app/views/family/invitations/show.html.erb index 99580a8d..fac38927 100644 --- a/app/views/family/invitations/show.html.erb +++ b/app/views/family/invitations/show.html.erb @@ -121,7 +121,7 @@
<% if user_signed_in? %> - <%= link_to accept_family_invitation_path(@invitation.family, @invitation), + <%= link_to accept_family_invitation_path(token: @invitation.token), method: :post, class: "btn btn-success btn-lg w-full text-lg shadow-lg" do %> ✓ Accept Invitation & Join Family diff --git a/app/views/family_mailer/invitation.html.erb b/app/views/family_mailer/invitation.html.erb index 13d46b11..7a469b53 100644 --- a/app/views/family_mailer/invitation.html.erb +++ b/app/views/family_mailer/invitation.html.erb @@ -42,7 +42,7 @@

Best regards,
- The Dawarich Team + Evgenii from Dawarich

- \ No newline at end of file + diff --git a/app/views/family_mailer/invitation.text.erb b/app/views/family_mailer/invitation.text.erb index 39bcc527..cd9b266c 100644 --- a/app/views/family_mailer/invitation.text.erb +++ b/app/views/family_mailer/invitation.text.erb @@ -19,4 +19,4 @@ If you don't have a Dawarich account yet, you'll be able to create one when you If you didn't expect this invitation, you can safely ignore this email. Best regards, -The Dawarich Team \ No newline at end of file +Evgenii from Dawarich diff --git a/app/views/shared/_navbar.html.erb b/app/views/shared/_navbar.html.erb index d1c05889..94e66489 100644 --- a/app/views/shared/_navbar.html.erb +++ b/app/views/shared/_navbar.html.erb @@ -21,7 +21,7 @@ <% end %> <% else %> - <%= link_to 'Family', new_family_path, class: "#{active_class?(new_family_path)}" %> + <%= link_to 'Familyα'.html_safe, new_family_path, class: "#{active_class?(new_family_path)}" %> <% end %> <% end %> @@ -79,14 +79,14 @@
<%= link_to family_path, class: "mx-1 #{active_class?(family_path)} flex items-center space-x-2" do %> - Family + Familyα
<% end %>
<% else %> - <%= link_to 'Family', new_family_path, class: "mx-1 #{active_class?(new_family_path)}" %> + <%= link_to 'Familyα'.html_safe, new_family_path, class: "mx-1 #{active_class?(new_family_path)}" %> <% end %> <% end %> diff --git a/app/views/stats/_reverse_geocoding_stats.html.erb b/app/views/stats/_reverse_geocoding_stats.html.erb index 11d3768f..7d387ec9 100644 --- a/app/views/stats/_reverse_geocoding_stats.html.erb +++ b/app/views/stats/_reverse_geocoding_stats.html.erb @@ -38,17 +38,18 @@ <%= current_user.total_cities %>
Cities visited
- -
-

Cities visited

-

- <% current_user.cities_visited.each do |city| %> -

<%= city %>

- <% end %> -

-
-
- -
- + +
+

Cities visited

+

+ <% current_user.cities_visited.each do |city| %> +

<%= city %>

+ <% end %> +

+
+
+ +
+ + diff --git a/config/routes.rb b/config/routes.rb index 9e35fa26..d34aa775 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -62,15 +62,12 @@ Rails.application.routes.draw do resource :family, only: %i[show new create edit update destroy] do patch :update_location_sharing, on: :member - resources :invitations, except: %i[edit update], controller: 'family/invitations' do - member do - post :accept - end - end + resources :invitations, except: %i[edit update], controller: 'family/invitations' resources :members, only: %i[destroy], controller: 'family/memberships' end get 'invitations/:token', to: 'family/invitations#show', as: :public_invitation + post 'family/memberships', to: 'family/memberships#create', as: :accept_family_invitation end resources :points, only: %i[index] do diff --git a/spec/factories/family_invitations.rb b/spec/factories/family_invitations.rb index 5d3e7785..41e71969 100644 --- a/spec/factories/family_invitations.rb +++ b/spec/factories/family_invitations.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true FactoryBot.define do - factory :family_invitation do + factory :family_invitation, class: 'Family::Invitation' do association :family association :invited_by, factory: :user sequence(:email) { |n| "invite#{n}@example.com" } diff --git a/spec/factories/family_memberships.rb b/spec/factories/family_memberships.rb index 9979df5a..0796c9af 100644 --- a/spec/factories/family_memberships.rb +++ b/spec/factories/family_memberships.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true FactoryBot.define do - factory :family_membership do + factory :family_membership, class: 'Family::Membership' do association :family association :user role { :member } diff --git a/spec/models/family_invitation_spec.rb b/spec/models/family/invitation_spec.rb similarity index 93% rename from spec/models/family_invitation_spec.rb rename to spec/models/family/invitation_spec.rb index 3792bd72..2abd5db4 100644 --- a/spec/models/family_invitation_spec.rb +++ b/spec/models/family/invitation_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -RSpec.describe FamilyInvitation, type: :model do +RSpec.describe Family::Invitation, type: :model do describe 'associations' do it { is_expected.to belong_to(:family) } it { is_expected.to belong_to(:invited_by).class_name('User') } @@ -44,9 +44,9 @@ RSpec.describe FamilyInvitation, type: :model do describe '.active' do it 'returns only pending and non-expired invitations' do - expect(FamilyInvitation.active).to include(pending_invitation) - expect(FamilyInvitation.active).not_to include(expired_invitation) - expect(FamilyInvitation.active).not_to include(accepted_invitation) + expect(Family::Invitation.active).to include(pending_invitation) + expect(Family::Invitation.active).not_to include(expired_invitation) + expect(Family::Invitation.active).not_to include(accepted_invitation) end end end @@ -63,7 +63,7 @@ RSpec.describe FamilyInvitation, type: :model do it 'sets expiry date' do invitation.save - expect(invitation.expires_at).to be_within(1.minute).of(FamilyInvitation::EXPIRY_DAYS.days.from_now) + expect(invitation.expires_at).to be_within(1.minute).of(Family::Invitation::EXPIRY_DAYS.days.from_now) end it 'does not override existing token' do @@ -136,7 +136,7 @@ RSpec.describe FamilyInvitation, type: :model do describe 'constants' do it 'defines EXPIRY_DAYS' do - expect(FamilyInvitation::EXPIRY_DAYS).to eq(7) + expect(Family::Invitation::EXPIRY_DAYS).to eq(7) end end diff --git a/spec/models/family_membership_spec.rb b/spec/models/family/membership_spec.rb similarity index 97% rename from spec/models/family_membership_spec.rb rename to spec/models/family/membership_spec.rb index 96d76f07..0cc859e7 100644 --- a/spec/models/family_membership_spec.rb +++ b/spec/models/family/membership_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -RSpec.describe FamilyMembership, type: :model do +RSpec.describe Family::Membership, type: :model do describe 'associations' do it { is_expected.to belong_to(:family) } it { is_expected.to belong_to(:user) } diff --git a/spec/models/family_spec.rb b/spec/models/family_spec.rb index a6d6cf12..7f81b898 100644 --- a/spec/models/family_spec.rb +++ b/spec/models/family_spec.rb @@ -107,8 +107,8 @@ RSpec.describe Family, type: :model do it 'destroys associated invitations when family is destroyed' do invitation = create(:family_invitation, family: family, invited_by: user) - expect { family.destroy }.to change(FamilyInvitation, :count).by(-1) - expect(FamilyInvitation.find_by(id: invitation.id)).to be_nil + expect { family.destroy }.to change(Family::Invitation, :count).by(-1) + expect(Family::Invitation.find_by(id: invitation.id)).to be_nil end end @@ -118,8 +118,8 @@ RSpec.describe Family, type: :model do it 'destroys associated memberships when family is destroyed' do membership = create(:family_membership, family: family, user: user, role: :owner) - expect { family.destroy }.to change(FamilyMembership, :count).by(-1) - expect(FamilyMembership.find_by(id: membership.id)).to be_nil + expect { family.destroy }.to change(Family::Membership, :count).by(-1) + expect(Family::Membership.find_by(id: membership.id)).to be_nil end end end diff --git a/spec/models/user_family_spec.rb b/spec/models/user_family_spec.rb index 9afacb1c..0a0d2879 100644 --- a/spec/models/user_family_spec.rb +++ b/spec/models/user_family_spec.rb @@ -12,7 +12,7 @@ RSpec.describe User, 'family methods', type: :model do is_expected.to have_one(:created_family).class_name('Family').with_foreign_key('creator_id').dependent(:destroy) } it { - is_expected.to have_many(:sent_family_invitations).class_name('FamilyInvitation').with_foreign_key('invited_by_id').dependent(:destroy) + is_expected.to have_many(:sent_family_invitations).class_name('Family::Invitation').with_foreign_key('invited_by_id').dependent(:destroy) } end @@ -119,7 +119,7 @@ RSpec.describe User, 'family methods', type: :model do end it 'destroys associated invitations when user is destroyed' do - expect { user.destroy }.to change(FamilyInvitation, :count).by(-1) + expect { user.destroy }.to change(Family::Invitation, :count).by(-1) end end @@ -129,7 +129,7 @@ RSpec.describe User, 'family methods', type: :model do end it 'destroys associated membership when user is destroyed' do - expect { user.destroy }.to change(FamilyMembership, :count).by(-1) + expect { user.destroy }.to change(Family::Membership, :count).by(-1) end end end diff --git a/spec/requests/families_spec.rb b/spec/requests/families_spec.rb index 4bab31bf..5bc4e826 100644 --- a/spec/requests/families_spec.rb +++ b/spec/requests/families_spec.rb @@ -69,7 +69,7 @@ RSpec.describe 'Family', type: :request do it 'creates a family membership for the user' do expect do post '/family', params: valid_attributes - end.to change(FamilyMembership, :count).by(1) + end.to change(Family::Membership, :count).by(1) end it 'redirects to the new family with success message' do diff --git a/spec/requests/family/invitations_spec.rb b/spec/requests/family/invitations_spec.rb index fd9e1c19..b75d501e 100644 --- a/spec/requests/family/invitations_spec.rb +++ b/spec/requests/family/invitations_spec.rb @@ -92,7 +92,7 @@ RSpec.describe 'Family::Invitations', type: :request do it 'creates a new invitation' do expect do post "/family/invitations", params: valid_params - end.to change(FamilyInvitation, :count).by(1) + end.to change(Family::Invitation, :count).by(1) end it 'redirects with success message' do @@ -112,7 +112,7 @@ RSpec.describe 'Family::Invitations', type: :request do invitation # create the existing invitation expect do post "/family/invitations", params: duplicate_params - end.not_to change(FamilyInvitation, :count) + end.not_to change(Family::Invitation, :count) end it 'redirects with error message' do @@ -161,81 +161,6 @@ RSpec.describe 'Family::Invitations', type: :request do end end - describe 'POST /family/invitations/:id/accept' do - let(:invitee) { create(:user) } - let(:invitee_invitation) { create(:family_invitation, family: family, invited_by: user, email: invitee.email) } - - context 'with valid invitation and user' do - before { sign_in invitee } - - it 'accepts the invitation' do - expect do - post "/family/invitations/#{invitee_invitation.token}/accept" - end.to change { invitee.reload.family }.from(nil).to(family) - end - - it 'redirects with success message' do - post "/family/invitations/#{invitee_invitation.token}/accept" - expect(response).to redirect_to(family_path) - follow_redirect! - expect(response.body).to include('Welcome to the family!') - end - - it 'marks invitation as accepted' do - post "/family/invitations/#{invitee_invitation.token}/accept" - invitee_invitation.reload - expect(invitee_invitation.status).to eq('accepted') - end - end - - context 'when user is already in a family' do - let(:other_family) { create(:family) } - - before do - create(:family_membership, user: invitee, family: other_family, role: :member) - sign_in invitee - end - - it 'does not accept the invitation' do - expect do - post "/family/invitations/#{invitee_invitation.token}/accept" - end.not_to(change { invitee.reload.family }) - end - - it 'redirects with error message' do - post "/family/invitations/#{invitee_invitation.token}/accept" - expect(response).to redirect_to(root_path) - expect(flash[:alert]).to include('You must leave your current family before joining a new one') - end - end - - context 'when invitation is expired' do - before do - invitee_invitation.update!(expires_at: 1.day.ago) - sign_in invitee - end - - it 'does not accept the invitation' do - expect do - post "/family/invitations/#{invitee_invitation.token}/accept" - end.not_to(change { invitee.reload.family }) - end - - it 'redirects with error message' do - post "/family/invitations/#{invitee_invitation.token}/accept" - expect(response).to redirect_to(root_path) - expect(flash[:alert]).to include('This invitation is no longer valid or has expired') - end - end - - context 'when not authenticated' do - it 'redirects to login' do - post "/family/invitations/#{invitee_invitation.token}/accept" - expect(response).to redirect_to(new_user_session_path) - end - end - end - describe 'DELETE /family/invitations/:id' do before { sign_in user } @@ -294,7 +219,7 @@ RSpec.describe 'Family::Invitations', type: :request do } expect(response).to redirect_to(family_path) - created_invitation = FamilyInvitation.last + created_invitation = Family::Invitation.last expect(created_invitation.email).to eq(invitee.email) # 2. Invitee views public invitation page @@ -304,7 +229,7 @@ RSpec.describe 'Family::Invitations', type: :request do # 3. Invitee accepts invitation sign_in invitee - post "/family/invitations/#{created_invitation.token}/accept" + post accept_family_invitation_path(token: created_invitation.token) expect(response).to redirect_to(family_path) # 4. Verify invitee is now in family diff --git a/spec/requests/family/memberships_spec.rb b/spec/requests/family/memberships_spec.rb index c276147b..5efde1ba 100644 --- a/spec/requests/family/memberships_spec.rb +++ b/spec/requests/family/memberships_spec.rb @@ -15,12 +15,89 @@ RSpec.describe 'Family::Memberships', type: :request do sign_in user end + describe 'POST /family/memberships' do + let(:invitee) { create(:user) } + let(:invitee_invitation) { create(:family_invitation, family: family, invited_by: user, email: invitee.email) } + + context 'with valid invitation and user' do + before { sign_in invitee } + + it 'accepts the invitation' do + expect do + post accept_family_invitation_path(token: invitee_invitation.token) + end.to change { invitee.reload.family }.from(nil).to(family) + end + + it 'redirects with success message' do + post accept_family_invitation_path(token: invitee_invitation.token) + expect(response).to redirect_to(family_path) + follow_redirect! + expect(response.body).to include('Welcome to the family!') + end + + it 'marks invitation as accepted' do + post accept_family_invitation_path(token: invitee_invitation.token) + invitee_invitation.reload + expect(invitee_invitation.status).to eq('accepted') + end + end + + context 'when user is already in a family' do + let(:other_family) { create(:family) } + + before do + create(:family_membership, user: invitee, family: other_family, role: :member) + sign_in invitee + end + + it 'does not accept the invitation' do + expect do + post accept_family_invitation_path(token: invitee_invitation.token) + end.not_to(change { invitee.reload.family }) + end + + it 'redirects with error message' do + post accept_family_invitation_path(token: invitee_invitation.token) + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to include('You must leave your current family before joining a new one') + end + end + + context 'when invitation is expired' do + before do + invitee_invitation.update!(expires_at: 1.day.ago) + sign_in invitee + end + + it 'does not accept the invitation' do + expect do + post accept_family_invitation_path(token: invitee_invitation.token) + end.not_to(change { invitee.reload.family }) + end + + it 'redirects with error message' do + post accept_family_invitation_path(token: invitee_invitation.token) + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to include('This invitation is no longer valid or has expired') + end + end + + context 'when not authenticated' do + before { sign_out user } + + it 'redirects to login' do + post accept_family_invitation_path(token: invitee_invitation.token) + expect(response).to redirect_to(new_user_session_path) + end + end + end + describe 'DELETE /family/members/:id' do context 'when removing a regular member' do it 'removes the member from the family' do expect do delete "/family/members/#{member_membership.id}" - end.to change(FamilyMembership, :count).by(-1) + end.to change(Family::Membership, :count).by(-1) end it 'redirects with success message' do @@ -41,7 +118,7 @@ RSpec.describe 'Family::Memberships', type: :request do it 'does not remove the owner' do expect do delete "/family/members/#{owner_membership.id}" - end.not_to change(FamilyMembership, :count) + end.not_to change(Family::Membership, :count) end it 'redirects with error message explaining owners must delete family' do @@ -56,7 +133,7 @@ RSpec.describe 'Family::Memberships', type: :request do expect do delete "/family/members/#{owner_membership.id}" - end.not_to change(FamilyMembership, :count) + end.not_to change(Family::Membership, :count) expect(response).to redirect_to(family_path) follow_redirect! @@ -149,7 +226,7 @@ RSpec.describe 'Family::Memberships', type: :request do # Try to remove owner - should be prevented expect do delete "/family/members/#{owner_membership.id}" - end.not_to change(FamilyMembership, :count) + end.not_to change(Family::Membership, :count) expect(response).to redirect_to(family_path) expect(user.reload.family).to eq(family) diff --git a/spec/requests/family_workflows_spec.rb b/spec/requests/family_workflows_spec.rb index b4c1e576..38f64ed9 100644 --- a/spec/requests/family_workflows_spec.rb +++ b/spec/requests/family_workflows_spec.rb @@ -52,7 +52,7 @@ RSpec.describe 'Family Workflows', type: :request do # User2 accepts invitation sign_in user2 - post "/family/invitations/#{invitation.token}/accept" + post accept_family_invitation_path(token: invitation.token) expect(response).to redirect_to(family_path) expect(user2.reload.family).to eq(family) @@ -71,7 +71,7 @@ RSpec.describe 'Family Workflows', type: :request do # Step 5: User3 accepts invitation sign_in user3 - post "/family/invitations/#{invitation2.token}/accept" + post accept_family_invitation_path(token: invitation2.token) expect(user3.reload.family).to eq(family) expect(family.reload.members.count).to eq(3) @@ -108,7 +108,7 @@ RSpec.describe 'Family Workflows', type: :request do # User2 tries to accept expired invitation sign_in user2 - post "/family/invitations/#{invitation.token}/accept" + post accept_family_invitation_path(token: invitation.token) expect(response).to redirect_to(root_path) expect(user2.reload.family).to be_nil @@ -127,12 +127,12 @@ RSpec.describe 'Family Workflows', type: :request do it 'prevents users from joining multiple families' do # User3 accepts invitation to Family 1 sign_in user3 - post "/family/invitations/#{invitation1.token}/accept" + post accept_family_invitation_path(token: invitation1.token) expect(response).to redirect_to(family_path) expect(user3.family).to eq(family1) # User3 tries to accept invitation to Family 2 - post "/family/invitations/#{invitation2.token}/accept" + post accept_family_invitation_path(token: invitation2.token) expect(response).to redirect_to(root_path) expect(flash[:alert]).to include('You must leave your current family') @@ -268,7 +268,7 @@ RSpec.describe 'Family Workflows', type: :request do post "/family/invitations", params: { family_invitation: { email: 'newuser@example.com' } } - end.to change(FamilyInvitation, :count).by(1) + end.to change(Family::Invitation, :count).by(1) invitation = family.family_invitations.find_by(email: 'newuser@example.com') expect(invitation.email).to eq('newuser@example.com') diff --git a/spec/services/families/accept_invitation_spec.rb b/spec/services/families/accept_invitation_spec.rb index 80492e6c..bf947b4a 100644 --- a/spec/services/families/accept_invitation_spec.rb +++ b/spec/services/families/accept_invitation_spec.rb @@ -11,8 +11,8 @@ RSpec.describe Families::AcceptInvitation do describe '#call' do context 'when invitation can be accepted' do it 'creates membership for user' do - expect { service.call }.to change(FamilyMembership, :count).by(1) - membership = invitee.family_membership + expect { service.call }.to change(Family::Membership, :count).by(1) + membership = invitee.reload.family_membership expect(membership.family).to eq(family) expect(membership.role).to eq('member') end @@ -47,7 +47,7 @@ RSpec.describe Families::AcceptInvitation do end it 'does not create membership' do - expect { service.call }.not_to change(FamilyMembership, :count) + expect { service.call }.not_to change(Family::Membership, :count) end it 'sets appropriate error message' do @@ -68,7 +68,7 @@ RSpec.describe Families::AcceptInvitation do end it 'does not create membership' do - expect { service.call }.not_to change(FamilyMembership, :count) + expect { service.call }.not_to change(Family::Membership, :count) end end @@ -80,7 +80,7 @@ RSpec.describe Families::AcceptInvitation do end it 'does not create membership' do - expect { service.call }.not_to change(FamilyMembership, :count) + expect { service.call }.not_to change(Family::Membership, :count) end end @@ -93,7 +93,7 @@ RSpec.describe Families::AcceptInvitation do end it 'does not create membership' do - expect { service.call }.not_to change(FamilyMembership, :count) + expect { service.call }.not_to change(Family::Membership, :count) end end @@ -108,7 +108,7 @@ RSpec.describe Families::AcceptInvitation do end it 'does not create membership' do - expect { service.call }.not_to change(FamilyMembership, :count) + expect { service.call }.not_to change(Family::Membership, :count) end end end diff --git a/spec/services/families/create_spec.rb b/spec/services/families/create_spec.rb index f4329001..216c344b 100644 --- a/spec/services/families/create_spec.rb +++ b/spec/services/families/create_spec.rb @@ -16,7 +16,7 @@ RSpec.describe Families::Create do it 'creates owner membership' do service.call - membership = user.family_membership + membership = user.reload.family_membership expect(membership.role).to eq('owner') expect(membership.family).to eq(service.family) end @@ -38,7 +38,7 @@ RSpec.describe Families::Create do end it 'does not create a membership' do - expect { service.call }.not_to change(FamilyMembership, :count) + expect { service.call }.not_to change(Family::Membership, :count) end it 'sets appropriate error message' do @@ -65,7 +65,7 @@ RSpec.describe Families::Create do end it 'does not create a membership' do - expect { service.call }.not_to change(FamilyMembership, :count) + expect { service.call }.not_to change(Family::Membership, :count) end it 'sets appropriate error message' do diff --git a/spec/services/families/invite_spec.rb b/spec/services/families/invite_spec.rb index 3367efd3..8ea3c747 100644 --- a/spec/services/families/invite_spec.rb +++ b/spec/services/families/invite_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Families::Invite do describe '#call' do context 'when invitation is valid' do it 'creates an invitation' do - expect { service.call }.to change(FamilyInvitation, :count).by(1) + expect { service.call }.to change(Family::Invitation, :count).by(1) invitation = owner.sent_family_invitations.last @@ -51,7 +51,7 @@ RSpec.describe Families::Invite do end it 'does not create invitation' do - expect { service.call }.not_to change(FamilyInvitation, :count) + expect { service.call }.not_to change(Family::Invitation, :count) end end @@ -66,7 +66,7 @@ RSpec.describe Families::Invite do end it 'does not create invitation' do - expect { service.call }.not_to change(FamilyInvitation, :count) + expect { service.call }.not_to change(Family::Invitation, :count) end end @@ -83,7 +83,7 @@ RSpec.describe Families::Invite do end it 'does not create invitation' do - expect { service.call }.not_to change(FamilyInvitation, :count) + expect { service.call }.not_to change(Family::Invitation, :count) end end @@ -97,7 +97,7 @@ RSpec.describe Families::Invite do end it 'does not create another invitation' do - expect { service.call }.not_to change(FamilyInvitation, :count) + expect { service.call }.not_to change(Family::Invitation, :count) end end diff --git a/spec/services/families/memberships/destroy_spec.rb b/spec/services/families/memberships/destroy_spec.rb index e1227d7a..ac2475e5 100644 --- a/spec/services/families/memberships/destroy_spec.rb +++ b/spec/services/families/memberships/destroy_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Families::Memberships::Destroy do it 'removes the membership' do result = service.call expect(result).to be_truthy, "Expected service to succeed but got error: #{service.error_message}" - expect(FamilyMembership.count).to eq(1) # Only owner should remain + expect(Family::Membership.count).to eq(1) # Only owner should remain expect(member.reload.family_membership).to be_nil end @@ -47,7 +47,7 @@ RSpec.describe Families::Memberships::Destroy do let!(:membership) { create(:family_membership, user: user, family: family, role: :owner) } it 'prevents owner from leaving' do - expect { service.call }.not_to change(FamilyMembership, :count) + expect { service.call }.not_to change(Family::Membership, :count) expect(user.reload.family_membership).to be_present end @@ -75,7 +75,7 @@ RSpec.describe Families::Memberships::Destroy do end it 'does not remove membership' do - expect { service.call }.not_to change(FamilyMembership, :count) + expect { service.call }.not_to change(Family::Membership, :count) expect(user.reload.family_membership).to be_present end end