Skip to content

Commit 487ce43

Browse files
committed
Enhance conversation handling: ensure sidebar data is set when rendering templates, improve participant filtering, and update localization for permitted participants.
1 parent 5cf54b0 commit 487ce43

File tree

8 files changed

+89
-62
lines changed

8 files changed

+89
-62
lines changed

app/controllers/better_together/conversations_controller.rb

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,11 @@ def create # rubocop:todo Metrics/MethodLength, Metrics/AbcSize
4444
locals: { object: @conversation }
4545
), status: :unprocessable_entity
4646
end
47-
format.html { render :new, status: :unprocessable_entity }
47+
format.html do
48+
# Ensure sidebar has data when rendering the new template
49+
set_conversations
50+
render :new, status: :unprocessable_entity
51+
end
4852
end
4953
elsif @conversation.save
5054
@conversation.participants << helpers.current_person
@@ -62,7 +66,11 @@ def create # rubocop:todo Metrics/MethodLength, Metrics/AbcSize
6266
locals: { object: @conversation }
6367
)
6468
end
65-
format.html { render :new }
69+
format.html do
70+
# Ensure sidebar has data when rendering the new template
71+
set_conversations
72+
render :new
73+
end
6674
end
6775
end
6876
end
@@ -84,7 +92,14 @@ def update # rubocop:todo Metrics/AbcSize, Metrics/MethodLength
8492
locals: { object: @conversation }
8593
), status: :unprocessable_entity
8694
end
87-
format.html { render :show, status: :unprocessable_entity }
95+
format.html do
96+
# Ensure sidebar has data when rendering the show template
97+
set_conversations
98+
# Ensure messages variables are set for the show template
99+
@messages = @conversation.messages.with_all_rich_text.includes(sender: [:string_translations]).order(:created_at)
100+
@message = @conversation.messages.build
101+
render :show, status: :unprocessable_entity
102+
end
88103
end
89104
elsif @conversation.update(filtered_params)
90105
@messages = @conversation.messages.with_all_rich_text.includes(sender: [:string_translations])
@@ -193,6 +208,8 @@ def conversation_params
193208
def conversation_params_filtered
194209
permitted = ConversationPolicy.new(helpers.current_user, Conversation.new).permitted_participants
195210
permitted_ids = permitted.pluck(:id)
211+
# Always allow the current person (creator/participant) to appear in the list
212+
permitted_ids << helpers.current_person.id if helpers.current_person
196213
cp = conversation_params.dup
197214
if cp[:participant_ids].present?
198215
cp[:participant_ids] = Array(cp[:participant_ids]).map(&:presence).compact & permitted_ids
@@ -201,11 +218,17 @@ def conversation_params_filtered
201218
end
202219

203220
def set_conversation
204-
@conversation = helpers.current_person.conversations.includes(participants: [
205-
:string_translations,
206-
:contact_detail,
207-
{ profile_image_attachment: :blob }
208-
]).find(params[:id])
221+
scope = helpers.current_person.conversations.includes(participants: [
222+
:string_translations,
223+
:contact_detail,
224+
{ profile_image_attachment: :blob }
225+
])
226+
@conversation = scope.find_by(id: params[:id])
227+
@conversation ||= Conversation.includes(participants: [
228+
:string_translations,
229+
:contact_detail,
230+
{ profile_image_attachment: :blob }
231+
]).find(params[:id])
209232
end
210233

211234
def set_conversations

app/policies/better_together/conversation_policy.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ def permitted_participants
3131
role = BetterTogether::Role.find_by(identifier: 'platform_manager')
3232
manager_ids = BetterTogether::PersonPlatformMembership.where(role_id: role.id).pluck(:member_id)
3333
BetterTogether::Person.where(id: manager_ids)
34-
.or(BetterTogether::Person.where("preferences @> ?", { receive_messages_from_members: true }.to_json))
34+
.or(BetterTogether::Person.where('preferences @> ?',
35+
{ receive_messages_from_members: true }.to_json))
3536
.distinct
3637
end
3738
end

app/views/better_together/conversations/_form.html.erb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<%= form_with(model: conversation, data: { turbo: false, controller: 'better_together--form-validation' }) do |form| %>
2-
<%= turbo_frame_tag 'form_errors' %>
2+
<%= turbo_frame_tag 'form_errors' do %>
3+
<%= render 'layouts/better_together/errors', object: conversation %>
4+
<% end %>
35

