Merge branch 'main' into dependabot/bundler/aws-sdk-s3-1.198.0

Author: rsmithlal

.github/copilot-instructions.md

@@ -3,6 +3,7 @@ AllCops:
     - 'bin/*'
     - 'node_modules/**/*'
     - 'spec/dummy/db/schema.rb'
+    - 'db/schema.rb'
     - 'vendor/**/*'
   NewCops: enable
   TargetRubyVersion: 3.4

.rubocop.yml

@@ -12,26 +12,37 @@ Instructions for GitHub Copilot and other automated contributors working in this
 
 ## Setup
 - Environment runs a setup script that installs Ruby 3.4.4, Node 20, Postgres + PostGIS, and ES7, then prepares databases.
+- **Docker Environment**: All commands requiring database access must use `bin/dc-run` to execute within the containerized environment.
+- **Dummy App Commands**: Use `bin/dc-run-dummy` for Rails commands that need the dummy app context (e.g., `bin/dc-run-dummy rails console`, `bin/dc-run-dummy rails db:migrate`).
 - Databases:
   - development: `community_engine_development`
   - test: `community_engine_test`
 - Use `DATABASE_URL` to connect (overrides fallback host in `config/database.yml`).
 
 ## Commands
-- **Tests:** `bin/ci`
-  (Equivalent: `cd spec/dummy && bundle exec rspec`)
-- **Lint:** `bundle exec rubocop`
-- **Security:** `bundle exec brakeman --quiet --no-pager` and `bundle exec bundler-audit --update`
-- **Style:** `bin/codex_style_guard`
-- **I18n:** `bin/i18n [normalize|check|health|all]` (runs normalize + missing + interpolation checks by default)
+- **Tests:** `bin/dc-run bin/ci`
+  (Equivalent: `bin/dc-run bash -c "cd spec/dummy && bundle exec rspec"`)
+- **Running specific tests:** 
+  - Single spec file: `bin/dc-run bundle exec rspec spec/path/to/file_spec.rb`
+  - Specific line: `bin/dc-run bundle exec rspec spec/path/to/file_spec.rb:123`
+  - Multiple files: `bin/dc-run bundle exec rspec spec/file1_spec.rb spec/file2_spec.rb`
+  - Multiple specific lines: `bin/dc-run bundle exec rspec spec/file1_spec.rb:123 spec/file2_spec.rb:456`
+  - **Important**: RSpec does NOT support hyphenated line numbers (e.g., `spec/file_spec.rb:123-456` is INVALID)
+  - **Do NOT use `-v` flag**: The `-v` flag displays RSpec version information, NOT verbose output. Use `--format documentation` for detailed test descriptions.
+- **Rails Console:** `bin/dc-run-dummy rails console` (runs console in the dummy app context)
+- **Rails Commands in Dummy App:** `bin/dc-run-dummy rails [command]` for any Rails commands that need the dummy app environment
+- **Lint:** `bin/dc-run bundle exec rubocop`
+- **Security:** `bin/dc-run bundle exec brakeman --quiet --no-pager` and `bin/dc-run bundle exec bundler-audit --update`
+- **Style:** `bin/dc-run bin/codex_style_guard`
+- **I18n:** `bin/dc-run bin/i18n [normalize|check|health|all]` (runs normalize + missing + interpolation checks by default)
 - **Documentation:**
   - **Table of Contents**: [`docs/table_of_contents.md`](docs/table_of_contents.md) - Main documentation index
   - **Progress tracking**: `docs/scripts/update_progress.sh` - Update system completion status
   - **Diagram rendering**: `bin/render_diagrams` - Generate PNG/SVG from Mermaid sources
   - **Validation**: `docs/scripts/validate_documentation_tooling.sh` - Validate doc system integrity
 
 ## Security Requirements
-- **Run Brakeman before generating code**: `bundle exec brakeman --quiet --no-pager` 
+- **Run Brakeman before generating code**: `bin/dc-run bundle exec brakeman --quiet --no-pager` 
 - **Fix high-confidence vulnerabilities immediately** - never ignore security warnings with "High" confidence
 - **Review and address medium-confidence warnings** that are security-relevant
 - **Safe coding practices when generating code:**
@@ -41,11 +52,11 @@ Instructions for GitHub Copilot and other automated contributors working in this
   - Use strong parameters in controllers
   - 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 `bundle exec brakeman --quiet --no-pager -c UnsafeReflection,SQL,CrossSiteScripting` after major code changes
