diff --git a/app/controllers/better_together/conversations_controller.rb b/app/controllers/better_together/conversations_controller.rb index d6d48d17f..bbcc474e4 100644 --- a/app/controllers/better_together/conversations_controller.rb +++ b/app/controllers/better_together/conversations_controller.rb @@ -25,11 +25,33 @@ 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 } + ), status: :unprocessable_entity + end + format.html do + # Ensure sidebar has data when rendering the new template + set_conversations + render :new, status: :unprocessable_entity + end + end + elsif @conversation.save @conversation.participants << helpers.current_person respond_to do |format| @@ -45,15 +67,44 @@ def create # rubocop:todo Metrics/MethodLength, Metrics/AbcSize locals: { object: @conversation } ) end - format.html { render :new } + format.html do + # Ensure sidebar has data when rendering the new template + set_conversations + render :new + end end end end - def update # rubocop:todo Metrics/AbcSize, Metrics/MethodLength + def update # rubocop:todo Metrics/AbcSize, Metrics/MethodLength, Metrics/PerceivedComplexity 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 } + ), status: :unprocessable_entity + end + format.html do + # Ensure sidebar has data when rendering the show template + set_conversations + # Ensure messages variables are set for the show template + @messages = @conversation.messages.with_all_rich_text + .includes(sender: [:string_translations]).order(:created_at) + @message = @conversation.messages.build + render :show, status: :unprocessable_entity + end + end + elsif @conversation.update(filtered_params) @messages = @conversation.messages.with_all_rich_text.includes(sender: [:string_translations]) .order(:created_at) @message = @conversation.messages.build @@ -147,26 +198,40 @@ def leave_conversation # rubocop:todo Metrics/MethodLength, Metrics/AbcSize private def available_participants - participants = Person.all - - unless helpers.current_person.permitted_to?('manage_platform') - # only allow messaging platform mangers unless you are a platform_manager - participants = participants.where(id: platform_manager_ids) - end - - participants + # Delegate to policy to centralize participant permission logic + ConversationPolicy.new(helpers.current_user, Conversation.new).permitted_participants end def conversation_params params.require(:conversation).permit(:title, participant_ids: []) end - def set_conversation - @conversation = helpers.current_person.conversations.includes(participants: [ - :string_translations, - :contact_detail, - { profile_image_attachment: :blob } - ]).find(params[:id]) + # 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 # rubocop:todo Metrics/AbcSize + permitted = ConversationPolicy.new(helpers.current_user, Conversation.new).permitted_participants + permitted_ids = permitted.pluck(:id) + # Always allow the current person (creator/participant) to appear in the list + permitted_ids << helpers.current_person.id if helpers.current_person + 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 # rubocop:todo Metrics/MethodLength + scope = helpers.current_person.conversations.includes(participants: [ + :string_translations, + :contact_detail, + { profile_image_attachment: :blob } + ]) + @conversation = scope.find(params[:id]) + @set_conversation ||= Conversation.includes(participants: [ + :string_translations, + :contact_detail, + { profile_image_attachment: :blob } + ]).find(params[:id]) end def set_conversations @@ -178,9 +243,6 @@ def set_conversations ]).order(updated_at: :desc).distinct(:id) end - def platform_manager_ids - role = BetterTogether::Role.find_by(identifier: 'platform_manager') - BetterTogether::PersonPlatformMembership.where(role_id: role.id).pluck(:member_id) - end + # platform_manager_ids now inferred by policy; kept here only if needed elsewhere end end diff --git a/app/models/better_together/person.rb b/app/models/better_together/person.rb index bf9ad5603..d2ecdd31e 100644 --- a/app/models/better_together/person.rb +++ b/app/models/better_together/person.rb @@ -71,6 +71,7 @@ def self.primary_community_delegation_attrs store_attributes :preferences do locale String, default: I18n.default_locale.to_s time_zone String, default: ENV.fetch('APP_TIME_ZONE', 'Newfoundland') + receive_messages_from_members Boolean, default: false end store_attributes :notification_preferences do diff --git a/app/policies/better_together/conversation_policy.rb b/app/policies/better_together/conversation_policy.rb index 6f45e1080..af45aa27f 100644 --- a/app/policies/better_together/conversation_policy.rb +++ b/app/policies/better_together/conversation_policy.rb @@ -23,6 +23,20 @@ def leave_conversation? user.present? && record.participants.size > 1 end + # Returns the people that the agent is permitted to message + def permitted_participants + if permitted_to?('manage_platform') + BetterTogether::Person.all + else + role = BetterTogether::Role.find_by(identifier: 'platform_manager') + manager_ids = BetterTogether::PersonPlatformMembership.where(role_id: role.id).pluck(:member_id) + BetterTogether::Person.where(id: manager_ids) + .or(BetterTogether::Person.where('preferences @> ?', + { receive_messages_from_members: true }.to_json)) + .distinct + end + end + class Scope < ApplicationPolicy::Scope end end diff --git a/app/views/better_together/conversations/_form.html.erb b/app/views/better_together/conversations/_form.html.erb index b75d08af5..9192cb121 100644 --- a/app/views/better_together/conversations/_form.html.erb +++ b/app/views/better_together/conversations/_form.html.erb @@ -1,5 +1,7 @@ <%= form_with(model: conversation, data: { turbo: false, controller: 'better_together--form-validation' }) do |form| %> - <%= turbo_frame_tag 'form_errors' %> + <%= turbo_frame_tag 'form_errors' do %> + <%= render 'layouts/better_together/errors', object: conversation %> + <% end %>
<%= form.label :participant_ids, t('.add_participants') %> @@ -15,4 +17,4 @@
<%= form.submit class: 'btn btn-sm btn-primary' %>
- <% end %> \ No newline at end of file + <% end %> diff --git a/app/views/better_together/people/_form.html.erb b/app/views/better_together/people/_form.html.erb index 13c45532a..94a7ddc7e 100644 --- a/app/views/better_together/people/_form.html.erb +++ b/app/views/better_together/people/_form.html.erb @@ -117,15 +117,20 @@ <%= language_select_field(form:, selected_locale: person.locale) %> <%= t('helpers.hint.person.locale') %>
-
-