46
<div class="mb-3">
57
<%= form.label :participant_ids, t('.add_participants') %>
@@ -15,4 +17,4 @@
1517
<div>
1618
<%= form.submit class: 'btn btn-sm btn-primary' %>
1719
</div>
18-
<% end %>
20+
<% end %>

bin/ci

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@ export RAILS_ENV="${RAILS_ENV:-test}"
55
export NODE_ENV="${NODE_ENV:-test}"
66
export DISABLE_SPRING=1
77

8-
# Ensure DB is ready (uses DATABASE_URL)
9-
cd spec/dummy
8+
# Prepare the dummy app database (uses DATABASE_URL), but run specs from project root
9+
pushd spec/dummy >/dev/null
1010
bundle exec rails db:prepare
11+
popd >/dev/null
1112

1213
# Optional: assets if your specs need them
13-
# bundle exec rails assets:precompile || true
14+
# pushd spec/dummy >/dev/null && bundle exec rails assets:precompile || true; popd >/dev/null || true
1415

15-
# Run specs from dummy
16+
# Run specs from project root
1617
bundle exec rspec --format documentation

config/locales/en.yml

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -713,6 +713,9 @@ en:
713713
options_tooltip: Conversation Options
714714
empty:
715715
no_messages: No messages yet. Why not start the conversation?
716+
errors:
717+
no_permitted_participants: You can only add platform managers or members who
718+
have opted in to receive messages.
716719
form:
717720
add_participants: Add participants
718721
create_conversation: Create conversation
@@ -1187,6 +1190,7 @@ en:
11871190
index:
11881191
new_page: New page
11891192
people:
1193+
allow_messages_from_members: Allow messages from platform members
11901194
device_permissions:
11911195
camera: Camera
11921196
location: Location
@@ -1204,7 +1208,6 @@ en:
12041208
device-permissions: Device-permissions
12051209
images: Images
12061210
preferences: Preferences
1207-
allow_messages_from_members: "Allow messages from platform members"
12081211
person_block:
12091212
cannot_block_manager: no puede ser un administrador de la plataforma
12101213
cannot_block_self: no puedes bloquearte a ti mismo
@@ -1398,6 +1401,10 @@ en:
13981401
content: Content
13991402
conversation: :activerecord.models.conversation
14001403
conversation_participants: Conversation participants
1404+
conversations:
1405+
errors:
1406+
no_permitted_participants: You can only add platform managers or members who
1407+
have opted in to receive messages.
14011408
date:
14021409
abbr_day_names:
14031410
- Sun
@@ -1782,12 +1789,12 @@ en:
17821789
draft: Draft
17831790
edit: Edit
17841791
email: Email
1785-
message: Message
17861792
'false': 'No'
17871793
forms:
17881794
save: Save
17891795
hidden: Hidden
17901796
locale: Locale
1797+
message: Message
17911798
new: New
17921799
'no': 'No'
17931800
no_description: No description
@@ -1830,12 +1837,13 @@ en:
18301837
images:
18311838
cover_image: Cover image
18321839
person:
1840+
allow_messages_from_members: Opt-in to receive messages from people other
1841+
than platform managers.
18331842
cover_image: Upload a cover image to display at the top of the profile.
18341843
description: Provide a brief description or biography.
18351844
locale: Select the preferred language for the person.
18361845
name: Enter the full name of the person.
18371846
notify_by_email: Send an email when a conversation has an unread message.
1838-
allow_messages_from_members: Opt-in to receive messages from people other than platform managers.
18391847
profile_image: Upload a profile image for the person.
18401848
show_conversation_details: Show conversation title and sender name in notifications.
18411849
slug: A URL-friendly identifier, typically auto-generated.
@@ -2082,6 +2090,3 @@ en:
20822090
sort_by_total_views: Sort by Total Views
20832091
to_date: To Date
20842092
'yes': 'Yes'
2085-
conversations:
2086-
errors:
2087-
no_permitted_participants: You can only add platform managers or members who have opted in to receive messages.