+- **Post-generation security check**: Run `bin/dc-run bundle exec brakeman --quiet --no-pager -c UnsafeReflection,SQL,CrossSiteScripting` after major code changes
 
 ## Conventions
 - Make incremental changes with passing tests.
-- **Security first**: Run `bundle exec brakeman --quiet --no-pager` before committing code changes.
+- **Security first**: Run `bin/dc-run bundle exec brakeman --quiet --no-pager` before committing code changes.
 - **Test every change**: Generate RSpec tests for all code modifications, including models, controllers, mailers, jobs, and JavaScript.
 - **Test coverage requirements**: All new features, bug fixes, and refactors must include comprehensive test coverage.
 - Avoid introducing new external services in tests; stub where possible.
@@ -150,14 +161,14 @@ We use the `i18n-tasks` gem to ensure all translation keys are present, normaliz
 
 ## Example Commands
 ```bash
-i18n-tasks normalize
-i18n-tasks missing
-i18n-tasks add-missing
-i18n-tasks health
+bin/dc-run i18n-tasks normalize
+bin/dc-run i18n-tasks missing
+bin/dc-run i18n-tasks add-missing
+bin/dc-run i18n-tasks health
 ```
 
 ## CI Note
-- The i18n GitHub Action installs dev/test gem groups to make `i18n-tasks` available. Locally, you can mirror CI with `bin/i18n`, which sets `BUNDLE_WITH=development:test` automatically.
+- The i18n GitHub Action installs dev/test gem groups to make `i18n-tasks` available. Locally, you can mirror CI with `bin/dc-run bin/i18n`, which sets `BUNDLE_WITH=development:test` automatically.
 
 See `.github/instructions/i18n-mobility.instructions.md` for additional translation rules.
 
@@ -199,6 +210,22 @@ For every implementation plan, create acceptance criteria covering relevant stak
 - **Test all layers**: models (validations, associations, methods), controllers (actions, authorization), services, mailers, jobs, and view components.
 - **JavaScript/Stimulus testing**: Include feature specs that exercise dynamic behaviors like form interactions and AJAX updates.
 
+## Test Environment Requirements
+- **Host Platform Configuration**: All controller, request, and feature tests MUST configure the host platform/community before testing.
+- **Use `configure_host_platform`**: Call this helper method in a `before` block for any test that makes HTTP requests or tests authentication/authorization.
+- **DeviseSessionHelpers**: Include this module and use authentication helpers like `login('[email protected]', 'password')` for authenticated tests.
+- **Platform Setup Pattern**:
+  ```ruby
+  RSpec.describe BetterTogether::SomeController do
+    before do
+      configure_host_platform  # Creates host platform with community
+      login('[email protected]', 'password')  # For authenticated tests
+    end
+  end
+  ```
+- **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.
+
 ## 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.
@@ -309,3 +336,70 @@ Each major system must include:
 - Security implications and access controls
 - API endpoints with request/response examples
 - Monitoring tools and troubleshooting procedures
