Message opt-in enforcement: reject all-disallowed selections with errors; add i18n; add model + request specs

Author: rsmithlal

app/controllers/better_together/conversations_controller.rb

@@ -25,11 +25,28 @@ def new
     end
 
     def create # rubocop:todo Metrics/MethodLength, Metrics/AbcSize
-      @conversation = Conversation.new(conversation_params.merge(creator: helpers.current_person))
+      # Check if user supplied only disallowed participants
+      submitted_any = conversation_params[:participant_ids].present?
+      filtered_params = conversation_params_filtered
+      filtered_empty = Array(filtered_params[:participant_ids]).blank?
+
+      @conversation = Conversation.new(filtered_params.merge(creator: helpers.current_person))
 
       authorize @conversation
 
-      if @conversation.save
+      if submitted_any && filtered_empty
+        @conversation.errors.add(:conversation_participants, t('better_together.conversations.errors.no_permitted_participants'))
+        respond_to do |format|
+          format.turbo_stream do
+            render turbo_stream: turbo_stream.update(
+              'form_errors',
+              partial: 'layouts/better_together/errors',
+              locals: { object: @conversation }
+            )
+          end
+          format.html { render :new }
+        end
+      elsif @conversation.save
         @conversation.participants << helpers.current_person
 
         respond_to do |format|
@@ -53,7 +70,23 @@ def create # rubocop:todo Metrics/MethodLength, Metrics/AbcSize
     def update # rubocop:todo Metrics/AbcSize, Metrics/MethodLength
       authorize @conversation
       ActiveRecord::Base.transaction do # rubocop:todo Metrics/BlockLength
-        if @conversation.update(conversation_params)
+        submitted_any = conversation_params[:participant_ids].present?
+        filtered_params = conversation_params_filtered
+        filtered_empty = Array(filtered_params[:participant_ids]).blank?
+
+        if submitted_any && filtered_empty
+          @conversation.errors.add(:conversation_participants, t('better_together.conversations.errors.no_permitted_participants'))
+          respond_to do |format|
+            format.turbo_stream do
+              render turbo_stream: turbo_stream.update(
+                'form_errors',
+                partial: 'layouts/better_together/errors',
+                locals: { object: @conversation }
+              )
+            end
+            format.html { redirect_to conversations_path, alert: t('better_together.conversations.errors.no_permitted_participants') }
+          end
+        elsif @conversation.update(filtered_params)
           @messages = @conversation.messages.with_all_rich_text.includes(sender: [:string_translations])
                                    .order(:created_at)
           @message = @conversation.messages.build
@@ -155,6 +188,18 @@ def conversation_params
       params.require(:conversation).permit(:title, participant_ids: [])
     end
 
