diff --git a/app/controllers/api/v1/families_controller.rb b/app/controllers/api/v1/families_controller.rb index 4ffe2450..0afb9f51 100644 --- a/app/controllers/api/v1/families_controller.rb +++ b/app/controllers/api/v1/families_controller.rb @@ -17,14 +17,14 @@ class Api::V1::FamiliesController < ApiController private def ensure_family_feature_enabled! - unless DawarichSettings.family_feature_enabled? - render json: { error: 'Family feature is not enabled' }, status: :forbidden - end + return if DawarichSettings.family_feature_enabled? + + render json: { error: 'Family feature is not enabled' }, status: :forbidden end def ensure_user_in_family! - unless current_api_user.in_family? - render json: { error: 'User is not part of a family' }, status: :forbidden - end + return if current_api_user.in_family? + + render json: { error: 'User is not part of a family' }, status: :forbidden end end diff --git a/app/controllers/families_controller.rb b/app/controllers/families_controller.rb index 4135f978..59dd9e9f 100644 --- a/app/controllers/families_controller.rb +++ b/app/controllers/families_controller.rb @@ -3,38 +3,36 @@ class FamiliesController < ApplicationController before_action :authenticate_user! before_action :ensure_family_feature_enabled! - before_action :set_family, only: %i[show edit update destroy leave update_location_sharing] - - def index - redirect_to family_path(current_user.family) if current_user.in_family? - end + before_action :set_family, only: %i[show edit update destroy update_location_sharing] def show authorize @family - # Use optimized family methods for better performance @members = @family.members.includes(:family_membership).order(:email) @pending_invitations = @family.active_invitations.order(:created_at) - # Use cached counts to avoid extra queries @member_count = @family.member_count @can_invite = @family.can_add_members? end def new - redirect_to family_path(current_user.family) if current_user.in_family? + redirect_to family_path and return if current_user.in_family? @family = Family.new + authorize @family end def create + @family = Family.new(family_params) + authorize @family + service = Families::Create.new( user: current_user, name: family_params[:name] ) if service.call - redirect_to family_path(service.family), notice: 'Family created successfully!' + redirect_to family_path, notice: 'Family created successfully!' else @family = Family.new(family_params) @@ -63,7 +61,7 @@ class FamiliesController < ApplicationController authorize @family if @family.update(family_params) - redirect_to family_path(@family), notice: 'Family updated successfully!' + redirect_to family_path, notice: 'Family updated successfully!' else render :edit, status: :unprocessable_content end @@ -73,31 +71,14 @@ class FamiliesController < ApplicationController authorize @family if @family.members.count > 1 - redirect_to family_path(@family), alert: 'Cannot delete family with members. Remove all members first.' + redirect_to family_path, alert: 'Cannot delete family with members. Remove all members first.' else @family.destroy - redirect_to families_path, notice: 'Family deleted successfully!' + redirect_to new_family_path, notice: 'Family deleted successfully!' end end - def leave - authorize @family, :leave? - - service = Families::Leave.new(user: current_user) - - if service.call - redirect_to families_path, notice: 'You have left the family' - else - redirect_to family_path(@family), alert: service.error_message || 'Cannot leave family.' - end - rescue Pundit::NotAuthorizedError - # Handle case where owner with members tries to leave - redirect_to family_path(@family), - alert: 'You cannot leave the family while you are the owner and there are other members. Remove all members first or transfer ownership.' - end - def update_location_sharing - # No authorization needed - users can control their own location sharing enabled = ActiveModel::Type::Boolean.new.cast(params[:enabled]) duration = params[:duration] @@ -109,7 +90,6 @@ class FamiliesController < ApplicationController message: build_sharing_message(enabled, duration) } - # Add expiration info if sharing is time-limited if enabled && current_user.family_sharing_expires_at.present? response_data[:expires_at] = current_user.family_sharing_expires_at.iso8601 response_data[:expires_at_formatted] = current_user.family_sharing_expires_at.strftime('%b %d at %I:%M %p') @@ -158,7 +138,7 @@ class FamiliesController < ApplicationController def set_family @family = current_user.family - redirect_to families_path unless @family + redirect_to new_family_path, alert: 'You are not in a family' unless @family end def family_params diff --git a/app/controllers/family/invitations_controller.rb b/app/controllers/family/invitations_controller.rb index 8763374f..0dccb414 100644 --- a/app/controllers/family/invitations_controller.rb +++ b/app/controllers/family/invitations_controller.rb @@ -3,9 +3,10 @@ class Family::InvitationsController < ApplicationController before_action :authenticate_user!, except: %i[show accept] before_action :ensure_family_feature_enabled!, except: %i[show accept] + before_action :set_invitation_by_token, only: %i[show] + before_action :set_invitation_by_id, only: %i[accept] before_action :set_family, except: %i[show accept] - before_action :set_invitation_by_token, only: %i[show accept] - before_action :set_invitation_by_id, only: %i[destroy] + before_action :set_invitation_by_id_and_family, only: %i[destroy] def index authorize @family, :show? @@ -37,9 +38,9 @@ class Family::InvitationsController < ApplicationController ) if service.call - redirect_to family_path(@family), notice: 'Invitation sent successfully!' + redirect_to family_path, notice: 'Invitation sent successfully!' else - redirect_to family_path(@family), alert: service.error_message || 'Failed to send invitation' + redirect_to family_path, alert: service.error_message || 'Failed to send invitation' end end @@ -65,8 +66,7 @@ class Family::InvitationsController < ApplicationController ) if service.call - redirect_to family_path(current_user.reload.family), - notice: 'Welcome to the family!' + redirect_to family_path, notice: 'Welcome to the family!' else redirect_to root_path, alert: service.error_message || 'Unable to accept invitation' end @@ -80,16 +80,13 @@ class Family::InvitationsController < ApplicationController begin if @invitation.update(status: :cancelled) - redirect_to family_path(@family), - notice: 'Invitation cancelled' + redirect_to family_path, notice: 'Invitation cancelled' else - redirect_to family_path(@family), - alert: 'Failed to cancel invitation. Please try again' + redirect_to family_path, alert: 'Failed to cancel invitation. Please try again' end rescue StandardError => e Rails.logger.error "Error cancelling family invitation: #{e.message}" - redirect_to family_path(@family), - alert: 'An unexpected error occurred while cancelling the invitation' + redirect_to family_path, alert: 'An unexpected error occurred while cancelling the invitation' end end @@ -104,15 +101,25 @@ class Family::InvitationsController < ApplicationController def set_family @family = current_user.family - redirect_to families_path, alert: 'Family not found' and return unless @family + redirect_to new_family_path, alert: 'You are not in a family' and return unless @family end def set_invitation_by_token - @invitation = FamilyInvitation.find_by!(token: params[:id]) + # For public unauthenticated route: /invitations/:token + @invitation = FamilyInvitation.find_by!(token: params[:token]) end def set_invitation_by_id - @invitation = @family.family_invitations.find(params[:id]) + # For authenticated nested routes without family validation: /families/:family_id/invitations/:id/accept + # The :id param contains the token value + @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 + @family = current_user.family + @invitation = @family.family_invitations.find_by!(token: params[:id]) end def invitation_params diff --git a/app/controllers/family/memberships_controller.rb b/app/controllers/family/memberships_controller.rb index 90e8ea59..3577ee91 100644 --- a/app/controllers/family/memberships_controller.rb +++ b/app/controllers/family/memberships_controller.rb @@ -9,13 +9,19 @@ class Family::MembershipsController < ApplicationController def destroy authorize @membership - if @membership.owner? - redirect_to family_path(@family), - alert: 'Family owners cannot remove their own membership. To leave the family, delete it instead.' + member_user = @membership.user + service = Families::Memberships::Destroy.new(user: current_user, member_to_remove: member_user) + + if service.call + if member_user == current_user + # User removed themselves + redirect_to new_family_path, notice: 'You have left the family' + else + # Owner removed another member + redirect_to family_path, notice: "#{member_user.email} has been removed from the family" + end else - member_email = @membership.user.email - @membership.destroy! - redirect_to family_path(@family), notice: "#{member_email} has been removed from the family" + redirect_to family_path, alert: service.error_message || 'Failed to remove member' end end @@ -30,7 +36,7 @@ class Family::MembershipsController < ApplicationController def set_family @family = current_user.family - redirect_to families_path, alert: 'Family not found' and return unless @family + redirect_to new_family_path, alert: 'You are not in a family' and return unless @family end def set_membership diff --git a/app/controllers/map_controller.rb b/app/controllers/map_controller.rb index b45273f4..cf4540c4 100644 --- a/app/controllers/map_controller.rb +++ b/app/controllers/map_controller.rb @@ -13,7 +13,6 @@ class MapController < ApplicationController @years = years_range @points_number = points_count @features = DawarichSettings.features - @family_member_locations = family_member_locations end private @@ -94,8 +93,4 @@ class MapController < ApplicationController def points_from_user current_user.points.without_raw_data.order(timestamp: :asc) end - - def family_member_locations - Families::Locations.new(current_user).call - end end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index 94c1782a..c3926413 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -2,17 +2,15 @@ class Users::RegistrationsController < Devise::RegistrationsController before_action :check_registration_allowed, only: [:new, :create] - before_action :load_invitation_context, only: [:new, :create] + before_action :set_invitation, only: [:new, :create] def new build_resource({}) - # Pre-fill email if invitation exists - if @invitation - resource.email = @invitation.email - end + resource.email = @invitation.email if @invitation yield resource if block_given? + respond_with resource end @@ -45,22 +43,18 @@ class Users::RegistrationsController < Devise::RegistrationsController private def check_registration_allowed - return true unless self_hosted_mode? + return true if DawarichSettings.self_hosted? return true if valid_invitation_token? redirect_to root_path, alert: 'Registration is not available. Please contact your administrator for access.' end - def load_invitation_context + def set_invitation return unless invitation_token.present? @invitation = FamilyInvitation.find_by(token: invitation_token) end - def self_hosted_mode? - ENV['SELF_HOSTED'] == 'true' - end - def valid_invitation_token? return false unless invitation_token.present? @@ -77,7 +71,6 @@ class Users::RegistrationsController < Devise::RegistrationsController def accept_invitation_for_user(user) return unless @invitation&.can_be_accepted? - # Use the existing invitation acceptance service service = Families::AcceptInvitation.new( invitation: @invitation, user: user @@ -96,4 +89,4 @@ class Users::RegistrationsController < Devise::RegistrationsController def sign_up_params super end -end \ No newline at end of file +end diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 092bd5d3..58e5984a 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -10,9 +10,9 @@ class Users::SessionsController < Devise::SessionsController protected def after_sign_in_path_for(resource) - # If there's an invitation token, redirect to the invitation page if invitation_token.present? invitation = FamilyInvitation.find_by(token: invitation_token) + if invitation&.can_be_accepted? return family_invitation_path(invitation.token) end @@ -32,4 +32,4 @@ class Users::SessionsController < Devise::SessionsController def invitation_token @invitation_token ||= params[:invitation_token] || session[:invitation_token] end -end \ No newline at end of file +end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 211fac44..4e715f28 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,15 +1,6 @@ # frozen_string_literal: true module ApplicationHelper - def classes_for_flash(flash_type) - case flash_type.to_sym - when :error - 'bg-red-100 text-red-700 border-red-300' - else - 'bg-blue-100 text-blue-700 border-blue-300' - end - end - def flash_alert_class(type) case type.to_sym when :notice, :success diff --git a/app/javascript/controllers/family_actions_controller.js b/app/javascript/controllers/family_actions_controller.js deleted file mode 100644 index 2b59dd8c..00000000 --- a/app/javascript/controllers/family_actions_controller.js +++ /dev/null @@ -1,81 +0,0 @@ -import { Controller } from "@hotwired/stimulus" - -export default class extends Controller { - static targets = ["confirmButton", "cancelButton"] - static values = { - action: String, - memberEmail: String, - familyName: String - } - - connect() { - this.setupConfirmationMessages() - } - - setupConfirmationMessages() { - const confirmButtons = this.element.querySelectorAll('[data-confirm]') - - confirmButtons.forEach(button => { - button.addEventListener('click', (event) => { - const action = button.dataset.action - const confirmMessage = this.getConfirmationMessage(action) - - if (!confirm(confirmMessage)) { - event.preventDefault() - return false - } - }) - }) - } - - getConfirmationMessage(action) { - switch(action) { - case 'leave-family': - return `Are you sure you want to leave "${this.familyNameValue}"? You'll need a new invitation to rejoin.` - case 'delete-family': - return `Are you sure you want to delete "${this.familyNameValue}"? This action cannot be undone.` - case 'remove-member': - return `Are you sure you want to remove ${this.memberEmailValue} from the family?` - case 'cancel-invitation': - return `Are you sure you want to cancel the invitation to ${this.memberEmailValue}?` - default: - return 'Are you sure you want to perform this action?' - } - } - - showLoadingState(button, action) { - const originalText = button.innerHTML - button.disabled = true - - const loadingText = this.getLoadingText(action) - button.innerHTML = ` - - ${loadingText} - ` - - // Store original text to restore if needed - button.dataset.originalText = originalText - } - - getLoadingText(action) { - switch(action) { - case 'leave-family': - return 'Leaving family...' - case 'delete-family': - return 'Deleting family...' - case 'remove-member': - return 'Removing member...' - case 'cancel-invitation': - return 'Cancelling invitation...' - default: - return 'Processing...' - } - } - - onConfirmedAction(event) { - const button = event.currentTarget - const action = button.dataset.action - - this.showLoadingState(button, action) - } -} diff --git a/app/javascript/controllers/family_invitation_controller.js b/app/javascript/controllers/family_invitation_controller.js deleted file mode 100644 index 500e5f22..00000000 --- a/app/javascript/controllers/family_invitation_controller.js +++ /dev/null @@ -1,66 +0,0 @@ -import { Controller } from "@hotwired/stimulus" - -export default class extends Controller { - static targets = ["form", "email", "submitButton", "errorMessage"] - static values = { maxMembers: Number, currentMembers: Number } - - connect() { - this.validateForm() - } - - validateForm() { - const email = this.emailTarget.value.trim() - const isValid = this.isValidEmail(email) && this.canInviteMoreMembers() - - this.submitButtonTarget.disabled = !isValid - - if (email && !this.isValidEmail(email)) { - this.showError("Please enter a valid email address") - } else if (!this.canInviteMoreMembers()) { - this.showError(`Family is full (${this.currentMembersValue}/${this.maxMembersValue} members)`) - } else { - this.hideError() - } - } - - onEmailInput() { - this.validateForm() - } - - isValidEmail(email) { - const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ - return emailRegex.test(email) - } - - canInviteMoreMembers() { - return this.currentMembersValue < this.maxMembersValue - } - - showError(message) { - if (this.hasErrorMessageTarget) { - this.errorMessageTarget.textContent = message - this.errorMessageTarget.classList.remove("hidden") - } - } - - hideError() { - if (this.hasErrorMessageTarget) { - this.errorMessageTarget.classList.add("hidden") - } - } - - onSubmit(event) { - if (!this.isValidEmail(this.emailTarget.value.trim()) || !this.canInviteMoreMembers()) { - event.preventDefault() - this.validateForm() - return false - } - - // Show loading state - this.submitButtonTarget.disabled = true - this.submitButtonTarget.innerHTML = ` - - Sending invitation... - ` - } -} \ No newline at end of file diff --git a/app/javascript/controllers/flash_message_controller.js b/app/javascript/controllers/flash_message_controller.js deleted file mode 100644 index ce6b5bac..00000000 --- a/app/javascript/controllers/flash_message_controller.js +++ /dev/null @@ -1,43 +0,0 @@ -import { Controller } from "@hotwired/stimulus" - -export default class extends Controller { - static values = { - type: String, - autoDismiss: Boolean - } - - connect() { - this.element.style.animation = 'slideInFromRight 0.3s ease-out forwards' - - if (this.autoDismissValue) { - this.scheduleDismissal() - } - } - - scheduleDismissal() { - // Auto-dismiss success/notice messages after 5 seconds - this.dismissTimeout = setTimeout(() => { - this.dismiss() - }, 5000) - } - - dismiss() { - if (this.dismissTimeout) { - clearTimeout(this.dismissTimeout) - } - - this.element.style.animation = 'slideOutToRight 0.3s ease-in forwards' - - setTimeout(() => { - if (this.element.parentNode) { - this.element.parentNode.removeChild(this.element) - } - }, 300) - } - - disconnect() { - if (this.dismissTimeout) { - clearTimeout(this.dismissTimeout) - } - } -} \ No newline at end of file diff --git a/app/models/family.rb b/app/models/family.rb index 9c9260f9..2baaef41 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -20,7 +20,6 @@ class Family < ApplicationRecord end def member_count - # Cache the count to avoid repeated queries @member_count ||= members.count end diff --git a/app/services/families/leave.rb b/app/services/families/leave.rb deleted file mode 100644 index 7adcb8c6..00000000 --- a/app/services/families/leave.rb +++ /dev/null @@ -1,111 +0,0 @@ -# frozen_string_literal: true - -module Families - class Leave - attr_reader :user, :error_message - - def initialize(user:) - @user = user - @error_message = nil - end - - def call - return false unless validate_can_leave - - # Store family info before removing membership - @family_name = user.family.name - @family_owner = user.family.owner - - ActiveRecord::Base.transaction do - handle_ownership_transfer if user.family_owner? - remove_membership - send_notifications - end - - true - rescue ActiveRecord::RecordInvalid => e - handle_record_invalid_error(e) - false - rescue StandardError => e - handle_generic_error(e) - false - end - - private - - def validate_can_leave - return false unless validate_in_family - return false unless validate_owner_can_leave - - true - end - - def validate_in_family - return true if user.in_family? - - @error_message = 'You are not currently in a family.' - false - end - - def validate_owner_can_leave - return true unless user.family_owner? && family_has_other_members? - - @error_message = 'You cannot leave the family while you are the owner and there are ' \ - 'other members. Remove all members first or transfer ownership.' - false - end - - def family_has_other_members? - user.family.members.count > 1 - end - - def handle_ownership_transfer - # If this is the last member (owner), delete the family - return unless user.family.members.count == 1 - - user.family.destroy! - - # If owner tries to leave with other members, it should be prevented in validation - end - - def remove_membership - user.family_membership.destroy! - end - - def send_notifications - return unless defined?(Notification) - - # Notify the user who left - Notification.create!( - user: user, - kind: :info, - title: 'Left Family', - content: "You've left the family \"#{@family_name}\"" - ) - - # Notify the family owner - return unless @family_owner&.persisted? - - Notification.create!( - user: @family_owner, - kind: :info, - title: 'Family Member Left', - content: "#{user.email} has left the family \"#{@family_name}\"" - ) - end - - def handle_record_invalid_error(error) - @error_message = if error.record&.errors&.any? - error.record.errors.full_messages.first - else - "Failed to leave family: #{error.message}" - end - end - - def handle_generic_error(error) - Rails.logger.error "Unexpected error in Families::Leave: #{error.message}" - Rails.logger.error error.backtrace.join("\n") - @error_message = 'An unexpected error occurred while leaving the family. Please try again' - end - end -end diff --git a/app/services/families/memberships/destroy.rb b/app/services/families/memberships/destroy.rb new file mode 100644 index 00000000..5439f40e --- /dev/null +++ b/app/services/families/memberships/destroy.rb @@ -0,0 +1,180 @@ +# frozen_string_literal: true + +module Families + module Memberships + class Destroy + attr_reader :user, :member_to_remove, :error_message + + def initialize(user:, member_to_remove: nil) + @user = user # The user performing the action (current_user) + @member_to_remove = member_to_remove || user # The user being removed (defaults to self) + @error_message = nil + end + + def call + return false unless validate_can_leave + + # Store family info before removing membership + @family_name = member_to_remove.family.name + @family_owner = member_to_remove.family.owner + + ActiveRecord::Base.transaction do + handle_ownership_transfer if member_to_remove.family_owner? + remove_membership + send_notifications + end + + true + rescue ActiveRecord::RecordInvalid => e + handle_record_invalid_error(e) + false + rescue StandardError => e + handle_generic_error(e) + false + end + + private + + def validate_can_leave + return false unless validate_in_family + return false unless validate_removal_allowed + + true + end + + def validate_in_family + return true if member_to_remove.in_family? + + @error_message = 'User is not currently in a family.' + false + end + + def validate_removal_allowed + # If removing self (user == member_to_remove) + if removing_self? + return validate_owner_can_leave + end + + # If removing another member, user must be owner and member must be in same family + return false unless validate_remover_is_owner + return false unless validate_same_family + return false unless validate_not_removing_owner + + true + end + + def removing_self? + user == member_to_remove + end + + def validate_owner_can_leave + return true unless member_to_remove.family_owner? + + @error_message = 'Family owners cannot remove their own membership. To leave the family, delete it instead.' + false + end + + def validate_remover_is_owner + return true if user.family_owner? + + @error_message = 'Only family owners can remove other members.' + false + end + + def validate_same_family + return true if user.family == member_to_remove.family + + @error_message = 'Cannot remove members from a different family.' + false + end + + def validate_not_removing_owner + return true unless member_to_remove.family_owner? + + @error_message = 'Cannot remove the family owner. The owner must delete the family or leave on their own.' + false + end + + def family_has_other_members? + member_to_remove.family.members.count > 1 + end + + def handle_ownership_transfer + # If this is the last member (owner), delete the family + return unless member_to_remove.family.members.count == 1 + + member_to_remove.family.destroy! + + # If owner tries to leave with other members, it should be prevented in validation + end + + def remove_membership + member_to_remove.family_membership.destroy! + end + + def send_notifications + return unless defined?(Notification) + + if removing_self? + send_self_removal_notifications + else + send_member_removed_notifications + end + end + + def send_self_removal_notifications + # Notify the user who left + Notification.create!( + user: member_to_remove, + kind: :info, + title: 'Left Family', + content: "You've left the family \"#{@family_name}\"" + ) + + # Notify the family owner + return unless @family_owner&.persisted? + + Notification.create!( + user: @family_owner, + kind: :info, + title: 'Family Member Left', + content: "#{member_to_remove.email} has left the family \"#{@family_name}\"" + ) + end + + def send_member_removed_notifications + # Notify the member who was removed + Notification.create!( + user: member_to_remove, + kind: :info, + title: 'Removed from Family', + content: "You have been removed from the family \"#{@family_name}\" by #{user.email}" + ) + + # Notify the owner who removed the member (if different from the member) + return unless user != member_to_remove + + Notification.create!( + user: user, + kind: :info, + title: 'Member Removed', + content: "#{member_to_remove.email} has been removed from the family \"#{@family_name}\"" + ) + end + + def handle_record_invalid_error(error) + @error_message = if error.record&.errors&.any? + error.record.errors.full_messages.first + else + "Failed to leave family: #{error.message}" + end + end + + def handle_generic_error(error) + Rails.logger.error "Unexpected error in Families::Memberships::Destroy: #{error.message}" + Rails.logger.error error.backtrace.join("\n") + @error_message = 'An unexpected error occurred while removing the membership. Please try again' + end + end + end +end diff --git a/app/views/families/edit.html.erb b/app/views/families/edit.html.erb index e9f6e4ff..89451916 100644 --- a/app/views/families/edit.html.erb +++ b/app/views/families/edit.html.erb @@ -5,7 +5,7 @@