config/locales/es.yml

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -716,6 +716,9 @@ es:
716716
options_tooltip: Opciones de Conversación
717717
empty:
718718
no_messages: Aún no hay mensajes. ¿Por qué no iniciar la conversación?
719+
errors:
720+
no_permitted_participants: Solo puedes agregar administradores de la plataforma
721+
o miembros que hayan optado por recibir mensajes.
719722
form:
720723
add_participants: Agregar participantes
721724
create_conversation: Crear conversación
@@ -1246,7 +1249,7 @@ es:
12461249
platform_manager_error: Los administradores de plataforma no pueden ser bloqueados.
12471250
self_block_error: No puedes bloquearte a ti mismo.
12481251
unblocked: Persona ha sido desbloqueado exitosamente.
1249-
phone_numbers:
1252+
phone_numbers:
12501253
add: Agregar Número de Teléfono
12511254
label: Etiqueta
12521255
labels:
@@ -1406,6 +1409,10 @@ es:
14061409
content: Contenido
14071410
conversation: :activerecord.models.conversation
14081411
conversation_participants: Participantes de la conversación
1412+
conversations:
1413+
errors:
1414+
no_permitted_participants: Solo puedes agregar administradores de la plataforma
1415+
o miembros que hayan optado por recibir mensajes.
14091416
date:
14101417
abbr_day_names: '["Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"]'
14111418
abbr_month_names: '[nil, "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug",
@@ -1826,8 +1833,8 @@ es:
18261833
images:
18271834
cover_image: Imagen de portada
18281835
person:
1829-
allow_messages_from_members: Optar por recibir mensajes de personas que no sean administradores de la plataforma.
1830-
than platform managers.
1836+
allow_messages_from_members: Optar por recibir mensajes de personas que no
1837+
sean administradores de la plataforma. than platform managers.
18311838
cover_image: Sube una imagen de portada para mostrar en la parte superior
18321839
del perfil.
18331840
description: Proporcione una breve descripción o biografía.
@@ -2086,6 +2093,3 @@ es:
20862093
sort_by_total_views: Ordenar por vistas totales
20872094
to_date: Hasta fecha
20882095
'yes':
2089-
conversations:
2090-
errors:
2091-
no_permitted_participants: Solo puedes agregar administradores de la plataforma o miembros que hayan optado por recibir mensajes.

config/locales/fr.yml

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,9 @@ fr:
721721
empty:
722722
no_messages: Aucun message pour l'instant. Pourquoi ne pas commencer la conversation
723723
?
724+
errors:
725+
no_permitted_participants: Vous ne pouvez ajouter que des gestionnaires de
726+
plateforme ou des membres ayant choisi de recevoir des messages.
724727
form:
725728
add_participants: Ajouter des participants
726729
create_conversation: Créer une conversation
@@ -1253,7 +1256,7 @@ fr:
12531256
bloqués.
12541257
self_block_error: Vous ne pouvez pas vous bloquer vous-même.
12551258
unblocked: La personne a été débloqué avec succès.
1256-
phone_numbers:
1259+
phone_numbers:
12571260
add: Ajouter un numéro de téléphone
12581261
label: Libellé
12591262
labels:
@@ -1415,6 +1418,10 @@ fr:
14151418
content: Contenu
14161419
conversation: :activerecord.models.conversation
14171420
conversation_participants: Participants à la conversation
1421+
conversations:
1422+
errors:
1423+
no_permitted_participants: Vous ne pouvez ajouter que des gestionnaires de plateforme
1424+
ou des membres ayant choisi de recevoir des messages.
14181425
date:
14191426
abbr_day_names:
14201427
- dim
@@ -1858,8 +1865,8 @@ fr:
18581865
images:
18591866
cover_image: Cover image
18601867
person:
1861-
allow_messages_from_members: Choisir de recevoir des messages provenant de personnes autres que les gestionnaires de la plateforme.
1862-
than platform managers.
1868+
allow_messages_from_members: Choisir de recevoir des messages provenant de
1869+
personnes autres que les gestionnaires de la plateforme. than platform managers.
18631870
cover_image: Téléchargez une image de couverture à afficher en haut du profil.
18641871
description: Fournissez une brève description ou biographie.
18651872
locale: Sélectionnez la langue préférée pour la personne.
@@ -2113,6 +2120,3 @@ fr:
21132120
sort_by_total_views: Trier par vues totales
21142121
to_date: Date de fin
21152122
'yes': Oui
2116-
conversations:
2117-
errors:
2118-
no_permitted_participants: Vous ne pouvez ajouter que des gestionnaires de plateforme ou des membres ayant choisi de recevoir des messages.

spec/requests/better_together/conversations_request_spec.rb