+
+## Testing Architecture Consistency Lessons Learned
+
+### Critical Testing Pattern: Request Specs vs Controller Specs
+- **Project Standard**: All tests use request specs (`type: :request`) for consistency with Rails engine routing
+- **Exception Handling**: Controller specs (`type: :controller`) require special URL helper configuration in Rails engines
+- **Why This Matters**: Request specs handle Rails engine routing automatically through the full HTTP stack, while controller specs test in isolation and need explicit configuration
+- **Debugging Indicator**: If you see `default_url_options` errors only in one spec while others pass, check if it's a controller spec in a request spec codebase
+
+### Rails Engine URL Helper Configuration
+- **Problem**: Controller specs in Rails engines throw `default_url_options` errors that request specs don't encounter
+- **Root Cause**: Engines need special URL helper setup for controller specs but not request specs
+- **Solution Patterns**:
+  ```ruby
+  # For controller spec assertions, use pattern matching instead of path helpers:
+  expect(response.location).to include('/person_blocks') # Good
+  expect(response).to redirect_to(person_blocks_path) # Problematic in controller specs
+  
+  # Ensure consistent route naming throughout:
+  # Controller: person_blocks_path (not blocks_path)
+  # Views: <%= link_to "Block", better_together.person_blocks_path %>
+  # Tests: params path should match controller actions
+  ```
+
+### Route Naming Convention Enforcement
+- **Pattern**: Engine routes follow full resource naming: `better_together.resource_name_path`
+- **Common Error**: Using shortened path names (`blocks_path`) instead of full names (`person_blocks_path`)
+- **Consistency Check**: Views, controllers, and tests must all use the same complete path helper names
+- **Verification**: Check all three layers when debugging routing issues
+
+### Factory and Association Dependencies
+- **Requirement**: Every Better Together model needs a corresponding FactoryBot factory
+- **Naming Convention**: Factory names follow `better_together_model_name` pattern with aliases
+- **Association Setup**: Factories must properly handle engine namespace associations
+- **Missing Factory Indicator**: Tests failing on association creation often indicate missing factories
+
+### Test Environment Configuration Enforcement
+- **Critical Setup**: `configure_host_platform` must be called before any controller/request/feature tests
+- **Why Required**: Better Together engine needs host platform setup for authentication and authorization
+- **Pattern Recognition**: Tests failing with authentication/authorization errors often need this setup
+- **Documentation Reference**: This pattern is well-documented but bears reinforcement
+
+### Architecture Consistency Principles
+- **Consistency Is Key**: When one component (PersonBlocksController) differs from project patterns, it requires special handling
+- **Pattern Detection**: Single anomalies (one controller spec among many request specs) signal architectural inconsistencies
+- **Prevention**: New tests should follow the established pattern (request specs) unless there's a compelling reason for exceptions
+- **Documentation**: When exceptions are necessary, document why they exist and how to handle their special requirements
+
+### Testing Strategy Recommendations
+- **Default Choice**: Use request specs for new controller tests to maintain consistency
+- **Engine Compatibility**: Request specs handle Rails engine complexity automatically
+- **Special Cases**: If controller specs are needed, prepare for URL helper configuration complexity
+- **Debugging Approach**: When testing errors occur in only one spec, compare its type and setup to working specs
+
+## Docker Environment Usage
+- **All database-dependent commands must use `bin/dc-run`**: This includes tests, generators, and any command that connects to PostgreSQL, Redis, or Elasticsearch
+- **Dummy app commands use `bin/dc-run-dummy`**: For Rails commands that need the dummy app context (console, migrations specific to dummy app)
+- **Examples of commands requiring `bin/dc-run`**:
+  - Tests: `bin/dc-run bundle exec rspec`
+  - Generators: `bin/dc-run rails generate model User`
+  - Brakeman: `bin/dc-run bundle exec brakeman`
+  - RuboCop: `bin/dc-run bundle exec rubocop`
+- **Examples of commands requiring `bin/dc-run-dummy`**:
+  - Rails console: `bin/dc-run-dummy rails console`
+  - Dummy app migrations: `bin/dc-run-dummy rails db:migrate`
+  - Dummy app database operations: `bin/dc-run-dummy rails db:seed`
+- **Commands that don't require bin/dc-run**: File operations, documentation generation (unless database access needed), static analysis tools that don't connect to services

AGENTS.md

@@ -105,6 +105,8 @@ group :test do
   gem 'database_cleaner-active_record'
   # # Easy installation and use of chromedriver to run system tests with Chrome
   # gem 'webdrivers'
+  # Rails controller testing for assigns method
+  gem 'rails-controller-testing'
   # RuboCop RSpec for RSpec-specific code analysis
   gem 'rubocop-capybara'
   gem 'rubocop-factory_bot'

Gemfile

@@ -562,6 +562,10 @@ GEM
       activesupport (= 7.1.5.2)
       bundler (>= 1.15.0)
       railties (= 7.1.5.2)
+    rails-controller-testing (1.0.5)
+      actionpack (>= 5.0.1.rc1)
+      actionview (>= 5.0.1.rc1)
+      activesupport (>= 5.0.1.rc1)
     rails-dom-testing (2.3.0)
       activesupport (>= 5.0.0)
       minitest
@@ -848,6 +852,7 @@ DEPENDENCIES
   rack-mini-profiler
   rack-protection
   rails (= 7.1.5.2)
+  rails-controller-testing
   rb-readline
   rbtrace
   redis (~> 5.4)

Gemfile.lock

@@ -117,3 +117,12 @@ Run the RSpec tests:
 ```bash
 bin/dc-run app rspec
 ```
+
+## Contributing
+
+We welcome contributions from the community.
+
+- Guidelines: See [CONTRIBUTING.md](CONTRIBUTING.md) for how to report issues, propose changes, and submit pull requests.
+- Code of Conduct: See [CODE_OF_CONDUCT.md](CODE_OF_CONDUCT.md) for expectations of behavior in our community.
+
+Thank you for helping make Better Together better for everyone.

