From 45ef4945a17af304e2f0027059ed1029515b94f6 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Fri, 8 Aug 2025 14:55:48 -0230 Subject: [PATCH 1/8] WIP: Add shortcut button to person profile to facilitate messaging --- app/views/better_together/people/show.html.erb | 5 +++++ config/locales/en.yml | 1 + 2 files changed, 6 insertions(+) diff --git a/app/views/better_together/people/show.html.erb b/app/views/better_together/people/show.html.erb index befee076f..54dd8fefa 100644 --- a/app/views/better_together/people/show.html.erb +++ b/app/views/better_together/people/show.html.erb @@ -36,6 +36,11 @@ <%= t('globals.delete') %> <% end %> <% end %> + <% if current_person && current_person != @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/config/locales/en.yml b/config/locales/en.yml index 432a10a29..ef6b3af78 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1273,6 +1273,7 @@ en: destroy: Destroy draft: Draft edit: Edit + message: Message 'false': 'No' forms: save: Save From 7fcdec6b8773dd7d1c6af08dab7db0ae89253878 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Fri, 8 Aug 2025 15:12:09 -0230 Subject: [PATCH 2/8] WIP: Add toggle switch to allow peopel to opt-in to receive messages from members --- app/models/better_together/person.rb | 1 + .../better_together/conversation_policy.rb | 15 ++++++++++++++- app/views/better_together/people/_form.html.erb | 9 +++++++-- app/views/better_together/people/show.html.erb | 3 ++- config/locales/en.yml | 14 ++++++++------ 5 files changed, 32 insertions(+), 10 deletions(-) diff --git a/app/models/better_together/person.rb b/app/models/better_together/person.rb index 5d735183c..1a1caa7c8 100644 --- a/app/models/better_together/person.rb +++ b/app/models/better_together/person.rb @@ -64,6 +64,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 35ab46a24..4fd25a0df 100644 --- a/app/policies/better_together/conversation_policy.rb +++ b/app/policies/better_together/conversation_policy.rb @@ -15,7 +15,20 @@ def show? user.present? && record.participants.include?(agent) end - class Scope < ApplicationPolicy::Scope + class Scope < ApplicationPolicy::Scope + 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 + end end diff --git a/app/views/better_together/people/_form.html.erb b/app/views/better_together/people/_form.html.erb index 6ff4e790e..8a3e3460a 100644 --- a/app/views/better_together/people/_form.html.erb +++ b/app/views/better_together/people/_form.html.erb @@ -126,8 +126,13 @@

<%= 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') %> -
+ <%= t('helpers.hint.person.notify_by_email') %> + +
+