<%= t('families.edit.title', default: 'Edit Family') %>

- <%= link_to family_path(@family), + <%= link_to family_path, class: "btn btn-ghost" do %> <%= t('families.edit.back', default: '← Back to Family') %> <% end %> @@ -77,14 +77,14 @@
<%= form.submit t('families.edit.save_changes', default: 'Save Changes'), class: "btn btn-primary" %> - <%= link_to family_path(@family), + <%= link_to family_path, class: "btn btn-neutral" do %> <%= t('families.edit.cancel', default: 'Cancel') %> <% end %>
<% if policy(@family).destroy? %> - <%= link_to family_path(@family), + <%= link_to family_path, method: :delete, data: { confirm: 'Are you sure you want to delete this family? This action cannot be undone.', turbo_confirm: 'Are you sure you want to delete this family? This action cannot be undone.' }, class: "btn btn-outline btn-error" do %> diff --git a/app/views/families/new.html.erb b/app/views/families/new.html.erb index 50c23c83..3d7c6871 100644 --- a/app/views/families/new.html.erb +++ b/app/views/families/new.html.erb @@ -55,7 +55,7 @@
<%= form.submit t('families.new.create_family', default: 'Create Family'), class: "btn btn-primary" %> - <%= link_to families_path, + <%= link_to root_path, class: "btn btn-ghost" do %> <%= t('families.new.back', default: '← Back') %> <% end %> diff --git a/app/views/families/show.html.erb b/app/views/families/show.html.erb index a47c1e9a..c6af543a 100644 --- a/app/views/families/show.html.erb +++ b/app/views/families/show.html.erb @@ -15,26 +15,26 @@
<% if policy(@family).update? %> - <%= link_to edit_family_path(@family), + <%= link_to edit_family_path, class: "btn btn-outline btn-info" do %> <%= icon 'square-pen', class: "inline-block w-4" %><%= t('families.show.edit', default: 'Edit') %> <% end %> <% end %> - <% if policy(@family).leave? && !current_user.family_owner? %> - <%= link_to leave_family_path(@family), + <% if !current_user.family_owner? && current_user.family_membership %> + <%= link_to family_member_path(current_user.family_membership), method: :delete, data: { confirm: 'Are you sure you want to leave this family?', turbo_confirm: 'Are you sure you want to leave this family?' }, - class: "btn btn-outline btn-warning" do %> + class: "btn btn-outline btm-sm btn-warning" do %> Leave Family <% end %> <% end %> <% if policy(@family).destroy? %> - <%= link_to family_path(@family), + <%= link_to family_path, method: :delete, data: { confirm: 'Are you sure you want to delete this family? This action cannot be undone.', turbo_confirm: 'Are you sure you want to delete this family? This action cannot be undone.' }, - class: "btn btn-outline btn-error" do %> + class: "btn btn-outline btm-sm btn-error" do %> <%= icon 'trash-2', class: "inline-block w-4" %> Delete <% end %> @@ -187,7 +187,7 @@
<% if policy(@family).manage_invitations? %> - <%= link_to family_invitation_path(@family, invitation), + <%= link_to family_invitation_path(invitation.token), method: :delete, data: { confirm: 'Are you sure you want to cancel this invitation?', turbo_confirm: 'Are you sure you want to cancel this invitation?' }, class: "btn btn-outline btn-warning btn-sm opacity-70" do %> diff --git a/app/views/family/invitations/index.html.erb b/app/views/family/invitations/index.html.erb index 5d53eac3..1831ae15 100644 --- a/app/views/family/invitations/index.html.erb +++ b/app/views/family/invitations/index.html.erb @@ -5,7 +5,7 @@