+    # Ensure participant_ids only include people the agent is allowed to message.
+    # If none remain, keep it empty; creator is always added after create.
+    def conversation_params_filtered
+      permitted = ConversationPolicy.new(helpers.current_user, Conversation.new).permitted_participants
+      permitted_ids = permitted.pluck(:id)
+      cp = conversation_params.dup
+      if cp[:participant_ids].present?
+        cp[:participant_ids] = Array(cp[:participant_ids]).map(&:presence).compact & permitted_ids
+      end
+      cp
+    end
+
     def set_conversation
       @conversation = helpers.current_person.conversations.includes(participants: [
                                                                       :string_translations,

config/locales/en.yml

@@ -2082,3 +2082,6 @@ en:
       sort_by_total_views: Sort by Total Views
       to_date: To Date
   'yes': 'Yes'
+    conversations:
+      errors:
+        no_permitted_participants: You can only add platform managers or members who have opted in to receive messages.

config/locales/es.yml

@@ -1191,6 +1191,7 @@ es:
       index:
         new_page: Nueva página
     people:
+      allow_messages_from_members: Permitir mensajes de miembros de la plataforma
       device_permissions:
         camera: Cámara
         location: Ubicación
@@ -1245,7 +1246,7 @@ es:
         platform_manager_error: Los administradores de plataforma no pueden ser bloqueados.
         self_block_error: No puedes bloquearte a ti mismo.
         unblocked: Persona ha sido desbloqueado exitosamente.
-    phone_numbers:
+  phone_numbers:
       add: Agregar Número de Teléfono
       label: Etiqueta
       labels:
@@ -1781,6 +1782,7 @@ es:
       save: Guardar
     hidden: Oculto
     locale: Idioma
+    message: Mensaje
     new: Nuevo
     'no': 'No'
     no_description: Sin descripción
@@ -1824,6 +1826,8 @@ es:
       images:
         cover_image: Imagen de portada
       person:
+        allow_messages_from_members: Optar por recibir mensajes de personas que no sean administradores de la plataforma.
+          than platform managers.
         cover_image: Sube una imagen de portada para mostrar en la parte superior
           del perfil.
         description: Proporcione una breve descripción o biografía.
@@ -2082,3 +2086,6 @@ es:
       sort_by_total_views: Ordenar por vistas totales
       to_date: Hasta fecha
   'yes': Sí
+    conversations:
+      errors:
+        no_permitted_participants: Solo puedes agregar administradores de la plataforma o miembros que hayan optado por recibir mensajes.

config/locales/fr.yml

@@ -1197,6 +1197,7 @@ fr:
       index:
         new_page: Nouvelle page
     people:
+      allow_messages_from_members: Autoriser les messages des membres de la plateforme
       device_permissions:
         camera: Caméra
         location: Localisation
@@ -1252,7 +1253,7 @@ fr:
           bloqués.
         self_block_error: Vous ne pouvez pas vous bloquer vous-même.
         unblocked: La personne a été débloqué avec succès.
-    phone_numbers:
+  phone_numbers:
       add: Ajouter un numéro de téléphone
       label: Libellé
       labels:
@@ -1813,6 +1814,7 @@ fr:
       save: Enregistrer
     hidden: Caché
     locale: Locale
+    message: Message
     new: Nouveau
     'no': Non
     no_description: Pas de description
@@ -1856,6 +1858,8 @@ fr:
       images:
         cover_image: Cover image
       person:
+        allow_messages_from_members: Choisir de recevoir des messages provenant de personnes autres que les gestionnaires de la plateforme.
+          than platform managers.
         cover_image: Téléchargez une image de couverture à afficher en haut du profil.
         description: Fournissez une brève description ou biographie.
         locale: Sélectionnez la langue préférée pour la personne.
@@ -2109,3 +2113,6 @@ fr:
       sort_by_total_views: Trier par vues totales
       to_date: Date de fin
   'yes': Oui
+    conversations:
+      errors:
+        no_permitted_participants: Vous ne pouvez ajouter que des gestionnaires de plateforme ou des membres ayant choisi de recevoir des messages.

spec/models/better_together/person_preferences_spec.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe BetterTogether::Person, type: :model do
+  describe 'message opt-in preference' do
+    it 'defaults to false and can be toggled to true' do
+      person = create(:better_together_person)
+      expect(person.preferences['receive_messages_from_members']).to be(false)
+
+      person.update!(preferences: person.preferences.merge('receive_messages_from_members' => true))
+      person.reload
+      expect(person.preferences['receive_messages_from_members']).to be(true)
+    end
+  end
+end
+

spec/requests/better_together/conversations_request_spec.rb

@@ -50,4 +50,80 @@
       end
     end
   end
+
+  describe 'POST /conversations' do
+    context 'as a regular member' do
+      let!(:regular_user) { create(:user, :confirmed, email: '[email protected]', password: 'password12345') }
+
+      before { login(regular_user.email, 'password12345') }
+
+      it 'filters out non-permitted participant_ids on create' do
+        post better_together.conversations_path(locale: I18n.default_locale), params: {
+          conversation: {
+            title: 'Hello',
+            participant_ids: [opted_in_person.id, non_opted_person.id]
+          }
+        }
+        expect(response).to have_http_status(:found)
+        convo = BetterTogether::Conversation.order(created_at: :desc).first
+        expect(convo.creator).to eq(regular_user.person)
+        ids = convo.participants.pluck(:id)
+        expect(ids).to include(regular_user.person.id) # creator always added
+        expect(ids).to include(opted_in_person.id)     # allowed
+        expect(ids).not_to include(non_opted_person.id) # filtered out
+      end
+
+      it 'shows an error when only non-permitted participants are submitted' do
+        before_count = BetterTogether::Conversation.count
+        post better_together.conversations_path(locale: I18n.default_locale), params: {
+          conversation: {
+            title: 'Hello',
+            participant_ids: [non_opted_person.id]
+          }
+        }
+        expect(response).to have_http_status(:ok)
+        expect(BetterTogether::Conversation.count).to eq(before_count)
+        expect(response.body).to include(I18n.t('better_together.conversations.errors.no_permitted_participants'))
+      end
+    end
+  end
+
+  describe 'PATCH /conversations/:id' do
+    context 'as a regular member' do
+      let!(:regular_user) { create(:user, :confirmed, email: '[email protected]', password: 'password12345') }
+      let!(:conversation) do
+        create('better_together/conversation', creator: regular_user.person).tap do |c|
+          c.participants << regular_user.person unless c.participants.exists?(regular_user.person.id)
+        end
+      end
+
+      before { login(regular_user.email, 'password12345') }
+
+      it 'does not add non-permitted participants on update' do
+        patch better_together.conversation_path(conversation, locale: I18n.default_locale), params: {
+          conversation: {
+            title: conversation.title,
+            participant_ids: [regular_user.person.id, non_opted_person.id]
+          }
+        }
+        expect(response).to have_http_status(:found)
+        conversation.reload
+        ids = conversation.participants.pluck(:id)
+        expect(ids).to include(regular_user.person.id)
+        expect(ids).not_to include(non_opted_person.id)
+      end
+
+      it 'shows an error when update attempts to add only non-permitted participants' do
+        patch better_together.conversation_path(conversation, locale: I18n.default_locale), params: {
+          conversation: {
+            title: conversation.title,
+            participant_ids: [non_opted_person.id]
+          }
+        }
+        expect(response).to have_http_status(:found)
+        follow_redirect!
+        expect(response.body).to include(I18n.t('better_together.conversations.errors.no_permitted_participants'))
+      end
+    end
+  end
 end