<%= 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 54dd8fefa..a39af421b 100644 --- a/app/views/better_together/people/show.html.erb +++ b/app/views/better_together/people/show.html.erb @@ -36,7 +36,8 @@ <%= t('globals.delete') %> <% end %> <% end %> - <% if current_person && current_person != @person %> + <% 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 %> diff --git a/config/locales/en.yml b/config/locales/en.yml index ef6b3af78..1b725fffc 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -837,12 +837,13 @@ en: submit: save: Save notification_preferences: Message notification preferences - tabs: - contact_details: Contact Details - details: Details - device-permissions: Device-permissions - images: Images - preferences: Preferences + tabs: + contact_details: Contact Details + details: Details + device-permissions: Device-permissions + images: Images + preferences: Preferences + allow_messages_from_members: "Allow messages from platform members" phone_numbers: add: Add Phone Number label: Label @@ -1320,6 +1321,7 @@ en: locale: Select the preferred language for the person. name: Enter the full name of the person. notify_by_email: Send an email when a conversation has an unread message. + allow_messages_from_members: Opt-in to receive messages from people other than platform managers. profile_image: Upload a profile image for the person. slug: A URL-friendly identifier, typically auto-generated. label: From 68dfe29ad2752d168bebed6a940ec1f5dae43f74 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Fri, 29 Aug 2025 08:47:58 -0230 Subject: [PATCH 3/8] Message opt-in: use policy for available participants; add policy spec; add request spec for participant list --- .../conversations_controller.rb | 15 ++---- .../conversation_policy_spec.rb | 39 ++++++++++++++ .../conversations_request_spec.rb | 53 +++++++++++++++++++ 3 files changed, 95 insertions(+), 12 deletions(-) create mode 100644 spec/policies/better_together/conversation_policy_spec.rb create mode 100644 spec/requests/better_together/conversations_request_spec.rb diff --git a/app/controllers/better_together/conversations_controller.rb b/app/controllers/better_together/conversations_controller.rb index d6d48d17f..c7e59b657 100644 --- a/app/controllers/better_together/conversations_controller.rb +++ b/app/controllers/better_together/conversations_controller.rb @@ -147,14 +147,8 @@ 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 @@ -178,9 +172,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/spec/policies/better_together/conversation_policy_spec.rb b/spec/policies/better_together/conversation_policy_spec.rb new file mode 100644 index 000000000..70b6b1c17 --- /dev/null +++ b/spec/policies/better_together/conversation_policy_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe BetterTogether::ConversationPolicy, type: :policy do + include RequestSpecHelper + + let!(:host_platform) { configure_host_platform } + + 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 + let!(:regular_user) { create(:user, :confirmed, password: 'password12345') } + + it 'includes platform managers and opted-in members, but not non-opted members' do + 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..ba5942e64 --- /dev/null +++ b/spec/requests/better_together/conversations_request_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'BetterTogether::Conversations' do + include RequestSpecHelper + + before do + configure_host_platform + end + + 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' do + let!(:regular_user) { create(:user, :confirmed, email: 'user@example.test', password: 'password12345') } + + before do + login(regular_user.email, 'password12345') + end + + it 'lists platform managers and opted-in members, but excludes non-opted members' do + 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' do + before do + login(manager_user.email, 'password12345') + end + + it 'lists all people as available participants' do + 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 +end From ff82b19d9ec97467589c8898f5502637f2bb14fe Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Fri, 29 Aug 2025 08:53:29 -0230 Subject: [PATCH 4/8] Message opt-in enforcement: reject all-disallowed selections with errors; add i18n; add model + request specs --- .../conversations_controller.rb | 51 ++++++++++++- config/locales/en.yml | 3 + config/locales/es.yml | 9 ++- config/locales/fr.yml | 9 ++- .../person_preferences_spec.rb | 17 +++++ .../conversations_request_spec.rb | 76 +++++++++++++++++++ 6 files changed, 160 insertions(+), 5 deletions(-) create mode 100644 spec/models/better_together/person_preferences_spec.rb diff --git a/app/controllers/better_together/conversations_controller.rb b/app/controllers/better_together/conversations_controller.rb index c7e59b657..e44cce816 100644 --- a/app/controllers/better_together/conversations_controller.rb +++ b/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, diff --git a/config/locales/en.yml b/config/locales/en.yml index 41fbd1715..1a9d4e4c8 100644 --- a/config/locales/en.yml +++ b/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. diff --git a/config/locales/es.yml b/config/locales/es.yml index 438b8e263..0b425155a 100644 --- a/config/locales/es.yml +++ b/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. diff --git a/config/locales/fr.yml b/config/locales/fr.yml index ae36775c8..22e47bac7 100644 --- a/config/locales/fr.yml +++ b/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. 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..3d573ea69 --- /dev/null +++ b/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 + diff --git a/spec/requests/better_together/conversations_request_spec.rb b/spec/requests/better_together/conversations_request_spec.rb index ba5942e64..51cb59906 100644 --- a/spec/requests/better_together/conversations_request_spec.rb +++ b/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: 'user2@example.test', 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: 'user3@example.test', 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 From 5cf54b09c3ed6bcccca8fdd4493bd6be7f879a88 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Fri, 29 Aug 2025 08:55:55 -0230 Subject: [PATCH 5/8] Conversations: render validation errors with 422 on disallowed participants for create/update; update request specs --- .../better_together/conversations_controller.rb | 8 ++++---- .../better_together/conversations_request_spec.rb | 7 +++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/app/controllers/better_together/conversations_controller.rb b/app/controllers/better_together/conversations_controller.rb index e44cce816..17366137f 100644 --- a/app/controllers/better_together/conversations_controller.rb +++ b/app/controllers/better_together/conversations_controller.rb @@ -42,9 +42,9 @@ def create # rubocop:todo Metrics/MethodLength, Metrics/AbcSize 'form_errors', partial: 'layouts/better_together/errors', locals: { object: @conversation } - ) + ), status: :unprocessable_entity end - format.html { render :new } + format.html { render :new, status: :unprocessable_entity } end elsif @conversation.save @conversation.participants << helpers.current_person @@ -82,9 +82,9 @@ def update # rubocop:todo Metrics/AbcSize, Metrics/MethodLength 'form_errors', partial: 'layouts/better_together/errors', locals: { object: @conversation } - ) + ), status: :unprocessable_entity end - format.html { redirect_to conversations_path, alert: t('better_together.conversations.errors.no_permitted_participants') } + format.html { render :show, status: :unprocessable_entity } end elsif @conversation.update(filtered_params) @messages = @conversation.messages.with_all_rich_text.includes(sender: [:string_translations]) diff --git a/spec/requests/better_together/conversations_request_spec.rb b/spec/requests/better_together/conversations_request_spec.rb index 51cb59906..cd068fb27 100644 --- a/spec/requests/better_together/conversations_request_spec.rb +++ b/spec/requests/better_together/conversations_request_spec.rb @@ -57,7 +57,7 @@ before { login(regular_user.email, 'password12345') } - it 'filters out non-permitted participant_ids on create' do + it 'creates conversation with permitted participants (opted-in) and excludes non-permitted' do post better_together.conversations_path(locale: I18n.default_locale), params: { conversation: { title: 'Hello', @@ -81,7 +81,7 @@ participant_ids: [non_opted_person.id] } } - expect(response).to have_http_status(:ok) + 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 @@ -120,8 +120,7 @@ participant_ids: [non_opted_person.id] } } - expect(response).to have_http_status(:found) - follow_redirect! + expect(response).to have_http_status(:unprocessable_entity) expect(response.body).to include(I18n.t('better_together.conversations.errors.no_permitted_participants')) end end From 487ce43b4479bfd02e7a787efc85efd6ab539cb4 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Fri, 29 Aug 2025 15:50:32 -0230 Subject: [PATCH 6/8] Enhance conversation handling: ensure sidebar data is set when rendering templates, improve participant filtering, and update localization for permitted participants. --- .../conversations_controller.rb | 39 ++++++++++++---- .../better_together/conversation_policy.rb | 3 +- .../conversations/_form.html.erb | 6 ++- bin/ci | 9 ++-- config/locales/en.yml | 17 ++++--- config/locales/es.yml | 16 ++++--- config/locales/fr.yml | 16 ++++--- .../conversations_request_spec.rb | 45 +++++++------------ 8 files changed, 89 insertions(+), 62 deletions(-) diff --git a/app/controllers/better_together/conversations_controller.rb b/app/controllers/better_together/conversations_controller.rb index 17366137f..f6e08f551 100644 --- a/app/controllers/better_together/conversations_controller.rb +++ b/app/controllers/better_together/conversations_controller.rb @@ -44,7 +44,11 @@ def create # rubocop:todo Metrics/MethodLength, Metrics/AbcSize locals: { object: @conversation } ), status: :unprocessable_entity end - format.html { render :new, status: :unprocessable_entity } + 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 @@ -62,7 +66,11 @@ 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 @@ -84,7 +92,14 @@ def update # rubocop:todo Metrics/AbcSize, Metrics/MethodLength locals: { object: @conversation } ), status: :unprocessable_entity end - format.html { render :show, status: :unprocessable_entity } + 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]) @@ -193,6 +208,8 @@ def conversation_params def conversation_params_filtered 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 @@ -201,11 +218,17 @@ def conversation_params_filtered end def set_conversation - @conversation = helpers.current_person.conversations.includes(participants: [ - :string_translations, - :contact_detail, - { profile_image_attachment: :blob } - ]).find(params[:id]) + scope = helpers.current_person.conversations.includes(participants: [ + :string_translations, + :contact_detail, + { profile_image_attachment: :blob } + ]) + @conversation = scope.find_by(id: params[:id]) + @conversation ||= Conversation.includes(participants: [ + :string_translations, + :contact_detail, + { profile_image_attachment: :blob } + ]).find(params[:id]) end def set_conversations diff --git a/app/policies/better_together/conversation_policy.rb b/app/policies/better_together/conversation_policy.rb index 4884a4a61..af45aa27f 100644 --- a/app/policies/better_together/conversation_policy.rb +++ b/app/policies/better_together/conversation_policy.rb @@ -31,7 +31,8 @@ def permitted_participants 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)) + .or(BetterTogether::Person.where('preferences @> ?', + { receive_messages_from_members: true }.to_json)) .distinct 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/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 1a9d4e4c8..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 @@ -1204,7 +1208,6 @@ en: device-permissions: Device-permissions images: Images preferences: Preferences - allow_messages_from_members: "Allow messages from platform members" person_block: cannot_block_manager: no puede ser un administrador de la plataforma cannot_block_self: no puedes bloquearte a ti mismo @@ -1398,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 @@ -1782,12 +1789,12 @@ en: draft: Draft edit: Edit email: Email - message: Message 'false': 'No' forms: save: Save hidden: Hidden locale: Locale + message: Message new: New 'no': 'No' no_description: No description @@ -1830,12 +1837,13 @@ 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. name: Enter the full name of the person. notify_by_email: Send an email when a conversation has an unread message. - allow_messages_from_members: Opt-in to receive messages from people other than platform managers. profile_image: Upload a profile image for the person. show_conversation_details: Show conversation title and sender name in notifications. slug: A URL-friendly identifier, typically auto-generated. @@ -2082,6 +2090,3 @@ 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. diff --git a/config/locales/es.yml b/config/locales/es.yml index 0b425155a..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 @@ -1246,7 +1249,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: @@ -1406,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", @@ -1826,8 +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. + 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. @@ -2086,6 +2093,3 @@ 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. diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 22e47bac7..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 @@ -1253,7 +1256,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: @@ -1415,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 @@ -1858,8 +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. + 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. @@ -2113,6 +2120,3 @@ 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. diff --git a/spec/requests/better_together/conversations_request_spec.rb b/spec/requests/better_together/conversations_request_spec.rb index cd068fb27..f17233124 100644 --- a/spec/requests/better_together/conversations_request_spec.rb +++ b/spec/requests/better_together/conversations_request_spec.rb @@ -2,13 +2,9 @@ require 'rails_helper' -RSpec.describe 'BetterTogether::Conversations' do +RSpec.describe 'BetterTogether::Conversations', :as_user do include RequestSpecHelper - before do - configure_host_platform - end - let!(:manager_user) do create(:user, :confirmed, :platform_manager, email: 'manager1@example.test', password: 'password12345') end @@ -18,12 +14,7 @@ let!(:non_opted_person) { create(:better_together_person, name: 'Non Opted User') } describe 'GET /conversations/new' do - context 'as a regular member' do - let!(:regular_user) { create(:user, :confirmed, email: 'user@example.test', password: 'password12345') } - - before do - login(regular_user.email, 'password12345') - end + context 'as a regular member', :as_user do it 'lists platform managers and opted-in members, but excludes non-opted members' do get better_together.new_conversation_path(locale: I18n.default_locale) @@ -36,10 +27,7 @@ end end - context 'as a platform manager' do - before do - login(manager_user.email, 'password12345') - end + context 'as a platform manager', :as_platform_manager do it 'lists all people as available participants' do get better_together.new_conversation_path(locale: I18n.default_locale) @@ -52,10 +40,7 @@ end describe 'POST /conversations' do - context 'as a regular member' do - let!(:regular_user) { create(:user, :confirmed, email: 'user2@example.test', password: 'password12345') } - - before { login(regular_user.email, 'password12345') } + context 'as a regular member', :as_user do it 'creates conversation with permitted participants (opted-in) and excludes non-permitted' do post better_together.conversations_path(locale: I18n.default_locale), params: { @@ -65,10 +50,12 @@ } } expect(response).to have_http_status(:found) + # Do not follow redirect here; just check DB state convo = BetterTogether::Conversation.order(created_at: :desc).first - expect(convo.creator).to eq(regular_user.person) + 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(regular_user.person.id) # creator always added + 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 @@ -89,27 +76,27 @@ end describe 'PATCH /conversations/:id' do - context 'as a regular member' do - let!(:regular_user) { create(:user, :confirmed, email: 'user3@example.test', password: 'password12345') } + context 'as a regular member', :as_user do 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) + # 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 - before { login(regular_user.email, 'password12345') } - it 'does not add non-permitted participants on update' do + 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: [regular_user.person.id, non_opted_person.id] + 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(regular_user.person.id) + expect(ids).to include(user.person.id) expect(ids).not_to include(non_opted_person.id) end From ca5e35147eb5958f59b5775ac9e981a5e4180005 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Fri, 29 Aug 2025 15:54:41 -0230 Subject: [PATCH 7/8] Rubocop fixes --- .../conversations_controller.rb | 33 ++++++++++--------- .../person_preferences_spec.rb | 5 ++- .../conversation_policy_spec.rb | 6 ++-- .../conversations_request_spec.rb | 33 ++++++++++++------- 4 files changed, 46 insertions(+), 31 deletions(-) diff --git a/app/controllers/better_together/conversations_controller.rb b/app/controllers/better_together/conversations_controller.rb index f6e08f551..e11608ca3 100644 --- a/app/controllers/better_together/conversations_controller.rb +++ b/app/controllers/better_together/conversations_controller.rb @@ -35,7 +35,8 @@ def create # rubocop:todo Metrics/MethodLength, Metrics/AbcSize authorize @conversation if submitted_any && filtered_empty - @conversation.errors.add(:conversation_participants, t('better_together.conversations.errors.no_permitted_participants')) + @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( @@ -75,7 +76,7 @@ def create # rubocop:todo Metrics/MethodLength, Metrics/AbcSize 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 submitted_any = conversation_params[:participant_ids].present? @@ -83,7 +84,8 @@ def update # rubocop:todo Metrics/AbcSize, Metrics/MethodLength 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')) + @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( @@ -96,7 +98,8 @@ def update # rubocop:todo Metrics/AbcSize, Metrics/MethodLength # 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) + @messages = @conversation.messages.with_all_rich_text + .includes(sender: [:string_translations]).order(:created_at) @message = @conversation.messages.build render :show, status: :unprocessable_entity end @@ -205,7 +208,7 @@ def conversation_params # 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 + 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 @@ -217,18 +220,18 @@ def conversation_params_filtered cp end - def set_conversation + def set_conversation # rubocop:todo Metrics/MethodLength scope = helpers.current_person.conversations.includes(participants: [ - :string_translations, - :contact_detail, - { profile_image_attachment: :blob } - ]) + :string_translations, + :contact_detail, + { profile_image_attachment: :blob } + ]) @conversation = scope.find_by(id: params[:id]) - @conversation ||= Conversation.includes(participants: [ - :string_translations, - :contact_detail, - { profile_image_attachment: :blob } - ]).find(params[:id]) + @set_conversation ||= Conversation.includes(participants: [ + :string_translations, + :contact_detail, + { profile_image_attachment: :blob } + ]).find(params[:id]) end def set_conversations diff --git a/spec/models/better_together/person_preferences_spec.rb b/spec/models/better_together/person_preferences_spec.rb index 3d573ea69..8e59e3905 100644 --- a/spec/models/better_together/person_preferences_spec.rb +++ b/spec/models/better_together/person_preferences_spec.rb @@ -2,9 +2,9 @@ require 'rails_helper' -RSpec.describe BetterTogether::Person, type: :model do +RSpec.describe BetterTogether::Person do describe 'message opt-in preference' do - it 'defaults to false and can be toggled to true' 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) @@ -14,4 +14,3 @@ end end end - diff --git a/spec/policies/better_together/conversation_policy_spec.rb b/spec/policies/better_together/conversation_policy_spec.rb index 70b6b1c17..c3041bd18 100644 --- a/spec/policies/better_together/conversation_policy_spec.rb +++ b/spec/policies/better_together/conversation_policy_spec.rb @@ -5,7 +5,7 @@ RSpec.describe BetterTogether::ConversationPolicy, type: :policy do include RequestSpecHelper - let!(:host_platform) { configure_host_platform } + 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 } @@ -25,10 +25,12 @@ end end - context 'when agent is a regular member' do + 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) diff --git a/spec/requests/better_together/conversations_request_spec.rb b/spec/requests/better_together/conversations_request_spec.rb index f17233124..70e5bbc91 100644 --- a/spec/requests/better_together/conversations_request_spec.rb +++ b/spec/requests/better_together/conversations_request_spec.rb @@ -14,9 +14,10 @@ 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 - + 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 @@ -27,9 +28,8 @@ end end - context 'as a platform manager', :as_platform_manager do - - it 'lists all people as available participants' do + 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) @@ -40,9 +40,11 @@ end describe 'POST /conversations' do - context 'as a regular member', :as_user 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', @@ -56,11 +58,14 @@ 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).to include(opted_in_person.id) # allowed expect(ids).not_to include(non_opted_person.id) # filtered out end + # rubocop:enable RSpec/ExampleLength - it 'shows an error when only non-permitted participants are submitted' do + # 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: { @@ -76,7 +81,7 @@ end describe 'PATCH /conversations/:id' do - context 'as a regular member', :as_user 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') @@ -85,7 +90,9 @@ end end - it 'does not add non-permitted participants on update' do + # 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: { @@ -100,7 +107,10 @@ 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, @@ -110,6 +120,7 @@ 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 From 4900c4f23798e450c8ea72cb671065be400d3086 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Fri, 29 Aug 2025 16:40:43 -0230 Subject: [PATCH 8/8] Raise not-found for inaccessible conversations --- .../better_together/conversations_controller.rb | 2 +- .../better_together/conversations_request_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/controllers/better_together/conversations_controller.rb b/app/controllers/better_together/conversations_controller.rb index e11608ca3..bbcc474e4 100644 --- a/app/controllers/better_together/conversations_controller.rb +++ b/app/controllers/better_together/conversations_controller.rb @@ -226,7 +226,7 @@ def set_conversation # rubocop:todo Metrics/MethodLength :contact_detail, { profile_image_attachment: :blob } ]) - @conversation = scope.find_by(id: params[:id]) + @conversation = scope.find(params[:id]) @set_conversation ||= Conversation.includes(participants: [ :string_translations, :contact_detail, diff --git a/spec/requests/better_together/conversations_request_spec.rb b/spec/requests/better_together/conversations_request_spec.rb index 70e5bbc91..ee8438ad7 100644 --- a/spec/requests/better_together/conversations_request_spec.rb +++ b/spec/requests/better_together/conversations_request_spec.rb @@ -80,6 +80,19 @@ 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