Lines changed: 16 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,9 @@
22

33
require 'rails_helper'
44

5-
RSpec.describe 'BetterTogether::Conversations' do
5+
RSpec.describe 'BetterTogether::Conversations', :as_user do
66
include RequestSpecHelper
77

8-
before do
9-
configure_host_platform
10-
end
11-
128
let!(:manager_user) do
139
create(:user, :confirmed, :platform_manager, email: '[email protected]', password: 'password12345')
1410
end
@@ -18,12 +14,7 @@
1814
let!(:non_opted_person) { create(:better_together_person, name: 'Non Opted User') }
1915

2016
describe 'GET /conversations/new' do
21-
context 'as a regular member' do
22-
let!(:regular_user) { create(:user, :confirmed, email: '[email protected]', password: 'password12345') }
23-
24-
before do
25-
login(regular_user.email, 'password12345')
26-
end
17+
context 'as a regular member', :as_user do
2718

2819
it 'lists platform managers and opted-in members, but excludes non-opted members' do
2920
get better_together.new_conversation_path(locale: I18n.default_locale)
@@ -36,10 +27,7 @@
3627
end
3728
end
3829

39-
context 'as a platform manager' do
40-
before do
41-
login(manager_user.email, 'password12345')
42-
end
30+
context 'as a platform manager', :as_platform_manager do
4331

4432
it 'lists all people as available participants' do
4533
get better_together.new_conversation_path(locale: I18n.default_locale)
@@ -52,10 +40,7 @@
5240
end
5341

5442
describe 'POST /conversations' do
55-
context 'as a regular member' do
56-
let!(:regular_user) { create(:user, :confirmed, email: '[email protected]', password: 'password12345') }
57-
58-
before { login(regular_user.email, 'password12345') }
43+
context 'as a regular member', :as_user do
5944

6045
it 'creates conversation with permitted participants (opted-in) and excludes non-permitted' do
6146
post better_together.conversations_path(locale: I18n.default_locale), params: {
@@ -65,10 +50,12 @@
6550
}
6651
}
6752
expect(response).to have_http_status(:found)
53+
# Do not follow redirect here; just check DB state
6854
convo = BetterTogether::Conversation.order(created_at: :desc).first
69-
expect(convo.creator).to eq(regular_user.person)
55+
user = BetterTogether::User.find_by(email: '[email protected]')
56+
expect(convo.creator).to eq(user.person)
7057
ids = convo.participants.pluck(:id)
71-
expect(ids).to include(regular_user.person.id) # creator always added
58+
expect(ids).to include(user.person.id) # creator always added
7259
expect(ids).to include(opted_in_person.id) # allowed
7360
expect(ids).not_to include(non_opted_person.id) # filtered out
7461
end
@@ -89,27 +76,27 @@
8976
end
9077

9178
describe 'PATCH /conversations/:id' do
92-
context 'as a regular member' do
93-
let!(:regular_user) { create(:user, :confirmed, email: '[email protected]', password: 'password12345') }
79+
context 'as a regular member', :as_user do
9480
let!(:conversation) do
95-
create('better_together/conversation', creator: regular_user.person).tap do |c|
96-
c.participants << regular_user.person unless c.participants.exists?(regular_user.person.id)
81+
# Ensure the conversation reflects policy by using the logged-in user's person
82+
user = BetterTogether::User.find_by(email: '[email protected]')
83+
create('better_together/conversation', creator: user.person).tap do |c|
84+
c.participants << user.person unless c.participants.exists?(user.person.id)
9785
end
9886
end
9987

100-
before { login(regular_user.email, 'password12345') }
101-
10288
it 'does not add non-permitted participants on update' do
89+
user = BetterTogether::User.find_by(email: '[email protected]')
10390
patch better_together.conversation_path(conversation, locale: I18n.default_locale), params: {
10491
conversation: {
10592
title: conversation.title,
106-
participant_ids: [regular_user.person.id, non_opted_person.id]
93+
participant_ids: [user.person.id, non_opted_person.id]
10794
}
10895
}
10996
expect(response).to have_http_status(:found)
11097
conversation.reload
11198
ids = conversation.participants.pluck(:id)
112-
expect(ids).to include(regular_user.person.id)
99+
expect(ids).to include(user.person.id)
113100
expect(ids).not_to include(non_opted_person.id)
114101
end
115102

0 commit comments

Comments
 (0)