README.md

@@ -5,6 +5,10 @@
   font-weight: bold;
 }
 
+.ss-content .ss-search input {
+  color: $light-background-text-color;
+}
+
 .form-label {
   font-weight: 500;
 }

app/assets/stylesheets/better_together/forms.scss

@@ -16,23 +16,44 @@ def seed_data
       end
 
       def build_community_roles
-        ::BetterTogether::Role.create!(community_role_attrs)
+        community_role_attrs.each do |attrs|
+          # Idempotent: find by unique identifier and update attributes if needed
+          role = ::BetterTogether::Role.find_or_initialize_by(identifier: attrs[:identifier])
+          role.assign_attributes(attrs)
+          role.save! if role.changed?
+        end
       end
 
       def build_community_resource_permissions
-        ::BetterTogether::ResourcePermission.create!(community_resource_permission_attrs)
+        community_resource_permission_attrs.each do |attrs|
+          perm = ::BetterTogether::ResourcePermission.find_or_initialize_by(identifier: attrs[:identifier])
+          perm.assign_attributes(attrs)
+          perm.save! if perm.changed?
+        end
       end
 
       def build_platform_resource_permissions
-        ::BetterTogether::ResourcePermission.create!(platform_resource_permission_attrs)
+        platform_resource_permission_attrs.each do |attrs|
+          perm = ::BetterTogether::ResourcePermission.find_or_initialize_by(identifier: attrs[:identifier])
+          perm.assign_attributes(attrs)
+          perm.save! if perm.changed?
+        end
       end
 
       def build_platform_roles
-        ::BetterTogether::Role.create!(platform_role_attrs)
+        platform_role_attrs.each do |attrs|
+          role = ::BetterTogether::Role.find_or_initialize_by(identifier: attrs[:identifier])
+          role.assign_attributes(attrs)
+          role.save! if role.changed?
+        end
       end
 
       def build_person_resource_permissions
-        ::BetterTogether::ResourcePermission.create!(person_resource_permission_attrs)
+        person_resource_permission_attrs.each do |attrs|
+          perm = ::BetterTogether::ResourcePermission.find_or_initialize_by(identifier: attrs[:identifier])
+          perm.assign_attributes(attrs)
+          perm.save! if perm.changed?
+        end
       end
 
       # Clear existing data - Use with caution!

app/builders/better_together/access_control_builder.rb

@@ -35,8 +35,7 @@ def build_privacy_policy # rubocop:todo Metrics/MethodLength, Metrics/AbcSize
 
         # If a Page exists for the privacy policy, link it so the page content
         # is shown to users instead of the agreement terms.
-        page = BetterTogether::Page.find_by(identifier: 'privacy_policy') ||
-               BetterTogether::Page.friendly.find('privacy-policy')
+        page = BetterTogether::Page.find_by(identifier: 'privacy_policy')
         agreement.update!(page: page) if page.present?
       end
       # rubocop:enable Metrics/AbcSize
@@ -57,8 +56,7 @@ def build_terms_of_service # rubocop:todo Metrics/MethodLength, Metrics/AbcSize
         end
 
         # Link a Terms of Service Page if one exists
-        page = BetterTogether::Page.find_by(identifier: 'terms_of_service') ||
-               BetterTogether::Page.friendly.find('terms-of-service')
+        page = BetterTogether::Page.find_by(identifier: 'terms_of_service')
         agreement.update!(page: page) if page.present?
       end
       # rubocop:enable Metrics/AbcSize
@@ -79,8 +77,7 @@ def build_code_of_conduct # rubocop:todo Metrics/AbcSize, Metrics/MethodLength
         end
 
         # Link a Code of Conduct Page if one exists
-        page = BetterTogether::Page.find_by(identifier: 'code_of_conduct') ||
-               BetterTogether::Page.friendly.find('code-of-conduct')
+        page = BetterTogether::Page.find_by(identifier: 'code_of_conduct')
         agreement.update!(page: page) if page.present?
       end
       # rubocop:enable Metrics/MethodLength

app/builders/better_together/agreement_builder.rb

@@ -352,7 +352,9 @@ def delete_navigation_areas
       end
 
       def delete_navigation_items
-        ::BetterTogether::NavigationItem.delete_all
+        # Delete children first to satisfy FK constraints, then parents
+        ::BetterTogether::NavigationItem.where.not(parent_id: nil).delete_all
+        ::BetterTogether::NavigationItem.where(parent_id: nil).delete_all
       end
     end
   end