diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md
index b8aac7c82..e01d210b8 100644
--- a/.github/copilot-instructions.md
+++ b/.github/copilot-instructions.md
@@ -116,6 +116,7 @@ This repository contains the **Better Together Community Engine** (an isolated R
- **Use allow-lists for dynamic class resolution**: Follow the `joatu_source_class` pattern with concern-based allow-lists
- **Validate user inputs**: Always sanitize and validate parameters, especially for file uploads and dynamic queries
- **Strong parameters**: Use Rails strong parameters in all controllers
+ - **Model-level permitted attributes**: Prefer defining a class method `self.permitted_attributes` on models that returns the permitted attribute array (including nested attributes). Controllers and shared resource code should call `Model.permitted_attributes` rather than hard-coding permit lists. Compose nested permitted attributes by referencing other models' `permitted_attributes` (for example: `Conversation.permitted_attributes` may include `{ messages_attributes: Message.permitted_attributes }`).
- **Authorization everywhere**: Implement Pundit policy checks on all actions
- **SQL injection prevention**: Use parameterized queries, avoid string interpolation in SQL
- **XSS prevention**: Use Rails auto-escaping, sanitize HTML inputs with allowlists
@@ -205,6 +206,35 @@ This repository contains the **Better Together Community Engine** (an isolated R
- **Rails-Controller-Testing**: Add `gem 'rails-controller-testing'` to Gemfile for `assigns` method in controller tests.
- Toggle requires_invitation and provide invitation_code when needed for registration tests.
+### Automatic test configuration & auth helper patterns
+
+This repository provides an automatic test-configuration layer (see `spec/support/automatic_test_configuration.rb`) that sets up the host `Platform` and, where appropriate, performs authentication for request, controller, and feature specs so most specs do NOT need to call `configure_host_platform` manually.
+
+- Automatic setup applies to specs with `type: :request`, `type: :controller`, and `type: :feature` by default.
+- Use these example metadata tags to control authentication explicitly:
+ - `:as_platform_manager` or `:platform_manager` — login as the platform manager (elevated privileges)
+ - `:as_user`, `:authenticated`, or `:user` — login as a regular user
+ - `:no_auth` or `:unauthenticated` — ensure no authentication is performed for the example
+ - `:skip_host_setup` — skip host platform creation/configuration for this example
+
+How it works:
+- The test helper inspects example metadata and description text (describe/context). If the description contains keywords such as "platform manager", "admin", "authenticated", or "signed in", it will automatically set appropriate tags and perform the corresponding authentication.
+- The helper creates a host `Platform` if one does not exist and marks the default setup wizard as completed.
+- For request specs it uses HTTP login helpers (`login(email, password)`); for controller specs it uses Devise test helpers (`sign_in`); for feature specs it uses Capybara UI login flows.
+
+Recommended usage:
+- Prefer using metadata tags (`:as_platform_manager`, `:as_user`, `:skip_host_setup`) in the `describe` or `context` header when a test needs a specific authentication state. Example:
+
+```ruby
+RSpec.describe 'Creating a conversation', type: :request, :as_user do
+ # host platform and user login are automatically configured
+end
+```
+
+- Avoid calling `configure_host_platform` manually in most specs; reserve manual calls for special cases (use `:skip_host_setup` to opt out of automatic config).
+
+Note: The helper set lives under `spec/support/automatic_test_configuration.rb` and provides helpers like `configure_host_platform`, `find_or_create_test_user`, and `capybara_login_as_platform_manager` to use directly if needed by unusual tests.
+
### Testing Architecture Standards
- **Project Standard**: Use request specs (`type: :request`) for all controller testing to maintain consistency
- **Request Specs Advantages**: Handle Rails engine routing automatically through full HTTP stack
diff --git a/AGENTS.md b/AGENTS.md
index f4ed7c1a6..f11e3327b 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -50,6 +50,7 @@ Instructions for GitHub Copilot and other automated contributors working in this
- Use allow-lists for dynamic class resolution (see `joatu_source_class` pattern)
- Sanitize and validate all user inputs
- Use strong parameters in controllers
+ - Define model-level permitted attributes: prefer a class method `self.permitted_attributes` on models that returns the permitted attribute list (including nested attribute structures). Controllers should call `Model.permitted_attributes` to build permit lists instead of hard-coding them. When composing nested attributes, reference other models' `permitted_attributes` (for example: `Conversation.permitted_attributes` may include `{ messages_attributes: Message.permitted_attributes }`).
- Implement proper authorization checks (Pundit policies)
- **For reflection-based features**: Create concerns with `included_in_models` class methods for safe dynamic class resolution
- **Post-generation security check**: Run `bin/dc-run bundle exec brakeman --quiet --no-pager -c UnsafeReflection,SQL,CrossSiteScripting` after major code changes
@@ -226,6 +227,35 @@ For every implementation plan, create acceptance criteria covering relevant stak
- **Required for**: Controller specs, request specs, feature specs, and any integration tests that involve routing or authentication.
- **Locale Parameters**: Engine controller tests require locale parameters (e.g., `params: { locale: I18n.default_locale }`) due to routing constraints.
+### Automatic test configuration & auth helper patterns
+
+This repository provides an automatic test-configuration layer (see `spec/support/automatic_test_configuration.rb`) that sets up the host `Platform` and, where appropriate, performs authentication for request, controller, and feature specs so most specs do NOT need to call `configure_host_platform` manually.
+
+- Automatic setup applies to specs with `type: :request`, `type: :controller`, and `type: :feature` by default.
+- Use these example metadata tags to control authentication explicitly:
+ - `:as_platform_manager` or `:platform_manager` — login as the platform manager (elevated privileges)
+ - `:as_user`, `:authenticated`, or `:user` — login as a regular user
+ - `:no_auth` or `:unauthenticated` — ensure no authentication is performed for the example
+ - `:skip_host_setup` — skip host platform creation/configuration for this example
+
+How it works:
+- The test helper inspects example metadata and description text (describe/context). If the description contains keywords such as "platform manager", "admin", "authenticated", or "signed in", it will automatically set appropriate tags and perform the corresponding authentication.
+- The helper creates a host `Platform` if one does not exist and marks the default setup wizard as completed.
+- For request specs it uses HTTP login helpers (`login(email, password)`); for controller specs it uses Devise test helpers (`sign_in`); for feature specs it uses Capybara UI login flows.
+
+Recommended usage:
+- Prefer using metadata tags (`:as_platform_manager`, `:as_user`, `:skip_host_setup`) in the `describe` or `context` header when a test needs a specific authentication state. Example:
+
+```ruby
+RSpec.describe 'Creating a conversation', type: :request, :as_user do
+ # host platform and user login are automatically configured
+end
+```
+
+- Avoid calling `configure_host_platform` manually in most specs; reserve manual calls for special cases (use `:skip_host_setup` to opt out of automatic config).
+
+Note: The helper set lives under `spec/support/automatic_test_configuration.rb` and provides helpers like `configure_host_platform`, `find_or_create_test_user`, and `capybara_login_as_platform_manager` to use directly if needed by unusual tests.
+
## Test Coverage Standards
- **Models**: Test validations, associations, scopes, instance methods, class methods, and callbacks.
- **Controllers**: Test all actions, authorization policies, parameter handling, and response formats.
diff --git a/app/assets/stylesheets/better_together/_resource_toolbar.scss b/app/assets/stylesheets/better_together/_resource_toolbar.scss
new file mode 100644
index 000000000..d1ace7896
--- /dev/null
+++ b/app/assets/stylesheets/better_together/_resource_toolbar.scss
@@ -0,0 +1,14 @@
+// Styles specific to the shared resource toolbar
+
+.resource-toolbar {
+ display: flex;
+ align-items: center;
+}
+
+.resource-toolbar-extra {
+ display: flex;
+ align-items: center;
+ margin-left: auto; // Fallback in case .ms-auto is unavailable
+ gap: 0.5rem; // Consistent spacing for appended actions
+}
+
diff --git a/app/assets/stylesheets/better_together/application.scss b/app/assets/stylesheets/better_together/application.scss
index 03bf6fecf..09631f3f3 100644
--- a/app/assets/stylesheets/better_together/application.scss
+++ b/app/assets/stylesheets/better_together/application.scss
@@ -37,6 +37,7 @@
@use 'share';
@use 'sidebar_nav';
@use 'trix-extensions/richtext';
+@use 'resource_toolbar';
// Styles that use the variables
.text-opposite-theme {
@@ -104,4 +105,3 @@
.spin-horizontal {
animation: flip 1s linear infinite;
}
-
diff --git a/app/assets/stylesheets/better_together/conversations.scss b/app/assets/stylesheets/better_together/conversations.scss
index 55e2f4249..1d9cd14a5 100644
--- a/app/assets/stylesheets/better_together/conversations.scss
+++ b/app/assets/stylesheets/better_together/conversations.scss
@@ -93,3 +93,74 @@
overflow-y: auto;
}
+/* Responsive tweaks for conversation header & participants */
+/* Reduce header padding and font size on small screens */
+@media (max-width: 991.98px) { // Bootstrap lg breakpoint
+ /* Reduce overall header vertical padding on small screens */
+ .card-header {
+ padding-top: 0.25rem !important;
+ padding-bottom: 0.25rem !important;
+ }
+
+ /* Make the title smaller when present */
+ .card-header h4 {
+ font-size: 1rem; /* smaller header */
+ margin: 0;
+ }
+
+ /* Allow header children to wrap so we can place participants on their own row */
+ .card-header {
+ flex-wrap: wrap;
+ }
+
+ /* Reduce padding on the back link (left arrow) to save vertical space */
+ .card-header > a.p-4 {
+ padding-top: 0.25rem !important;
+ padding-bottom: 0.25rem !important;
+ padding-left: 0.5rem !important;
+ padding-right: 0.5rem !important;
+ }
+
+ /* Reduce horizontal margin around the options dropdown icon */
+ .card-header .mx-4 {
+ margin-left: 0.5rem !important;
+ margin-right: 0.5rem !important;
+ }
+
+ /* Place the participants container on its own full-width row below the header
+ when an h4 title exists, but keep participant items inline (horizontal). */
+ .card-header h4 + .conversation-participants {
+ flex: 0 0 100% !important;
+ order: 3;
+ width: 100%;
+ display: flex !important;
+ flex-direction: row !important; /* keep items side-by-side */
+ align-items: center !important;
+ justify-content: space-evenly !important; /* distribute participants evenly */
+ gap: 0.5rem;
+ margin-top: 0.5rem;
+ overflow-x: auto;
+ }
+
+ /* Small spacing adjustments for individual participant blocks when moved below */
+ .card-header h4 + .conversation-participants .mention_person {
+ margin-right: 0.5rem !important;
+ padding-top: 0.15rem;
+ padding-bottom: 0.15rem;
+ }
+
+ /* Shrink avatar size for participants in the header row to reduce height */
+ .card-header h4 + .conversation-participants .mention_person .profile-image {
+ width: 32px !important;
+ height: 32px !important;
+ object-fit: cover;
+ }
+
+ /* Ensure the options (ellipsis) container hugs the right edge of the header */
+ .card-header > .align-self-center.mx-4 {
+ margin-left: auto !important;
+ order: 2; /* keep it on the top row before participants (participants are order:3) */
+ align-self: center !important;
+ }
+}
+
diff --git a/app/controllers/better_together/conversations_controller.rb b/app/controllers/better_together/conversations_controller.rb
index bbcc474e4..6b7ce2ee8 100644
--- a/app/controllers/better_together/conversations_controller.rb
+++ b/app/controllers/better_together/conversations_controller.rb
@@ -16,11 +16,23 @@ class ConversationsController < ApplicationController # rubocop:todo Metrics/Cla
helper_method :available_participants
def index
- authorize @conversations
+ # Conversations list is prepared by set_conversations (before_action)
+ # Provide a blank conversation for the new-conversation form in the sidebar
+ @conversation = Conversation.new
+ authorize @conversation
end
def new
- @conversation = Conversation.new
+ if params[:conversation].present?
+ conv_params = params.require(:conversation).permit(:title, participant_ids: [])
+ @conversation = Conversation.new(conv_params)
+ else
+ @conversation = Conversation.new
+ end
+
+ # Ensure nested message is available for the form (so users can create the first message inline)
+ @conversation.messages.build if @conversation.messages.empty?
+
authorize @conversation
end
@@ -32,6 +44,13 @@ def create # rubocop:todo Metrics/MethodLength, Metrics/AbcSize
@conversation = Conversation.new(filtered_params.merge(creator: helpers.current_person))
+ # If nested messages were provided, ensure the sender is set to the creator/current person
+ if @conversation.messages.any?
+ @conversation.messages.each do |m|
+ m.sender = helpers.current_person
+ end
+ end
+
authorize @conversation
if submitted_any && filtered_empty
@@ -52,7 +71,7 @@ def create # rubocop:todo Metrics/MethodLength, Metrics/AbcSize
end
end
elsif @conversation.save
- @conversation.participants << helpers.current_person
+ @conversation.add_participant_safe(helpers.current_person)
respond_to do |format|
format.turbo_stream
@@ -203,20 +222,47 @@ def available_participants
end
def conversation_params
- params.require(:conversation).permit(:title, participant_ids: [])
+ # Use model-defined permitted attributes so nested attributes composition stays DRY
+ params.require(:conversation).permit(*Conversation.permitted_attributes)
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 # rubocop:todo Metrics/AbcSize
+ def conversation_params_filtered # rubocop:todo Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity
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
+
+ # Filter participant_ids to only those the agent may message
if cp[:participant_ids].present?
cp[:participant_ids] = Array(cp[:participant_ids]).map(&:presence).compact & permitted_ids
end
+
+ # Protect nested messages on update: only allow creating messages via the create path.
+ # On update, permit edits only to existing messages that belong to the current person,
+ # and only allow their content (prevent sender_id spoofing or reassigning other people's messages).
+ if action_name == 'update' && cp[:messages_attributes].present?
+ safe_messages = Array(cp[:messages_attributes]).map do |m|
+ # handle ActionController::Parameters or Hash
+ attrs = m.respond_to?(:to_h) ? m.to_h : m
+ msg_id = attrs['id'] || attrs[:id]
+ next nil unless msg_id
+
+ msg = BetterTogether::Message.find_by(id: msg_id)
+ next nil unless msg && helpers.current_person && msg.sender_id == helpers.current_person.id
+
+ # Only allow content edits through this path
+ { 'id' => msg_id, 'content' => attrs['content'] || attrs[:content] }
+ end.compact
+
+ # Replace messages_attributes with the vetted set (or nil if none)
+ cp[:messages_attributes] = safe_messages.presence
+ end
+
+ # On create, leave messages_attributes as-is so nested creation works; controller will set sender.
cp
end
diff --git a/app/controllers/better_together/messages_controller.rb b/app/controllers/better_together/messages_controller.rb
index b72e3cbfe..7ac8c163a 100644
--- a/app/controllers/better_together/messages_controller.rb
+++ b/app/controllers/better_together/messages_controller.rb
@@ -28,7 +28,7 @@ def set_conversation
end
def message_params
- params.require(:message).permit(:content)
+ params.require(:message).permit(*BetterTogether::Message.permitted_attributes)
end
def notify_participants(message)
diff --git a/app/controllers/better_together/navigation_areas_controller.rb b/app/controllers/better_together/navigation_areas_controller.rb
index 14727f87b..b7996ffde 100644
--- a/app/controllers/better_together/navigation_areas_controller.rb
+++ b/app/controllers/better_together/navigation_areas_controller.rb
@@ -101,7 +101,7 @@ def set_navigation_area
end
def navigation_area_params
- params.require(:navigation_area).permit(:name, :slug, :visible, :style, :protected)
+ params.require(:navigation_area).permit(*resource_class.permitted_attributes)
end
def resource_class
diff --git a/app/controllers/better_together/navigation_items_controller.rb b/app/controllers/better_together/navigation_items_controller.rb
index d56e4298b..3fb41593e 100644
--- a/app/controllers/better_together/navigation_items_controller.rb
+++ b/app/controllers/better_together/navigation_items_controller.rb
@@ -137,11 +137,7 @@ def navigation_item
end
def navigation_item_params
- params.require(:navigation_item).permit(
- :navigation_area_id, :url, :icon, :position, :visible,
- :item_type, :linkable_id, :parent_id, :route_name,
- *resource_class.localized_attribute_list
- )
+ params.require(:navigation_item).permit(*resource_class.permitted_attributes)
end
def resource_class
diff --git a/app/helpers/better_together/form_helper.rb b/app/helpers/better_together/form_helper.rb
index d0508e7f8..6f733316b 100644
--- a/app/helpers/better_together/form_helper.rb
+++ b/app/helpers/better_together/form_helper.rb
@@ -116,25 +116,40 @@ def privacy_field(form:, klass:)
end
# rubocop:todo Metrics/MethodLength
- def required_label(form_or_object, field, **options) # rubocop:todo Metrics/AbcSize, Metrics/MethodLength
+ # Accepts an optional label_text override which, when provided, will be used
+ # instead of the model's human_attribute_name for the field. This is useful
+ # when the visible label needs to be different from the translated attribute
+ # name (for example: participant_ids -> "Add participants").
+ # rubocop:todo Metrics/PerceivedComplexity
+ def required_label(form_or_object, field, label_text: nil, **options) # rubocop:todo Metrics/AbcSize, Metrics/MethodLength
# Determine if it's a form object or just an object
if form_or_object.respond_to?(:object)
object = form_or_object.object
- label_text = object.class.human_attribute_name(field)
+ # Use provided label_text override if present, otherwise fall back to translation
+ label_text ||= object.class.human_attribute_name(field)
class_name = options.delete(:class_name)
# Use the provided class_name for validation check if present, otherwise use the object's class
else
object = form_or_object
- label_text = object.class.human_attribute_name(field)
+ label_text ||= object.class.human_attribute_name(field)
# Use the provided class_name for validation check if present, otherwise use the object's class
end
+
klass = class_name ? class_name.constantize : object.class
is_required = class_field_required(klass, field)
- # Append asterisk for required fields
- label_text += " *" if is_required
+ # Append asterisk for required fields and attach tooltip to the asterisk
+ if is_required
+ tooltip_text = I18n.t('helpers.required_info', default: 'This field is required')
+ # Make the asterisk keyboard-focusable and allow the tooltip to be
+ # triggered by click as well as hover/focus so it works on mobile.
+ asterisk = content_tag(:span, '*', class: 'required-indicator', tabindex: 0, role: 'button',
+ data: { bs_toggle: 'tooltip', bs_trigger: 'hover focus click' },
+ title: tooltip_text)
+ label_text += " #{asterisk}"
+ end
if form_or_object.respond_to?(:label)
form_or_object.label(field, label_text.html_safe, options)
@@ -142,6 +157,7 @@ def required_label(form_or_object, field, **options) # rubocop:todo Metrics/AbcS
label_tag(field, label_text.html_safe, options)
end
end
+ # rubocop:enable Metrics/PerceivedComplexity
# rubocop:enable Metrics/MethodLength
# rubocop:todo Metrics/MethodLength
diff --git a/app/javascript/controllers/better_together/form_validation_controller.js b/app/javascript/controllers/better_together/form_validation_controller.js
index 9fb35f340..1da210b5a 100644
--- a/app/javascript/controllers/better_together/form_validation_controller.js
+++ b/app/javascript/controllers/better_together/form_validation_controller.js
@@ -75,9 +75,11 @@ export default class extends Controller {
}
handleFormSubmit(event) {
- if (!this.element.checkValidity()) {
+ // Validate all fields (including trix editors) via checkAllFields
+ const allValid = this.checkAllFields();
+
+ if (!allValid) {
event.preventDefault();
- this.checkAllFields();
return;
}
@@ -107,46 +109,78 @@ export default class extends Controller {
}
checkAllFields() {
- const fields = this.element.querySelectorAll("input, select, textarea");
- fields.forEach(field => this.checkValidity({ target: field }));
+ // Include trix-editor elements so checkValidity routes appropriately
+ const fields = this.element.querySelectorAll("input, select, textarea, trix-editor");
+ let allValid = true;
+ fields.forEach(field => {
+ const valid = this.checkValidity({ target: field });
+ if (!valid) allValid = false;
+ });
+ return allValid;
}
checkValidity(event) {
const field = event.target;
- if (field.closest("trix-editor")) {
- return this.checkTrixValidity(event);
+ // If the target is a trix-editor itself, or it's the hidden input
+ // backing a trix-editor, route to the trix validity checker.
+ let trixEditorElem = null;
+ if (field && field.tagName && field.tagName.toLowerCase() === 'trix-editor') {
+ trixEditorElem = field;
+ } else if (field && field.tagName && field.tagName.toLowerCase() === 'input' && (field.type === 'hidden' || field.getAttribute('type') === 'hidden') && field.id) {
+ trixEditorElem = this.element.querySelector(`trix-editor[input="${field.id}"]`);
+ }
+
+ if (trixEditorElem) {
+ return this.checkTrixValidity({ target: trixEditorElem });
}
- if (field.checkValidity() && field.value.trim() === "") {
+ if (field.checkValidity && field.checkValidity() && field.value && field.value.trim() === "") {
field.classList.remove("is-valid", "is-invalid");
this.hideErrorMessage(field);
- } else if (field.checkValidity()) {
+ return true;
+ } else if (field.checkValidity && field.checkValidity()) {
field.classList.remove("is-invalid");
field.classList.add("is-valid");
this.hideErrorMessage(field);
+ return true;
} else {
- field.classList.add("is-invalid");
+ if (field.classList) field.classList.add("is-invalid");
this.showErrorMessage(field);
+ return false;
}
}
checkTrixValidity(event) {
const editor = event.target;
const field = editor.closest("trix-editor");
- const editorContent = editor.editor.getDocument().toString().trim();
+ const editorContent = (editor && editor.editor && typeof editor.editor.getDocument === 'function') ? editor.editor.getDocument().toString().trim() : (editor.textContent || '').trim();
+
+ // If the editor has no meaningful content, mark it invalid and show the
+ // associated .invalid-feedback so client-side validation blocks submit.
+ if (!editorContent || editorContent === "") {
+ if (field && field.classList) {
+ field.classList.remove("is-valid");
+ field.classList.add("is-invalid");
+ }
+ this.showErrorMessage(field);
+ return false;
+ }
- if (editorContent === "") {
- field.classList.remove("is-valid", "is-invalid");
- this.hideErrorMessage(field);
- } else if (editorContent.length > 0) {
- field.classList.remove("is-invalid");
- field.classList.add("is-valid");
+ // Non-empty content -> valid
+ if (editorContent.length > 0) {
+ if (field && field.classList) {
+ field.classList.remove("is-invalid");
+ field.classList.add("is-valid");
+ }
this.hideErrorMessage(field);
- } else {
- field.classList.add("is-invalid");
- this.showErrorMessage(field);
+ return true;
}
+
+ // Fallback: mark invalid
+ if (field && field.classList) field.classList.add("is-invalid");
+ this.showErrorMessage(field);
+ return false;
}
resetValidation() {
@@ -212,16 +246,25 @@ export default class extends Controller {
}
showErrorMessage(field) {
- const errorMessage = field.nextElementSibling;
- if (errorMessage?.classList.contains("invalid-feedback")) {
- errorMessage.style.display = "block";
+ // Trix editors may not place the invalid-feedback as the direct
+ // next sibling; search for a nearby .invalid-feedback first.
+ let errorMessage = field.nextElementSibling;
+ if (!errorMessage || !errorMessage.classList.contains('invalid-feedback')) {
+ // Try parent container
+ errorMessage = field.parentElement && field.parentElement.querySelector('.invalid-feedback');
+ }
+ if (errorMessage && errorMessage.classList.contains('invalid-feedback')) {
+ errorMessage.style.display = 'block';
}
}
hideErrorMessage(field) {
- const errorMessage = field.nextElementSibling;
- if (errorMessage?.classList.contains("invalid-feedback")) {
- errorMessage.style.display = "none";
+ let errorMessage = field.nextElementSibling;
+ if (!errorMessage || !errorMessage.classList.contains('invalid-feedback')) {
+ errorMessage = field.parentElement && field.parentElement.querySelector('.invalid-feedback');
+ }
+ if (errorMessage && errorMessage.classList.contains('invalid-feedback')) {
+ errorMessage.style.display = 'none';
}
}
}
diff --git a/app/javascript/controllers/better_together/message_visibility_controller.js b/app/javascript/controllers/better_together/message_visibility_controller.js
index b149f63df..50e41923a 100644
--- a/app/javascript/controllers/better_together/message_visibility_controller.js
+++ b/app/javascript/controllers/better_together/message_visibility_controller.js
@@ -20,7 +20,7 @@ export default class extends Controller {
entries.forEach(entry => {
if (entry.isIntersecting) {
const messageId = this.element.dataset.messageId;
- console.log(`Message ${messageId} is on screen.`);
+ // console.log(`Message ${messageId} is on screen.`);
if (this.element.dataset.readStatus === 'unread') { this.markAsRead(messageId); }
diff --git a/app/javascript/controllers/better_together/slim_select_controller.js b/app/javascript/controllers/better_together/slim_select_controller.js
index d7c57ea7f..a8bd7706d 100644
--- a/app/javascript/controllers/better_together/slim_select_controller.js
+++ b/app/javascript/controllers/better_together/slim_select_controller.js
@@ -58,6 +58,19 @@ export default class extends Controller {
select: this.element,
...options
});
+
+ // Ensure SlimSelect reflects any pre-selected options rendered by the server
+ // (useful when Turbo or server-side rendering supplies selected attributes)
+ try {
+ if (this.slimSelect && typeof this.slimSelect.set === 'function') {
+ // Pass current selected values from the underlying select
+ const selected = Array.from(this.element.selectedOptions).map(o => o.value);
+ this.slimSelect.set(selected);
+ }
+ } catch (e) {
+ // Fail silently - SlimSelect might not support set() in some versions
+ console.warn('Unable to refresh SlimSelect selected values:', e);
+ }
}
disconnect() {
diff --git a/app/models/better_together/conversation.rb b/app/models/better_together/conversation.rb
index c206087f8..14077646d 100644
--- a/app/models/better_together/conversation.rb
+++ b/app/models/better_together/conversation.rb
@@ -6,14 +6,80 @@ class Conversation < ApplicationRecord
encrypts :title, deterministic: true
belongs_to :creator, class_name: 'BetterTogether::Person'
has_many :messages, dependent: :destroy
+ accepts_nested_attributes_for :messages, allow_destroy: false
has_many :conversation_participants, dependent: :destroy
has_many :participants, through: :conversation_participants, source: :person
validate :at_least_one_participant
+ # Define permitted attributes for controller strong parameters
+ def self.permitted_attributes
+ [
+ :title,
+ { participant_ids: [] },
+ { messages_attributes: BetterTogether::Message.permitted_attributes }
+ ]
+ end
+
+ # Require participants on creation so the form helper `required_label`
+ # can detect required fields and the form-validation Stimulus controller
+ # will flag them client-side. Do NOT require a nested message on every
+ # conversation create: nested messages are optional unless a nested
+ # message was provided by the form. If a nested message is present, we
+ # validate its content below.
+ validates :participant_ids, presence: true, on: :create
+
+ # Provide a helper for the first message content so views/tests can
+ # access it easily and a custom validator can assert its presence
+ # (useful for nested attributes where Message validations may not yet
+ # surface on the parent object in some flows).
+ def first_message_content
+ first_msg = messages.first
+ return nil unless first_msg
+
+ if first_msg.respond_to?(:content) && first_msg.content.respond_to?(:to_plain_text)
+ first_msg.content.to_plain_text
+ else
+ first_msg.content
+ end
+ end
+
+ # Only validate the first nested message's content when a nested
+ # message actually exists (i.e., was provided via nested attributes or
+ # built in the controller). This avoids blocking conversation creation
+ # when no message is intended.
+ validate :first_message_content_present, on: :create
+
+ def first_message_content_present
+ return if messages.blank?
+
+ content = first_message_content
+ return unless content.nil? || content.to_s.strip.empty?
+
+ errors.add(:messages, :blank)
+ end
+
def to_s
title
end
+ # Safely add a person as a participant, retrying once if the person record
+ # raises an ActiveRecord::StaleObjectError due to an outdated lock_version.
+ # This centralizes optimistic-lock retry logic for participant additions.
+ def add_participant_safe(person)
+ return if person.nil?
+
+ attempts = 0
+ begin
+ participants << person unless participants.exists?(person.id)
+ rescue ActiveRecord::StaleObjectError
+ attempts += 1
+ raise unless attempts <= 1
+
+ person.reload
+ retry
+ end
+ end
+
private
def at_least_one_participant
diff --git a/app/models/better_together/message.rb b/app/models/better_together/message.rb
index ec3157fe3..660d3bf8e 100644
--- a/app/models/better_together/message.rb
+++ b/app/models/better_together/message.rb
@@ -12,6 +12,11 @@ class Message < ApplicationRecord
after_create_commit -> { broadcast_append_later_to conversation, target: 'conversation_messages' }
+ # Attributes permitted for strong parameters
+ def self.permitted_attributes
+ # include id and _destroy for nested attributes handling
+ %i[id content _destroy]
+ end
# def content
# super || self[:content]
# end
diff --git a/app/models/better_together/navigation_area.rb b/app/models/better_together/navigation_area.rb
index 8437fd27c..7821a6a77 100644
--- a/app/models/better_together/navigation_area.rb
+++ b/app/models/better_together/navigation_area.rb
@@ -47,5 +47,16 @@ def top_level_nav_items_includes_children
def to_s
name
end
+
+ def self.permitted_attributes(id: false, destroy: false)
+ # Allow core fields for creating/updating navigation areas
+ attrs = %i[
+ name
+ style
+ visible
+ ]
+
+ super + attrs
+ end
end
end
diff --git a/app/models/better_together/navigation_item.rb b/app/models/better_together/navigation_item.rb
index 49a6136db..8cb795658 100644
--- a/app/models/better_together/navigation_item.rb
+++ b/app/models/better_together/navigation_item.rb
@@ -196,6 +196,24 @@ def to_s
title
end
+ def self.permitted_attributes(id: false, destroy: false) # rubocop:todo Metrics/MethodLength
+ # Base attributes used when creating/updating navigation items
+ attrs = %i[
+ url
+ icon
+ position
+ visible
+ item_type
+ parent_id
+ route_name
+ linkable_type
+ linkable_id
+ navigation_area_id
+ ]
+
+ super + attrs
+ end
+
def url
fallback_url = "##{identifier}"
diff --git a/app/policies/better_together/conversation_policy.rb b/app/policies/better_together/conversation_policy.rb
index af45aa27f..bb93066f9 100644
--- a/app/policies/better_together/conversation_policy.rb
+++ b/app/policies/better_together/conversation_policy.rb
@@ -7,8 +7,19 @@ def index?
user.present?
end
- def create?
- user.present?
+ # Determines whether the current user can create a conversation.
+ # When `participants:` are provided, ensures they are within the permitted set.
+ def create?(participants: nil)
+ return false unless user.present?
+
+ return true if participants.nil?
+
+ permitted = permitted_participants
+ # Allow arrays of ids or Person records
+ Array(participants).all? do |p|
+ person_id = p.is_a?(::BetterTogether::Person) ? p.id : p
+ permitted.exists?(id: person_id)
+ end
end
def update?
@@ -30,10 +41,12 @@ def permitted_participants
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
+ # Include platform managers and any person who has explicitly opted in to receive messages
+ opted_in = BetterTogether::Person.where(
+ 'preferences @> ?', { receive_messages_from_members: true }.to_json
+ )
+
+ BetterTogether::Person.where(id: manager_ids).or(opted_in).distinct
end
end
diff --git a/app/views/better_together/conversations/_conversation_content.html.erb b/app/views/better_together/conversations/_conversation_content.html.erb
index 0391c7775..1720bf57f 100644
--- a/app/views/better_together/conversations/_conversation_content.html.erb
+++ b/app/views/better_together/conversations/_conversation_content.html.erb
@@ -7,7 +7,7 @@
<% if conversation.title.present? %>
-
+
<%= conversation.title %>
<% end %>
diff --git a/app/views/better_together/conversations/_form.html.erb b/app/views/better_together/conversations/_form.html.erb
index 9192cb121..986b773e8 100644
--- a/app/views/better_together/conversations/_form.html.erb
+++ b/app/views/better_together/conversations/_form.html.erb
@@ -1,20 +1,36 @@
<%= form_with(model: conversation, data: { turbo: false, controller: 'better_together--form-validation' }) do |form| %>
- <%= turbo_frame_tag 'form_errors' do %>
- <%= render 'layouts/better_together/errors', object: conversation %>
- <% end %>
+ <%= turbo_frame_tag 'form_errors' do %>
+ <%= render 'layouts/better_together/errors', object: conversation %>
+ <% end %>
+
+
+ <%= form.label :title %>
+ <%= form.text_field :title, class: 'form-control' %>
+ <%= t('.title_hint', default: 'Optional. A short descriptive title can help participants find this conversation later') %>
+
+ <%= required_label form, :participant_ids, label_text: t('.add_participants'), class: 'form-label' %>
+ <%= form.collection_select :participant_ids, available_participants, :id, :select_option_title, {}, { multiple: true, class: 'form-select', required: true, data: { controller: 'better_together--slim-select' } } %>
+ <%= t('.participants_hint', default: 'Select one or more people to include in the conversation') %>
+ This field is required
+