<%= t('better_together.people.notification_preferences') %>

- <%= render partial: 'better_together/shared/fields/toggle_switch', locals: {form:, attr: :notify_by_email} %> - <%= t('helpers.hint.person.notify_by_email') %> -
-
- <%= render partial: 'better_together/shared/fields/toggle_switch', locals: {form:, attr: :show_conversation_details} %> - <%= t('helpers.hint.person.show_conversation_details') %> -
+
+

<%= t('better_together.people.notification_preferences') %>

+ <%= render partial: 'better_together/shared/fields/toggle_switch', locals: {form:, attr: :notify_by_email} %> + <%= t('helpers.hint.person.notify_by_email') %> +
+
+ <%= render partial: 'better_together/shared/fields/toggle_switch', locals: {form:, attr: :show_conversation_details} %> + <%= t('helpers.hint.person.show_conversation_details') %> +
+
+

<%= t('better_together.people.allow_messages_from_members') %>

+ <%= render partial: 'better_together/shared/fields/toggle_switch', locals: {form:, attr: :receive_messages_from_members} %> + <%= t('helpers.hint.person.allow_messages_from_members') %> +
diff --git a/app/views/better_together/people/show.html.erb b/app/views/better_together/people/show.html.erb index ceb3f261b..ef7c607cd 100644 --- a/app/views/better_together/people/show.html.erb +++ b/app/views/better_together/people/show.html.erb @@ -32,6 +32,12 @@ destroy_path: policy(@person).destroy? ? person_path(@person) : nil, destroy_confirm: t('people.confirm_delete'), destroy_aria_label: 'Delete Profile' %> + <% conversation = BetterTogether::Conversation.new %> + <% if policy(conversation).create? && policy(conversation).permitted_participants.include?(@person) %> + <%= link_to new_conversation_path(conversation: { participant_ids: [@person.id] }), class: 'btn btn-outline-primary btn-sm me-2', 'aria-label' => t('globals.message') do %> + <%= t('globals.message') %> + <% end %> + <% end %> diff --git a/bin/ci b/bin/ci index 18e6ea9fe..665b8255e 100755 --- a/bin/ci +++ b/bin/ci @@ -5,12 +5,13 @@ export RAILS_ENV="${RAILS_ENV:-test}" export NODE_ENV="${NODE_ENV:-test}" export DISABLE_SPRING=1 -# Ensure DB is ready (uses DATABASE_URL) -cd spec/dummy +# Prepare the dummy app database (uses DATABASE_URL), but run specs from project root +pushd spec/dummy >/dev/null bundle exec rails db:prepare +popd >/dev/null # Optional: assets if your specs need them -# bundle exec rails assets:precompile || true +# pushd spec/dummy >/dev/null && bundle exec rails assets:precompile || true; popd >/dev/null || true -# Run specs from dummy +# Run specs from project root bundle exec rspec --format documentation diff --git a/config/locales/en.yml b/config/locales/en.yml index 445e227e0..9bc144ddf 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -713,6 +713,9 @@ en: options_tooltip: Conversation Options empty: no_messages: No messages yet. Why not start the conversation? + errors: + no_permitted_participants: You can only add platform managers or members who + have opted in to receive messages. form: add_participants: Add participants create_conversation: Create conversation @@ -1187,6 +1190,7 @@ en: index: new_page: New page people: + allow_messages_from_members: Allow messages from platform members device_permissions: camera: Camera location: Location @@ -1397,6 +1401,10 @@ en: content: Content conversation: :activerecord.models.conversation conversation_participants: Conversation participants + conversations: + errors: + no_permitted_participants: You can only add platform managers or members who + have opted in to receive messages. date: abbr_day_names: - Sun @@ -1786,6 +1794,7 @@ en: save: Save hidden: Hidden locale: Locale + message: Message new: New 'no': 'No' no_description: No description @@ -1828,6 +1837,8 @@ en: images: cover_image: Cover image person: + allow_messages_from_members: Opt-in to receive messages from people other + than platform managers. cover_image: Upload a cover image to display at the top of the profile. description: Provide a brief description or biography. locale: Select the preferred language for the person. diff --git a/config/locales/es.yml b/config/locales/es.yml index 438b8e263..cbaa3dde5 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -716,6 +716,9 @@ es: options_tooltip: Opciones de Conversación empty: no_messages: Aún no hay mensajes. ¿Por qué no iniciar la conversación? + errors: + no_permitted_participants: Solo puedes agregar administradores de la plataforma + o miembros que hayan optado por recibir mensajes. form: add_participants: Agregar participantes create_conversation: Crear conversación @@ -1191,6 +1194,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 @@ -1405,6 +1409,10 @@ es: content: Contenido conversation: :activerecord.models.conversation conversation_participants: Participantes de la conversación + conversations: + errors: + no_permitted_participants: Solo puedes agregar administradores de la plataforma + o miembros que hayan optado por recibir mensajes. date: abbr_day_names: '["Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"]' abbr_month_names: '[nil, "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", @@ -1781,6 +1789,7 @@ es: save: Guardar hidden: Oculto locale: Idioma + message: Mensaje new: Nuevo 'no': 'No' no_description: Sin descripción @@ -1824,6 +1833,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. diff --git a/config/locales/fr.yml b/config/locales/fr.yml index ae36775c8..5d5780679 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -721,6 +721,9 @@ fr: empty: no_messages: Aucun message pour l'instant. Pourquoi ne pas commencer la conversation ? + errors: + no_permitted_participants: Vous ne pouvez ajouter que des gestionnaires de + plateforme ou des membres ayant choisi de recevoir des messages. form: add_participants: Ajouter des participants create_conversation: Créer une conversation @@ -1197,6 +1200,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 @@ -1414,6 +1418,10 @@ fr: content: Contenu conversation: :activerecord.models.conversation conversation_participants: Participants à la conversation + conversations: + errors: + no_permitted_participants: Vous ne pouvez ajouter que des gestionnaires de plateforme + ou des membres ayant choisi de recevoir des messages. date: abbr_day_names: - dim @@ -1813,6 +1821,7 @@ fr: save: Enregistrer hidden: Caché locale: Locale + message: Message new: Nouveau 'no': Non no_description: Pas de description @@ -1856,6 +1865,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. diff --git a/spec/models/better_together/person_preferences_spec.rb b/spec/models/better_together/person_preferences_spec.rb new file mode 100644 index 000000000..8e59e3905 --- /dev/null +++ b/spec/models/better_together/person_preferences_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe BetterTogether::Person do + describe 'message opt-in preference' do + it 'defaults to false and can be toggled to true' do # rubocop:todo RSpec/MultipleExpectations + 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 diff --git a/spec/policies/better_together/conversation_policy_spec.rb b/spec/policies/better_together/conversation_policy_spec.rb new file mode 100644 index 000000000..c3041bd18 --- /dev/null +++ b/spec/policies/better_together/conversation_policy_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe BetterTogether::ConversationPolicy, type: :policy do + include RequestSpecHelper + + let!(:host_platform) { configure_host_platform } # rubocop:todo RSpec/LetSetup + + let!(:manager_user) { create(:user, :confirmed, :platform_manager, password: 'password12345') } + let!(:manager_person) { manager_user.person } + + let!(:opted_in_person) do + create(:better_together_person, preferences: { receive_messages_from_members: true }) + end + + let!(:non_opted_person) { create(:better_together_person) } + + describe '#permitted_participants' do + context 'when agent is a platform manager' do + it 'includes all people' do + policy = described_class.new(manager_user, BetterTogether::Conversation.new) + ids = policy.permitted_participants.pluck(:id) + expect(ids).to include(manager_person.id, opted_in_person.id, non_opted_person.id) + end + end + + context 'when agent is a regular member' do # rubocop:todo RSpec/MultipleMemoizedHelpers + let!(:regular_user) { create(:user, :confirmed, password: 'password12345') } + + # rubocop:todo RSpec/MultipleExpectations + it 'includes platform managers and opted-in members, but not non-opted members' do + # rubocop:enable RSpec/MultipleExpectations + policy = described_class.new(regular_user, BetterTogether::Conversation.new) + people = policy.permitted_participants + expect(people).to include(manager_person, opted_in_person) + expect(people).not_to include(non_opted_person) + end + end + end +end diff --git a/spec/requests/better_together/conversations_request_spec.rb b/spec/requests/better_together/conversations_request_spec.rb new file mode 100644 index 000000000..ee8438ad7 --- /dev/null +++ b/spec/requests/better_together/conversations_request_spec.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'BetterTogether::Conversations', :as_user do + include RequestSpecHelper + + let!(:manager_user) do + create(:user, :confirmed, :platform_manager, email: 'manager1@example.test', password: 'password12345') + end + let!(:opted_in_person) do + create(:better_together_person, preferences: { receive_messages_from_members: true }, name: 'Opted In User') + end + let!(:non_opted_person) { create(:better_together_person, name: 'Non Opted User') } + + describe 'GET /conversations/new' do + context 'as a regular member', :as_user do # rubocop:todo RSpec/ContextWording + # rubocop:todo RSpec/MultipleExpectations + it 'lists platform managers and opted-in members, but excludes non-opted members' do + # rubocop:enable RSpec/MultipleExpectations + get better_together.new_conversation_path(locale: I18n.default_locale) + expect(response).to have_http_status(:ok) + # Includes manager and opted-in person in the select options + expect(response.body).to include(manager_user.person.name) + expect(response.body).to include('Opted In User') + # Excludes non-opted person + expect(response.body).not_to include('Non Opted User') + end + end + + context 'as a platform manager', :as_platform_manager do # rubocop:todo RSpec/ContextWording + it 'lists all people as available participants' do # rubocop:todo RSpec/MultipleExpectations + get better_together.new_conversation_path(locale: I18n.default_locale) + expect(response).to have_http_status(:ok) + expect(response.body).to include(manager_user.person.name) + expect(response.body).to include('Opted In User') + expect(response.body).to include('Non Opted User') + end + end + end + + describe 'POST /conversations' do + context 'as a regular member', :as_user do # rubocop:todo RSpec/ContextWording + # rubocop:todo RSpec/ExampleLength + # rubocop:todo RSpec/MultipleExpectations + it 'creates conversation with permitted participants (opted-in) and excludes non-permitted' do + # rubocop:enable RSpec/MultipleExpectations + 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) + # Do not follow redirect here; just check DB state + convo = BetterTogether::Conversation.order(created_at: :desc).first + user = BetterTogether::User.find_by(email: 'user@example.test') + expect(convo.creator).to eq(user.person) + ids = convo.participants.pluck(:id) + expect(ids).to include(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 + # rubocop:enable RSpec/ExampleLength + + # rubocop:todo RSpec/MultipleExpectations + it 'shows an error when only non-permitted participants are submitted' do # rubocop:todo RSpec/ExampleLength + # rubocop:enable RSpec/MultipleExpectations + 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(:unprocessable_entity) + 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 'GET /conversations/:id' do + context 'as a non-participant', :as_user do # rubocop:todo RSpec/ContextWording + it 'returns not found' do + conversation = create('better_together/conversation', creator: manager_user.person).tap do |c| + c.participants << manager_user.person unless c.participants.exists?(manager_user.person.id) + end + + get better_together.conversation_path(conversation, locale: I18n.default_locale) + expect(response).to have_http_status(:not_found) + end + end + end + + describe 'PATCH /conversations/:id' do + context 'as a regular member', :as_user do # rubocop:todo RSpec/ContextWording + let!(:conversation) do + # Ensure the conversation reflects policy by using the logged-in user's person + user = BetterTogether::User.find_by(email: 'user@example.test') + create('better_together/conversation', creator: user.person).tap do |c| + c.participants << user.person unless c.participants.exists?(user.person.id) + end + end + + # rubocop:todo RSpec/MultipleExpectations + it 'does not add non-permitted participants on update' do # rubocop:todo RSpec/ExampleLength, RSpec/MultipleExpectations + # rubocop:enable RSpec/MultipleExpectations + user = BetterTogether::User.find_by(email: 'user@example.test') + patch better_together.conversation_path(conversation, locale: I18n.default_locale), params: { + conversation: { + title: conversation.title, + participant_ids: [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(user.person.id) + expect(ids).not_to include(non_opted_person.id) + end + + # rubocop:todo RSpec/ExampleLength + # rubocop:todo RSpec/MultipleExpectations + it 'shows an error when update attempts to add only non-permitted participants' do + # rubocop:enable RSpec/MultipleExpectations + 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(:unprocessable_entity) + expect(response.body).to include(I18n.t('better_together.conversations.errors.no_permitted_participants')) + end + # rubocop:enable RSpec/ExampleLength + end + end +end