<%= t('family_invitations.index.title', default: 'Family Invitations') %>

- <%= link_to family_path(@family), + <%= link_to family_path, class: "btn btn-neutral" do %> <%= t('family_invitations.index.back_to_family', default: 'Back to Family') %> <% end %> @@ -34,7 +34,7 @@ <% end %> <% if policy(@family).manage_invitations? %> - <%= link_to family_invitation_path(@family, invitation), + <%= link_to family_invitation_path(invitation.token), method: :delete, confirm: t('family_invitations.index.cancel_confirm', default: 'Are you sure you want to cancel this invitation?'), class: "btn btn-ghost btn-sm text-error" do %> diff --git a/app/views/shared/_flash.html.erb b/app/views/shared/_flash.html.erb index 08166411..876e8d5e 100644 --- a/app/views/shared/_flash.html.erb +++ b/app/views/shared/_flash.html.erb @@ -1,12 +1,18 @@
<% flash.each do |key, value| %>
-
<%= value %>
- - diff --git a/app/views/shared/_flash_messages.html.erb b/app/views/shared/_flash_messages.html.erb deleted file mode 100644 index 1f66e1c7..00000000 --- a/app/views/shared/_flash_messages.html.erb +++ /dev/null @@ -1,28 +0,0 @@ -<% if flash.any? %> -
- <% flash.each do |type, message| %> - <% next if message.blank? %> -
-
- <%= flash_icon(type) %> -
-
- <%= message %> -
-
- -
-
- <% end %> -
-<% end %> \ No newline at end of file diff --git a/app/views/shared/_navbar.html.erb b/app/views/shared/_navbar.html.erb index 04cf77cd..d1c05889 100644 --- a/app/views/shared/_navbar.html.erb +++ b/app/views/shared/_navbar.html.erb @@ -13,7 +13,7 @@ <% if current_user.in_family? %>
- <%= link_to family_path(current_user.family), class: "#{active_class?(families_path)} flex items-center space-x-2" do %> + <%= link_to family_path, class: "#{active_class?(family_path)} flex items-center space-x-2" do %> Family
<% else %> - <%= link_to 'Family', families_path, class: "#{active_class?(families_path)}" %> + <%= link_to 'Family', new_family_path, class: "#{active_class?(new_family_path)}" %> <% end %> <% end %> @@ -78,7 +78,7 @@ <% if current_user.in_family? %>
- <%= link_to family_path(current_user.family), class: "mx-1 #{active_class?(families_path)} flex items-center space-x-2" do %> + <%= link_to family_path, class: "mx-1 #{active_class?(family_path)} flex items-center space-x-2" do %> Family
<% else %> - <%= link_to 'Family', families_path, class: "mx-1 #{active_class?(families_path)}" %> + <%= link_to 'Family', new_family_path, class: "mx-1 #{active_class?(new_family_path)}" %> <% end %> <% end %> diff --git a/config/initializers/03_dawarich_settings.rb b/config/initializers/03_dawarich_settings.rb index 5e9e3e8b..89a49267 100644 --- a/config/initializers/03_dawarich_settings.rb +++ b/config/initializers/03_dawarich_settings.rb @@ -40,23 +40,7 @@ class DawarichSettings end def family_feature_enabled? - @family_feature_enabled ||= self_hosted? || family_subscription_active? - end - - def family_subscription_active? - # Will be implemented when cloud subscriptions are added - # For now, return false for cloud instances to keep family feature - # available only for self-hosted instances - false - end - - def family_feature_available_for?(user) - return true if self_hosted? - return false unless user - - # For cloud instances, check if user has family subscription - # This will be implemented when subscription system is added - false + @family_feature_enabled ||= self_hosted? end def features diff --git a/config/routes.rb b/config/routes.rb index 3590bba4..9e35fa26 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -58,23 +58,21 @@ Rails.application.routes.draw do resources :trips # Family management routes (only if feature is enabled) - # if DawarichSettings.family_feature_enabled? - resources :families do - member do - delete :leave - patch :update_location_sharing - end - resources :invitations, except: %i[edit update], controller: 'family/invitations' do - member do - post :accept + if DawarichSettings.family_feature_enabled? + 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 :members, only: %i[destroy], controller: 'family/memberships' end - resources :members, only: %i[destroy], controller: 'family/memberships' + + get 'invitations/:token', to: 'family/invitations#show', as: :public_invitation end - # Public family invitation acceptance (no auth required) - get 'invitations/:id', to: 'family/invitations#show', as: :public_invitation - # end resources :points, only: %i[index] do collection do delete :bulk_destroy diff --git a/docs/FAMILY_FEATURES.md b/docs/FAMILY_FEATURES.md deleted file mode 100644 index f8f744fd..00000000 --- a/docs/FAMILY_FEATURES.md +++ /dev/null @@ -1,385 +0,0 @@ -# Family Features Documentation - -## Overview - -The Family Features system allows users to create and manage family groups for shared location tracking and collaboration. This feature is designed with flexibility for both self-hosted and cloud deployments. - -## Architecture - -### Core Models - -- **Family**: Central entity representing a family group -- **FamilyMembership**: Join table linking users to families with roles -- **FamilyInvitation**: Manages invitation flow for new family members - -### Database Schema - -```sql --- families table -CREATE TABLE families ( - id BIGSERIAL PRIMARY KEY, - name VARCHAR(255) NOT NULL, - creator_id BIGINT NOT NULL REFERENCES users(id), - created_at TIMESTAMP NOT NULL, - updated_at TIMESTAMP NOT NULL -); - --- family_memberships table -CREATE TABLE family_memberships ( - id BIGSERIAL PRIMARY KEY, - family_id BIGINT NOT NULL REFERENCES families(id), - user_id BIGINT NOT NULL REFERENCES users(id), - role INTEGER NOT NULL DEFAULT 0, - created_at TIMESTAMP NOT NULL, - updated_at TIMESTAMP NOT NULL -); - --- family_invitations table -CREATE TABLE family_invitations ( - id BIGSERIAL PRIMARY KEY, - family_id BIGINT NOT NULL REFERENCES families(id), - email VARCHAR(255) NOT NULL, - invited_by_id BIGINT NOT NULL REFERENCES users(id), - status INTEGER NOT NULL DEFAULT 0, - expires_at TIMESTAMP NOT NULL, - created_at TIMESTAMP NOT NULL, - updated_at TIMESTAMP NOT NULL -); -``` - -### Performance Optimizations - -The system includes several performance optimizations: - -- **Database Indexes**: Optimized indexes for common queries -- **Caching**: Model-level caching for frequently accessed data -- **Background Jobs**: Asynchronous email processing -- **Query Optimization**: Includes and preloading for N+1 prevention - -## Feature Gating - -### Configuration - -Family features can be enabled/disabled through `DawarichSettings`: - -```ruby -# Check if family feature is enabled -DawarichSettings.family_feature_enabled? - -# Check if feature is available for specific user -DawarichSettings.family_feature_available_for?(user) -``` - -### Deployment Types - -- **Self-hosted**: Family features are enabled by default -- **Cloud hosted**: Features require subscription validation -- **Disabled**: All family routes and UI elements are hidden - -## API Endpoints - -### REST API - -``` -GET /families # List/redirect to user's family -GET /families/:id # Show family details -POST /families # Create new family -PATCH /families/:id # Update family -DELETE /families/:id # Delete family -DELETE /families/:id/leave # Leave family - -# Family Invitations -GET /families/:family_id/invitations # List invitations -POST /families/:family_id/invitations # Send invitation -GET /families/:family_id/invitations/:id # Show invitation -DELETE /families/:family_id/invitations/:id # Cancel invitation - -# Family Members -GET /families/:family_id/members # List members -GET /families/:family_id/members/:id # Show member -DELETE /families/:family_id/members/:id # Remove member - -# Public Invitation Acceptance -GET /family_invitations/:token # Show invitation -POST /family_invitations/:token/accept # Accept invitation -POST /family_invitations/:token/decline # Decline invitation -``` - -### API Responses - -All endpoints return consistent JSON responses: - -```json -{ - "success": true, - "data": { ... }, - "errors": [] -} -``` - -## Security - -### Authorization - -The system uses Pundit policies for authorization: - -- **FamilyPolicy**: Controls family access and modifications -- **FamilyInvitationPolicy**: Manages invitation permissions -- **FamilyMembershipPolicy**: Controls member management - -### Access Control - -- Only family owners can send invitations -- Only family owners can remove members -- Members can only leave families voluntarily -- Invitations expire automatically for security - -### Data Protection - -- Email addresses in invitations are validated -- Invitation tokens are cryptographically secure -- User data is protected through proper authorization - -## Error Handling - -### Service Layer - -All family services implement comprehensive error handling: - -```ruby -class Families::Create - include ActiveModel::Validations - - def call - return false unless valid? - # ... implementation - rescue ActiveRecord::RecordInvalid => e - handle_record_invalid_error(e) - false - rescue StandardError => e - handle_generic_error(e) - false - end - - def error_message - return errors.full_messages.first if errors.any? - return @custom_error_message if @custom_error_message - 'Operation failed' - end -end -``` - -### Error Types - -- **Validation Errors**: Invalid input data -- **Authorization Errors**: Insufficient permissions -- **Business Logic Errors**: Family limits, existing memberships -- **System Errors**: Database, email delivery failures - -## UI Components - -### Interactive Elements - -- **Family Creation Form**: Real-time validation -- **Invitation Management**: Dynamic invite sending -- **Member Management**: Role-based controls -- **Flash Messages**: Animated feedback system - -### Stimulus Controllers - -JavaScript controllers provide enhanced interactivity: - -- `family_invitation_controller.js`: Invitation form validation -- `family_member_controller.js`: Member management actions -- `flash_message_controller.js`: Animated notifications - -## Background Jobs - -### Email Processing - -```ruby -# Invitation emails are sent asynchronously -FamilyMailer.invitation(@invitation).deliver_later( - queue: :mailer, - retry: 3, - wait: 30.seconds -) -``` - -### Cleanup Jobs - -```ruby -# Automatic cleanup of expired invitations -class FamilyInvitationsCleanupJob < ApplicationJob - def perform - # Update expired invitations - # Remove old expired/cancelled invitations - end -end -``` - -## Configuration - -### Environment Variables - -```bash -# Feature toggles -FAMILY_FEATURE_ENABLED=true - -# Email configuration for invitations -SMTP_HOST=smtp.example.com -SMTP_USERNAME=user@example.com -SMTP_PASSWORD=secret - -# Background job configuration -REDIS_URL=redis://localhost:6379/0 -``` - -### Cron Jobs - -```ruby -# config/schedule.rb -every 1.hour do - runner "FamilyInvitationsCleanupJob.perform_later" -end -``` - -## Testing - -### Test Coverage - -The family features include comprehensive test coverage: - -- **Unit Tests**: Service classes, models, helpers -- **Integration Tests**: Controller actions, API endpoints -- **System Tests**: End-to-end user workflows -- **Job Tests**: Background job processing - -### Test Patterns - -```ruby -# Service testing pattern -RSpec.describe Families::Create do - describe '#call' do - context 'with valid parameters' do - it 'creates a family successfully' do - # ... test implementation - end - end - - context 'with invalid parameters' do - it 'returns false and sets error message' do - # ... test implementation - end - end - end -end -``` - -## Deployment - -### Database Migrations - -Run migrations to set up family tables: - -```bash -rails db:migrate -``` - -### Index Creation - -Performance indexes are created concurrently: - -```bash -# Handled automatically in migration -# Uses disable_ddl_transaction! for zero-downtime deployment -``` - -### Background Jobs - -Ensure Sidekiq is running for email processing: - -```bash -bundle exec sidekiq -``` - -### Cron Jobs - -Set up periodic cleanup: - -```bash -# Add to crontab or use whenever gem -0 * * * * cd /app && bundle exec rails runner "FamilyInvitationsCleanupJob.perform_later" -``` - -## Monitoring - -### Metrics - -Key metrics to monitor: - -- Family creation rate -- Invitation acceptance rate -- Email delivery success rate -- Background job processing time - -### Logging - -Important events are logged: - -```ruby -Rails.logger.info "Family created: #{family.id}" -Rails.logger.warn "Failed to send invitation email: #{error.message}" -Rails.logger.error "Unexpected error in family service: #{error.message}" -``` - -## Troubleshooting - -### Common Issues - -1. **Email Delivery Failures** - - Check SMTP configuration - - Verify email credentials - - Monitor Sidekiq queue - -2. **Authorization Errors** - - Verify Pundit policies - - Check user permissions - - Review family membership status - -3. **Performance Issues** - - Monitor database indexes - - Check query optimization - - Review caching implementation - -### Debug Commands - -```bash -# Check family feature status -rails console -> DawarichSettings.family_feature_enabled? - -# Monitor background jobs -bundle exec sidekiq -> Sidekiq::Queue.new('mailer').size - -# Check database indexes -rails dbconsole -> \d family_invitations -``` - -## Future Enhancements - -### Planned Features - -- **Family Statistics**: Shared analytics dashboard -- **Location Sharing**: Real-time family member locations -- **Group Trips**: Collaborative trip planning -- **Enhanced Permissions**: Granular access controls - -### Scalability Considerations - -- **Horizontal Scaling**: Stateless service design -- **Database Sharding**: Family-based data partitioning -- **Caching Strategy**: Redis-based family data caching -- **API Rate Limiting**: Per-family API quotas \ No newline at end of file diff --git a/docs/FAMILY_README.md b/docs/FAMILY_README.md deleted file mode 100644 index 624cec16..00000000 --- a/docs/FAMILY_README.md +++ /dev/null @@ -1,417 +0,0 @@ -# Family Features - -Dawarich includes comprehensive family management features that allow users to create family groups, invite members, and collaborate on location tracking. - -## Quick Start - -### For Self-Hosted Deployments - -Family features are enabled by default for self-hosted installations: - -```bash -# Family features are automatically available -# No additional configuration required -``` - -### For Cloud Deployments - -Family features require subscription validation: - -```bash -# Contact support to enable family features -# Subscription-based access control -``` - -## Features Overview - -### Family Management -- Create and name family groups -- Invite members via email -- Role-based permissions (owner/member) -- Member management and removal - -### Invitation System -- Secure email-based invitations -- Automatic expiration (7 days) -- Token-based acceptance flow -- Cancellation and resending options - -### Security & Privacy -- Authorization via Pundit policies -- Encrypted invitation tokens -- Email validation and verification -- Automatic cleanup of expired data - -### Performance & Scalability -- Optimized database indexes -- Background job processing -- Intelligent caching strategies -- Concurrent database operations - -## Getting Started - -### Creating a Family - -1. Navigate to the Families section -2. Click "Create Family" -3. Enter a family name -4. You become the family owner automatically - -### Inviting Members - -1. Go to your family page -2. Click "Invite Member" -3. Enter the email address -4. The invitation is sent automatically -5. Member receives email with acceptance link - -### Accepting Invitations - -1. Member receives invitation email -2. Clicks the invitation link -3. Must be logged in to Dawarich -4. Accepts or declines the invitation -5. Automatically joins the family if accepted - -## API Documentation - -### REST Endpoints - -```bash -# List families or redirect to user's family -GET /families - -# Show family details (requires authorization) -GET /families/:id - -# Create new family -POST /families -Content-Type: application/json -{ - "family": { - "name": "Smith Family" - } -} - -# Update family name -PATCH /families/:id -Content-Type: application/json -{ - "family": { - "name": "Updated Name" - } -} - -# Delete family (owner only, requires empty family) -DELETE /families/:id - -# Leave family (members only) -DELETE /families/:id/leave - -# Send invitation -POST /families/:family_id/invitations -Content-Type: application/json -{ - "invitation": { - "email": "member@example.com" - } -} - -# Cancel invitation -DELETE /families/:family_id/invitations/:id - -# Accept invitation (public endpoint) -POST /family_invitations/:token/accept - -# Decline invitation (public endpoint) -POST /family_invitations/:token/decline -``` - -### API Responses - -All endpoints return JSON responses: - -```json -{ - "success": true, - "data": { - "family": { - "id": 1, - "name": "Smith Family", - "member_count": 3, - "creator": { - "id": 1, - "email": "owner@example.com" - }, - "members": [...], - "pending_invitations": [...] - } - }, - "errors": [] -} -``` - -## Configuration - -### Environment Variables - -```bash -# Enable/disable family features -FAMILY_FEATURE_ENABLED=true - -# For cloud deployments - require subscription -FAMILY_SUBSCRIPTION_REQUIRED=true - -# Email configuration for invitations -SMTP_HOST=smtp.example.com -SMTP_USERNAME=noreply@example.com -SMTP_PASSWORD=secret_password - -# Background jobs -REDIS_URL=redis://localhost:6379/0 -``` - -### Feature Gating - -Family features can be controlled programmatically: - -```ruby -# Check if features are enabled -DawarichSettings.family_feature_enabled? -# => true/false - -# Check if available for specific user (cloud) -DawarichSettings.family_feature_available_for?(user) -# => true/false based on subscription -``` - -## Database Schema - -### Core Tables - -```sql --- Main family entity -CREATE TABLE families ( - id bigserial PRIMARY KEY, - name varchar(255) NOT NULL, - creator_id bigint NOT NULL REFERENCES users(id), - created_at timestamp NOT NULL, - updated_at timestamp NOT NULL -); - --- User-family relationships with roles -CREATE TABLE family_memberships ( - id bigserial PRIMARY KEY, - family_id bigint NOT NULL REFERENCES families(id), - user_id bigint NOT NULL REFERENCES users(id), - role integer NOT NULL DEFAULT 0, -- 0: member, 1: owner - created_at timestamp NOT NULL, - updated_at timestamp NOT NULL, - UNIQUE(family_id, user_id) -); - --- Invitation management -CREATE TABLE family_invitations ( - id bigserial PRIMARY KEY, - family_id bigint NOT NULL REFERENCES families(id), - email varchar(255) NOT NULL, - invited_by_id bigint NOT NULL REFERENCES users(id), - token varchar(255) NOT NULL UNIQUE, - status integer NOT NULL DEFAULT 0, -- 0: pending, 1: accepted, 2: declined, 3: expired, 4: cancelled - expires_at timestamp NOT NULL, - created_at timestamp NOT NULL, - updated_at timestamp NOT NULL -); -``` - -### Performance Indexes - -```sql --- Optimized for common queries -CREATE INDEX CONCURRENTLY idx_family_invitations_family_status_expires - ON family_invitations (family_id, status, expires_at); - -CREATE INDEX CONCURRENTLY idx_family_memberships_family_role - ON family_memberships (family_id, role); - -CREATE INDEX CONCURRENTLY idx_family_invitations_email - ON family_invitations (email); - -CREATE INDEX CONCURRENTLY idx_family_invitations_status_expires - ON family_invitations (status, expires_at); -``` - -## Testing - -### Running Tests - -```bash -# Run all family-related tests -bundle exec rspec spec/models/family_spec.rb -bundle exec rspec spec/services/families/ -bundle exec rspec spec/controllers/families_controller_spec.rb -bundle exec rspec spec/requests/families_spec.rb - -# Run specific test categories -bundle exec rspec --tag family -bundle exec rspec --tag invitation -``` - -### Test Coverage - -The family features include comprehensive test coverage: - -- **Unit Tests**: Models, services, helpers -- **Integration Tests**: Controllers, API endpoints -- **System Tests**: End-to-end user workflows -- **Job Tests**: Background email processing - -## Deployment - -### Production Deployment - -```bash -# 1. Run database migrations -RAILS_ENV=production bundle exec rails db:migrate - -# 2. Precompile assets (includes family JS/CSS) -RAILS_ENV=production bundle exec rails assets:precompile - -# 3. Start background job workers -bundle exec sidekiq -e production -d - -# 4. Verify deployment -curl -H "Authorization: Bearer $API_TOKEN" \ - https://your-app.com/families -``` - -### Zero-Downtime Deployment - -The family feature supports zero-downtime deployment: - -- Database indexes created with `CONCURRENTLY` -- Backward-compatible migrations -- Feature flags for gradual rollout -- Background job graceful shutdown - -### Monitoring - -Key metrics to monitor: - -```yaml -# Family creation rate -family_creation_rate: rate(families_created_total[5m]) - -# Invitation success rate -invitation_success_rate: - rate(invitations_accepted_total[5m]) / - rate(invitations_sent_total[5m]) - -# Email delivery rate -email_delivery_success_rate: - rate(family_emails_delivered_total[5m]) / - rate(family_emails_sent_total[5m]) - -# API response times -family_api_p95_response_time: - histogram_quantile(0.95, family_api_duration_seconds) -``` - -## Security - -### Authorization Model - -Family features use Pundit policies for authorization: - -```ruby -# Family access control -class FamilyPolicy < ApplicationPolicy - def show? - user_is_member? - end - - def update? - user_is_owner? - end - - def destroy? - user_is_owner? && family.members.count <= 1 - end -end -``` - -### Data Protection - -- All invitation tokens are cryptographically secure -- Email addresses are validated before storage -- Automatic cleanup of expired invitations -- User data protected through proper authorization - -### Security Best Practices - -- Never log invitation tokens -- Validate all email addresses -- Use HTTPS for all invitation links -- Implement rate limiting on invitation sending -- Monitor for suspicious activity patterns - -## Troubleshooting - -### Common Issues - -**1. Email Delivery Failures** -```bash -# Check SMTP configuration -RAILS_ENV=production bundle exec rails console -> ActionMailer::Base.smtp_settings - -# Monitor Sidekiq queue -bundle exec sidekiq -e production -> Sidekiq::Queue.new('mailer').size -``` - -**2. Authorization Errors** -```bash -# Verify user permissions -RAILS_ENV=production bundle exec rails console -> user = User.find(1) -> family = Family.find(1) -> FamilyPolicy.new(user, family).show? -``` - -**3. Performance Issues** -```sql --- Check index usage -SELECT schemaname, tablename, indexname, idx_scan -FROM pg_stat_user_indexes -WHERE tablename LIKE 'family%' -ORDER BY idx_scan DESC; - --- Monitor slow queries -SELECT query, mean_time, calls -FROM pg_stat_statements -WHERE query LIKE '%family%' -ORDER BY mean_time DESC; -``` - -## Support - -### Documentation -- [Family Features Guide](FAMILY_FEATURES.md) -- [Deployment Guide](FAMILY_DEPLOYMENT.md) -- [API Documentation](/api-docs) - -### Community -- [GitHub Issues](https://github.com/Freika/dawarich/issues) -- [Discord Server](https://discord.gg/pHsBjpt5J8) -- [GitHub Discussions](https://github.com/Freika/dawarich/discussions) - -### Contributing - -Contributions to family features are welcome: - -1. Check existing issues for family-related bugs -2. Follow the existing code patterns and conventions -3. Include comprehensive tests for new features -4. Update documentation for any API changes -5. Follow the contribution guidelines in CONTRIBUTING.md \ No newline at end of file diff --git a/spec/requests/families_spec.rb b/spec/requests/families_spec.rb index b7243932..5715e471 100644 --- a/spec/requests/families_spec.rb +++ b/spec/requests/families_spec.rb @@ -34,9 +34,9 @@ RSpec.describe 'Families', type: :request do end end - describe 'GET /families/:id' do + describe 'GET /family' do it 'shows the family page' do - get "/families/#{family.id}" + get "/family" expect(response).to have_http_status(:ok) end @@ -46,8 +46,8 @@ RSpec.describe 'Families', type: :request do before { sign_in outsider } it 'redirects to families index' do - get "/families/#{family.id}" - expect(response).to redirect_to(families_path) + get "/family" + expect(response).to redirect_to(family_path) end end end @@ -119,7 +119,7 @@ RSpec.describe 'Families', type: :request do describe 'GET /families/:id/edit' do it 'shows the edit form' do - get "/families/#{family.id}/edit" + get "/family/edit" expect(response).to have_http_status(:ok) end @@ -127,7 +127,7 @@ RSpec.describe 'Families', type: :request do before { membership.update!(role: :member) } it 'redirects due to authorization failure' do - get "/families/#{family.id}/edit" + get "/family/edit" expect(response).to have_http_status(:see_other) expect(flash[:alert]).to include('not authorized') end @@ -139,7 +139,7 @@ RSpec.describe 'Families', type: :request do context 'with valid attributes' do it 'updates the family' do - patch "/families/#{family.id}", params: new_attributes + patch "/family", params: new_attributes family.reload expect(family.name).to eq('Updated Family Name') expect(response).to redirect_to(family_path(family)) @@ -151,7 +151,7 @@ RSpec.describe 'Families', type: :request do it 'does not update the family' do original_name = family.name - patch "/families/#{family.id}", params: invalid_attributes + patch "/family", params: invalid_attributes family.reload expect(family.name).to eq(original_name) expect(response).to have_http_status(:unprocessable_content) @@ -162,7 +162,7 @@ RSpec.describe 'Families', type: :request do before { membership.update!(role: :member) } it 'redirects due to authorization failure' do - patch "/families/#{family.id}", params: new_attributes + patch "/family", params: new_attributes expect(response).to have_http_status(:see_other) expect(flash[:alert]).to include('not authorized') end @@ -173,9 +173,9 @@ RSpec.describe 'Families', type: :request do context 'when family has only one member' do it 'deletes the family' do expect do - delete "/families/#{family.id}" + delete "/family" end.to change(Family, :count).by(-1) - expect(response).to redirect_to(families_path) + expect(response).to redirect_to(family_path) end end @@ -186,7 +186,7 @@ RSpec.describe 'Families', type: :request do it 'does not delete the family' do expect do - delete "/families/#{family.id}" + delete "/family" end.not_to change(Family, :count) expect(response).to redirect_to(family_path(family)) follow_redirect! @@ -198,49 +198,13 @@ RSpec.describe 'Families', type: :request do before { membership.update!(role: :member) } it 'redirects due to authorization failure' do - delete "/families/#{family.id}" + delete "/family" expect(response).to have_http_status(:see_other) expect(flash[:alert]).to include('not authorized') end end end - describe 'DELETE /families/:id/leave' do - context 'when user is not the owner' do - before { membership.update!(role: :member) } - - it 'allows user to leave the family' do - expect do - delete "/families/#{family.id}/leave" - end.to change { user.reload.family }.from(family).to(nil) - expect(response).to redirect_to(families_path) - end - end - - context 'when user is the owner with other members' do - before do - create(:family_membership, user: other_user, family: family, role: :member) - end - - it 'prevents leaving and shows error message' do - expect do - delete "/families/#{family.id}/leave" - end.not_to(change { user.reload.family }) - expect(response).to redirect_to(family_path(family)) - follow_redirect! - expect(response.body).to include('cannot leave') - end - end - - context 'when user is the only owner' do - it 'allows leaving and deletes the family' do - expect do - delete "/families/#{family.id}/leave" - end.to change(Family, :count).by(-1) - expect(response).to redirect_to(families_path) - end - end - end describe 'authorization for outsiders' do let(:outsider) { create(:user) } @@ -248,29 +212,25 @@ RSpec.describe 'Families', type: :request do before { sign_in outsider } it 'denies access to show when user is not in family' do - get "/families/#{family.id}" - expect(response).to redirect_to(families_path) + get "/family" + expect(response).to redirect_to(family_path) end it 'redirects to families index when user is not in family for edit' do - get "/families/#{family.id}/edit" - expect(response).to redirect_to(families_path) + get "/family/edit" + expect(response).to redirect_to(family_path) end it 'redirects to families index when user is not in family for update' do - patch "/families/#{family.id}", params: { family: { name: 'Hacked' } } - expect(response).to redirect_to(families_path) + patch "/family", params: { family: { name: 'Hacked' } } + expect(response).to redirect_to(family_path) end it 'redirects to families index when user is not in family for destroy' do - delete "/families/#{family.id}" - expect(response).to redirect_to(families_path) + delete "/family" + expect(response).to redirect_to(family_path) end - it 'redirects to families index when user is not in family for leave' do - delete "/families/#{family.id}/leave" - expect(response).to redirect_to(families_path) - end end describe 'authentication required' do @@ -282,7 +242,7 @@ RSpec.describe 'Families', type: :request do end it 'redirects to login for show' do - get "/families/#{family.id}" + get "/family" expect(response).to redirect_to(new_user_session_path) end @@ -297,22 +257,17 @@ RSpec.describe 'Families', type: :request do end it 'redirects to login for edit' do - get "/families/#{family.id}/edit" + get "/family/edit" expect(response).to redirect_to(new_user_session_path) end it 'redirects to login for update' do - patch "/families/#{family.id}", params: { family: { name: 'Test' } } + patch "/family", params: { family: { name: 'Test' } } expect(response).to redirect_to(new_user_session_path) end it 'redirects to login for destroy' do - delete "/families/#{family.id}" - expect(response).to redirect_to(new_user_session_path) - end - - it 'redirects to login for leave' do - delete "/families/#{family.id}/leave" + delete "/family" expect(response).to redirect_to(new_user_session_path) end end diff --git a/spec/requests/family/invitations_spec.rb b/spec/requests/family/invitations_spec.rb index 8f94d50c..6e840c56 100644 --- a/spec/requests/family/invitations_spec.rb +++ b/spec/requests/family/invitations_spec.rb @@ -18,7 +18,7 @@ RSpec.describe 'Family::Invitations', type: :request do it 'shows pending invitations' do invitation # create the invitation - get "/families/#{family.id}/invitations" + get "/family/invitations" expect(response).to have_http_status(:ok) end @@ -28,8 +28,8 @@ RSpec.describe 'Family::Invitations', type: :request do before { sign_in outsider } it 'redirects to families index' do - get "/families/#{family.id}/invitations" - expect(response).to redirect_to(families_path) + get "/family/invitations" + expect(response).to redirect_to(new_family_path) end end @@ -37,7 +37,7 @@ RSpec.describe 'Family::Invitations', type: :request do before { sign_out user } it 'redirects to login' do - get "/families/#{family.id}/invitations" + get "/family/invitations" expect(response).to redirect_to(new_user_session_path) end end @@ -91,13 +91,13 @@ RSpec.describe 'Family::Invitations', type: :request do it 'creates a new invitation' do expect do - post "/families/#{family.id}/invitations", params: valid_params + post "/family/invitations", params: valid_params end.to change(FamilyInvitation, :count).by(1) end it 'redirects with success message' do - post "/families/#{family.id}/invitations", params: valid_params - expect(response).to redirect_to(family_path(family)) + post "/family/invitations", params: valid_params + expect(response).to redirect_to(family_path) follow_redirect! expect(response.body).to include('Invitation sent successfully!') end @@ -111,14 +111,14 @@ RSpec.describe 'Family::Invitations', type: :request do it 'does not create a duplicate invitation' do invitation # create the existing invitation expect do - post "/families/#{family.id}/invitations", params: duplicate_params + post "/family/invitations", params: duplicate_params end.not_to change(FamilyInvitation, :count) end it 'redirects with error message' do invitation # create the existing invitation - post "/families/#{family.id}/invitations", params: duplicate_params - expect(response).to redirect_to(family_path(family)) + post "/family/invitations", params: duplicate_params + expect(response).to redirect_to(family_path) follow_redirect! expect(response.body).to include('Invitation already sent to this email') end @@ -128,7 +128,7 @@ RSpec.describe 'Family::Invitations', type: :request do before { membership.update!(role: :member) } it 'redirects due to authorization failure' do - post "/families/#{family.id}/invitations", params: { + post "/family/invitations", params: { family_invitation: { email: 'test@example.com' } } expect(response).to have_http_status(:see_other) @@ -142,10 +142,10 @@ RSpec.describe 'Family::Invitations', type: :request do before { sign_in outsider } it 'redirects to families index' do - post "/families/#{family.id}/invitations", params: { + post "/family/invitations", params: { family_invitation: { email: 'test@example.com' } } - expect(response).to redirect_to(families_path) + expect(response).to redirect_to(new_family_path) end end @@ -153,7 +153,7 @@ RSpec.describe 'Family::Invitations', type: :request do before { sign_out user } it 'redirects to login' do - post "/families/#{family.id}/invitations", params: { + post "/family/invitations", params: { family_invitation: { email: 'test@example.com' } } expect(response).to redirect_to(new_user_session_path) @@ -170,19 +170,19 @@ RSpec.describe 'Family::Invitations', type: :request do it 'accepts the invitation' do expect do - post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" + 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 "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" - expect(response).to redirect_to(family_path(family)) + 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 "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" + post "/family/invitations/#{invitee_invitation.token}/accept" invitee_invitation.reload expect(invitee_invitation.status).to eq('accepted') end @@ -198,12 +198,12 @@ RSpec.describe 'Family::Invitations', type: :request do it 'does not accept the invitation' do expect do - post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" + post "/family/invitations/#{invitee_invitation.token}/accept" end.not_to(change { invitee.reload.family }) end it 'redirects with error message' do - post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" + 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 @@ -217,12 +217,12 @@ RSpec.describe 'Family::Invitations', type: :request do it 'does not accept the invitation' do expect do - post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" + post "/family/invitations/#{invitee_invitation.token}/accept" end.not_to(change { invitee.reload.family }) end it 'redirects with error message' do - post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" + 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 @@ -230,7 +230,7 @@ RSpec.describe 'Family::Invitations', type: :request do context 'when not authenticated' do it 'redirects to login' do - post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" + post "/family/invitations/#{invitee_invitation.token}/accept" expect(response).to redirect_to(new_user_session_path) end end @@ -240,14 +240,14 @@ RSpec.describe 'Family::Invitations', type: :request do before { sign_in user } it 'cancels the invitation' do - delete "/families/#{family.id}/invitations/#{invitation.id}" + delete "/family/invitations/#{invitation.token}" invitation.reload expect(invitation.status).to eq('cancelled') end it 'redirects with success message' do - delete "/families/#{family.id}/invitations/#{invitation.id}" - expect(response).to redirect_to(family_path(family)) + delete "/family/invitations/#{invitation.token}" + expect(response).to redirect_to(family_path) follow_redirect! expect(response.body).to include('Invitation cancelled') end @@ -256,7 +256,7 @@ RSpec.describe 'Family::Invitations', type: :request do before { membership.update!(role: :member) } it 'redirects due to authorization failure' do - delete "/families/#{family.id}/invitations/#{invitation.id}" + delete "/family/invitations/#{invitation.token}" expect(response).to have_http_status(:see_other) expect(flash[:alert]).to include('not authorized') end @@ -268,8 +268,8 @@ RSpec.describe 'Family::Invitations', type: :request do before { sign_in outsider } it 'redirects to families index' do - delete "/families/#{family.id}/invitations/#{invitation.token}" - expect(response).to redirect_to(families_path) + delete "/family/invitations/#{invitation.token}" + expect(response).to redirect_to(new_family_path) end end @@ -277,7 +277,7 @@ RSpec.describe 'Family::Invitations', type: :request do before { sign_out user } it 'redirects to login' do - delete "/families/#{family.id}/invitations/#{invitation.token}" + delete "/family/invitations/#{invitation.token}" expect(response).to redirect_to(new_user_session_path) end end @@ -289,10 +289,10 @@ RSpec.describe 'Family::Invitations', type: :request do it 'completes full invitation acceptance workflow' do # 1. Owner creates invitation sign_in user - post "/families/#{family.id}/invitations", params: { + post "/family/invitations", params: { family_invitation: { email: invitee.email } } - expect(response).to redirect_to(family_path(family)) + expect(response).to redirect_to(family_path) created_invitation = FamilyInvitation.last expect(created_invitation.email).to eq(invitee.email) @@ -304,8 +304,8 @@ RSpec.describe 'Family::Invitations', type: :request do # 3. Invitee accepts invitation sign_in invitee - post "/families/#{family.id}/invitations/#{created_invitation.token}/accept" - expect(response).to redirect_to(family_path(family)) + post "/family/invitations/#{created_invitation.token}/accept" + expect(response).to redirect_to(family_path) # 4. Verify invitee is now in family expect(invitee.reload.family).to eq(family) diff --git a/spec/requests/family/memberships_spec.rb b/spec/requests/family/memberships_spec.rb index c1bbe6d0..4bee77cc 100644 --- a/spec/requests/family/memberships_spec.rb +++ b/spec/requests/family/memberships_spec.rb @@ -19,20 +19,20 @@ RSpec.describe 'Family::Memberships', type: :request do context 'when removing a regular member' do it 'removes the member from the family' do expect do - delete "/families/#{family.id}/members/#{member_membership.id}" + delete "/family/members/#{member_membership.id}" end.to change(FamilyMembership, :count).by(-1) end it 'redirects with success message' do member_email = member_user.email - delete "/families/#{family.id}/members/#{member_membership.id}" - expect(response).to redirect_to(family_path(family)) + delete "/family/members/#{member_membership.id}" + expect(response).to redirect_to(family_path) follow_redirect! expect(response.body).to include("#{member_email} has been removed from the family") end it 'removes the user from the family' do - delete "/families/#{family.id}/members/#{member_membership.id}" + delete "/family/members/#{member_membership.id}" expect(member_user.reload.family).to be_nil end end @@ -40,13 +40,13 @@ RSpec.describe 'Family::Memberships', type: :request do context 'when trying to remove the owner' do it 'does not remove the owner' do expect do - delete "/families/#{family.id}/members/#{owner_membership.id}" + delete "/family/members/#{owner_membership.id}" end.not_to change(FamilyMembership, :count) end it 'redirects with error message explaining owners must delete family' do - delete "/families/#{family.id}/members/#{owner_membership.id}" - expect(response).to redirect_to(family_path(family)) + delete "/family/members/#{owner_membership.id}" + expect(response).to redirect_to(family_path) follow_redirect! expect(response.body).to include('Family owners cannot remove their own membership. To leave the family, delete it instead.') end @@ -55,10 +55,10 @@ RSpec.describe 'Family::Memberships', type: :request do member_membership.destroy! expect do - delete "/families/#{family.id}/members/#{owner_membership.id}" + delete "/family/members/#{owner_membership.id}" end.not_to change(FamilyMembership, :count) - expect(response).to redirect_to(family_path(family)) + expect(response).to redirect_to(family_path) follow_redirect! expect(response.body).to include('Family owners cannot remove their own membership') end @@ -80,8 +80,8 @@ RSpec.describe 'Family::Memberships', type: :request do before { sign_in outsider } it 'redirects to families index' do - delete "/families/#{family.id}/members/#{member_membership.id}" - expect(response).to redirect_to(families_path) + delete "/family/members/#{member_membership.id}" + expect(response).to redirect_to(new_family_path) end end @@ -89,7 +89,7 @@ RSpec.describe 'Family::Memberships', type: :request do before { sign_out user } it 'redirects to login' do - delete "/families/#{family.id}/members/#{member_membership.id}" + delete "/family/members/#{member_membership.id}" expect(response).to redirect_to(new_user_session_path) end end @@ -100,7 +100,7 @@ RSpec.describe 'Family::Memberships', type: :request do before { sign_in member_user } it 'returns forbidden' do - delete "/families/#{family.id}/members/#{owner_membership.id}" + delete "/family/members/#{owner_membership.id}" expect(response).to have_http_status(:see_other) expect(flash[:alert]).to include('not authorized') end @@ -115,7 +115,7 @@ RSpec.describe 'Family::Memberships', type: :request do expect(member_user.family).to eq(family) # Remove member - delete "/families/#{family.id}/members/#{member_membership.id}" + delete "/family/members/#{member_membership.id}" # Verify removal expect(response).to redirect_to(family_path(family)) @@ -148,7 +148,7 @@ RSpec.describe 'Family::Memberships', type: :request do # Try to remove owner - should be prevented expect do - delete "/families/#{family.id}/members/#{owner_membership.id}" + delete "/family/members/#{owner_membership.id}" end.not_to change(FamilyMembership, :count) expect(response).to redirect_to(family_path(family)) @@ -157,20 +157,14 @@ RSpec.describe 'Family::Memberships', type: :request do end it 'requires owners to use family deletion to leave the family' do - # This test documents that owners must delete the family to leave - # rather than removing their membership - - # Remove other member first member_membership.destroy! - # Try to remove owner membership - should fail - delete "/families/#{family.id}/members/#{owner_membership.id}" - expect(response).to redirect_to(family_path(family)) + delete "/family/members/#{owner_membership.id}" + expect(response).to redirect_to(family_path) expect(flash[:alert]).to include('Family owners cannot remove their own membership') - # Owner must delete the family instead - delete "/families/#{family.id}" - expect(response).to redirect_to(families_path) + delete "/family" + expect(response).to redirect_to(new_family_path) expect(user.reload.family).to be_nil end end diff --git a/spec/requests/family_workflows_spec.rb b/spec/requests/family_workflows_spec.rb index 4832d20f..3acade0c 100644 --- a/spec/requests/family_workflows_spec.rb +++ b/spec/requests/family_workflows_spec.rb @@ -17,13 +17,10 @@ RSpec.describe 'Family Workflows', type: :request do # Step 1: User1 creates a family sign_in user1 - get '/families' + get '/family/new' expect(response).to have_http_status(:ok) - get '/families/new' - expect(response).to have_http_status(:ok) - - post '/families', params: { family: { name: 'The Smith Family' } } + post '/family', params: { family: { name: 'The Smith Family' } } # The redirect should be to the newly created family expect(response).to have_http_status(:found) @@ -35,10 +32,10 @@ RSpec.describe 'Family Workflows', type: :request do expect(user1.family_owner?).to be true # Step 2: User1 invites User2 - post "/families/#{family.id}/invitations", params: { + post "/family/invitations", params: { family_invitation: { email: user2.email } } - expect(response).to redirect_to(family_path(family)) + expect(response).to redirect_to(family_path) invitation = family.family_invitations.find_by(email: user2.email) expect(invitation).to be_present @@ -55,8 +52,8 @@ RSpec.describe 'Family Workflows', type: :request do # User2 accepts invitation sign_in user2 - post "/families/#{family.id}/invitations/#{invitation.token}/accept" - expect(response).to redirect_to(family_path(family)) + post "/family/invitations/#{invitation.token}/accept" + expect(response).to redirect_to(family_path) expect(user2.reload.family).to eq(family) expect(user2.family_owner?).to be false @@ -64,7 +61,7 @@ RSpec.describe 'Family Workflows', type: :request do # Step 4: User1 invites User3 sign_in user1 - post "/families/#{family.id}/invitations", params: { + post "/family/invitations", params: { family_invitation: { email: user3.email } } @@ -74,19 +71,19 @@ RSpec.describe 'Family Workflows', type: :request do # Step 5: User3 accepts invitation sign_in user3 - post "/families/#{family.id}/invitations/#{invitation2.token}/accept" + post "/family/invitations/#{invitation2.token}/accept" expect(user3.reload.family).to eq(family) expect(family.reload.members.count).to eq(3) # Step 6: Family owner views members on family show page sign_in user1 - get "/families/#{family.id}" + get "/family" expect(response).to have_http_status(:ok) # Step 7: Owner removes a member - delete "/families/#{family.id}/members/#{user2.family_membership.id}" - expect(response).to redirect_to(family_path(family)) + delete "/family/members/#{user2.family_membership.id}" + expect(response).to redirect_to(family_path) expect(user2.reload.family).to be_nil expect(family.reload.members.count).to eq(2) @@ -111,7 +108,7 @@ RSpec.describe 'Family Workflows', type: :request do # User2 tries to accept expired invitation sign_in user2 - post "/families/#{family.id}/invitations/#{invitation.token}/accept" + post "/family/invitations/#{invitation.token}/accept" expect(response).to redirect_to(root_path) expect(user2.reload.family).to be_nil @@ -130,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 "/families/#{family1.id}/invitations/#{invitation1.token}/accept" + post "/family/invitations/#{invitation1.token}/accept" expect(response).to redirect_to(family_path(user3.reload.family)) expect(user3.family).to eq(family1) # User3 tries to accept invitation to Family 2 - post "/families/#{family2.id}/invitations/#{invitation2.token}/accept" + post "/family/invitations/#{invitation2.token}/accept" expect(response).to redirect_to(root_path) expect(flash[:alert]).to include('You must leave your current family') @@ -151,11 +148,12 @@ RSpec.describe 'Family Workflows', type: :request do it 'prevents owner from leaving when members exist' do sign_in user1 - # Owner tries to leave family with members - delete "/families/#{family.id}/leave" - expect(response).to redirect_to(family_path(family)) + # Owner tries to leave family with members (using memberships destroy route) + owner_membership = user1.family_membership + delete "/family/members/#{owner_membership.id}" + expect(response).to redirect_to(family_path) follow_redirect! - expect(response.body).to include('cannot leave') + expect(response.body).to include('cannot remove their own membership') expect(user1.reload.family).to eq(family) expect(user1.family_owner?).to be true @@ -165,22 +163,23 @@ RSpec.describe 'Family Workflows', type: :request do sign_in user1 # Remove the member first - delete "/families/#{family.id}/members/#{member_membership.id}" + delete "/family/members/#{member_membership.id}" - # Now owner can leave (which deletes the family) - expect do - delete "/families/#{family.id}/leave" - end.to change(Family, :count).by(-1) + # Owner cannot leave even when alone - they must delete the family instead + owner_membership = user1.reload.family_membership + delete "/family/members/#{owner_membership.id}" + expect(response).to redirect_to(family_path) + follow_redirect! + expect(response.body).to include('cannot remove their own membership') - expect(response).to redirect_to(families_path) - expect(user1.reload.family).to be_nil + expect(user1.reload.family).to eq(family) end it 'allows members to leave freely' do sign_in user2 - delete "/families/#{family.id}/leave" - expect(response).to redirect_to(families_path) + delete "/family/members/#{member_membership.id}" + expect(response).to redirect_to(new_family_path) expect(user2.reload.family).to be_nil expect(family.reload.members.count).to eq(1) @@ -200,10 +199,10 @@ RSpec.describe 'Family Workflows', type: :request do sign_in user1 expect do - delete "/families/#{family.id}" + delete "/family" end.not_to change(Family, :count) - expect(response).to redirect_to(family_path(family)) + expect(response).to redirect_to(family_path) follow_redirect! expect(response.body).to include('Cannot delete family with members') end @@ -213,10 +212,10 @@ RSpec.describe 'Family Workflows', type: :request do sign_in user1 expect do - delete "/families/#{family.id}" + delete "/family" end.to change(Family, :count).by(-1) - expect(response).to redirect_to(families_path) + expect(response).to redirect_to(new_family_path) expect(user1.reload.family).to be_nil end end @@ -229,19 +228,19 @@ RSpec.describe 'Family Workflows', type: :request do it 'enforces proper authorization for family management' do # Member cannot invite others sign_in user2 - post "/families/#{family.id}/invitations", params: { + post "/family/invitations", params: { family_invitation: { email: user3.email } } expect(response).to have_http_status(:see_other) expect(flash[:alert]).to include('not authorized') # Member cannot remove other members - delete "/families/#{family.id}/members/#{owner_membership.id}" + delete "/family/members/#{owner_membership.id}" expect(response).to have_http_status(:see_other) expect(flash[:alert]).to include('not authorized') # Member cannot edit family - patch "/families/#{family.id}", params: { family: { name: 'Hacked Family' } } + patch "/family", params: { family: { name: 'Hacked Family' } } expect(response).to have_http_status(:see_other) expect(flash[:alert]).to include('not authorized') @@ -252,8 +251,8 @@ RSpec.describe 'Family Workflows', type: :request do # Outsider cannot access family sign_in user3 - get "/families/#{family.id}" - expect(response).to redirect_to(families_path) + get "/family" + expect(response).to redirect_to(new_family_path) end end @@ -266,7 +265,7 @@ RSpec.describe 'Family Workflows', type: :request do # Mock email delivery expect do - post "/families/#{family.id}/invitations", params: { + post "/family/invitations", params: { family_invitation: { email: 'newuser@example.com' } } end.to change(FamilyInvitation, :count).by(1) @@ -280,22 +279,22 @@ RSpec.describe 'Family Workflows', type: :request do describe 'Navigation and redirect workflow' do it 'handles proper redirects for family-related navigation' do - # User without family sees index + # User without family can access new family page sign_in user1 - get '/families' + get '/family/new' expect(response).to have_http_status(:ok) # User creates family - post '/families', params: { family: { name: 'Test Family' } } + post '/family', params: { family: { name: 'Test Family' } } expect(response).to have_http_status(:found) - # User with family gets redirected from index to family page - get '/families' - expect(response).to redirect_to(family_path(user1.reload.family)) + # User with family can view their family + get '/family' + expect(response).to have_http_status(:ok) # User with family gets redirected from new family page - get '/families/new' - expect(response).to redirect_to(family_path(user1.reload.family)) + get '/family/new' + expect(response).to redirect_to(family_path) end end end diff --git a/spec/services/families/leave_spec.rb b/spec/services/families/memberships/destroy_spec.rb similarity index 83% rename from spec/services/families/leave_spec.rb rename to spec/services/families/memberships/destroy_spec.rb index 49344785..e1227d7a 100644 --- a/spec/services/families/leave_spec.rb +++ b/spec/services/families/memberships/destroy_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -RSpec.describe Families::Leave do +RSpec.describe Families::Memberships::Destroy do let(:user) { create(:user) } let(:family) { create(:family, creator: user) } let(:service) { described_class.new(user: user) } @@ -46,17 +46,22 @@ RSpec.describe Families::Leave do context 'when user is family owner with no other members' do let!(:membership) { create(:family_membership, user: user, family: family, role: :owner) } - it 'removes the membership' do - expect { service.call }.to change(FamilyMembership, :count).by(-1) - expect(user.reload.family_membership).to be_nil + it 'prevents owner from leaving' do + expect { service.call }.not_to change(FamilyMembership, :count) + expect(user.reload.family_membership).to be_present end - it 'deletes the family' do - expect { service.call }.to change(Family, :count).by(-1) + it 'does not delete the family' do + expect { service.call }.not_to change(Family, :count) end - it 'returns true' do - expect(service.call).to be true + it 'returns false' do + expect(service.call).to be false + end + + it 'sets error message' do + service.call + expect(service.error_message).to include('cannot remove their own membership') end end