From eacc8195a38f543996703724b4873a99438c2a4f Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Sat, 23 Aug 2025 18:06:52 -0230 Subject: [PATCH 01/77] Add RSpec command usage instructions to documentation --- .github/copilot-instructions.md | 7 +++++++ AGENTS.md | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 5dc1a8604..46468ea6a 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -201,6 +201,13 @@ This repository contains the **Better Together Community Engine** (an isolated R - Ensure blobs are encrypted at rest - **Testing** - RSpec (if present) or Minitest – follow existing test framework + - **RSpec command usage**: + - Single spec file: `bundle exec rspec spec/path/to/file_spec.rb` + - Specific line: `bundle exec rspec spec/path/to/file_spec.rb:123` + - Multiple files: `bundle exec rspec spec/file1_spec.rb spec/file2_spec.rb` + - Multiple specific lines: `bundle exec rspec spec/file1_spec.rb:123 spec/file2_spec.rb:456` + - **IMPORTANT**: RSpec does NOT support hyphenated line ranges (e.g., `spec/file_spec.rb:123-456` is INVALID) + - Use individual line numbers or run the full spec file instead - **Test-Driven Development (TDD) Required**: Use stakeholder-focused TDD approach for all features - **Define acceptance criteria first**: Before writing code, define stakeholder acceptance criteria using `docs/tdd_acceptance_criteria_template.md` as template - **Red-Green-Refactor cycle**: Write failing tests first (RED), implement minimal code to pass (GREEN), refactor while maintaining tests (REFACTOR) diff --git a/AGENTS.md b/AGENTS.md index 749a5e217..574e71e3f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,6 +20,12 @@ Instructions for GitHub Copilot and other automated contributors working in this ## Commands - **Tests:** `bin/ci` (Equivalent: `cd spec/dummy && bundle exec rspec`) +- **Running specific tests:** + - Single spec file: `bundle exec rspec spec/path/to/file_spec.rb` + - Specific line: `bundle exec rspec spec/path/to/file_spec.rb:123` + - Multiple files: `bundle exec rspec spec/file1_spec.rb spec/file2_spec.rb` + - Multiple specific lines: `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) - **Lint:** `bundle exec rubocop` - **Security:** `bundle exec brakeman --quiet --no-pager` and `bundle exec bundler-audit --update` - **Style:** `bin/codex_style_guard` From 1ca76946061c08137880eb95c07df189b9a7cf7d Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Sat, 23 Aug 2025 20:16:12 -0230 Subject: [PATCH 02/77] Implement Block Management Interface with Search and Unblock Features - Added `rails-controller-testing` gem for enhanced controller testing. - Updated `PersonBlocksController` to include search functionality for blocked users and display blocking timestamps. - Created views for managing blocked users, including index and new block forms. - Implemented AJAX responses for blocking and unblocking users with Turbo Streams. - Enhanced policies to manage user permissions for blocking actions. - Added acceptance criteria documentation for the Block Management Interface. - Developed comprehensive controller specs to ensure functionality and edge cases are covered. --- .github/copilot-instructions.md | 243 ++++------------ AGENTS.md | 123 ++++++-- Gemfile | 2 + Gemfile.lock | 5 + .../person_blocks_controller.rb | 101 ++++++- .../better_together/person_block_policy.rb | 4 + app/policies/better_together/person_policy.rb | 92 +++++- .../person_blocks/_index_content.html.erb | 46 +++ .../person_blocks/index.html.erb | 49 +++- .../person_blocks/new.html.erb | 34 +++ config/locales/en.yml | 30 ++ config/routes.rb | 2 +- ...anagement_interface_acceptance_criteria.md | 155 ++++++++++ .../person_blocks_controller_spec.rb | 271 ++++++++++++++++++ .../person_platform_memberships.rb | 14 + spec/rails_helper.rb | 1 + 16 files changed, 947 insertions(+), 225 deletions(-) create mode 100644 app/views/better_together/person_blocks/_index_content.html.erb create mode 100644 app/views/better_together/person_blocks/new.html.erb create mode 100644 docs/implementation/current_plans/block_management_interface_acceptance_criteria.md create mode 100644 spec/controllers/better_together/person_blocks_controller_spec.rb create mode 100644 spec/factories/better_together/person_platform_memberships.rb diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 46468ea6a..dc1dcde3b 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -4,7 +4,7 @@ This repository contains the **Better Together Community Engine** (an isolated R ## Core Principles -- **Security first**: Run `bundle exec brakeman --quiet --no-pager` before generating code; fix high-confidence vulnerabilities +- **Security first**: Run `bin/dc-run bundle exec brakeman --quiet --no-pager` before generating code; fix high-confidence vulnerabilities - **Accessibility first** (WCAG AA/AAA): semantic HTML, ARIA roles, keyboard nav, proper contrast. - **Hotwire everywhere**: Turbo for navigation/updates; Stimulus controllers for interactivity. - **Keep controllers thin**; move business logic to POROs/service objects or concerns. @@ -29,6 +29,7 @@ This repository contains the **Better Together Community Engine** (an isolated R - **Mobility** for attribute translations - **Elasticsearch 7** via `elasticsearch-rails` - **Importmap-Rails** for JS deps (no bundler by default) +- **Docker** containerized development environment - use `bin/dc-run` for all database-dependent commands - **Dokku** (Docker-based PaaS) for deployment; Cloudflare for DNS/DDoS/tunnels - **AWS S3 / MinIO** for file storage (transitioning to self-hosted MinIO) - **Action Mailer + locale-aware emails** @@ -91,8 +92,22 @@ This repository contains the **Better Together Community Engine** (an isolated R ## Coding Guidelines +### 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 + ### 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:** @@ -104,7 +119,7 @@ This repository contains the **Better Together Community Engine** (an isolated R - **SQL injection prevention**: Use parameterized queries, avoid string interpolation in SQL - **XSS prevention**: Use Rails auto-escaping, sanitize HTML inputs with allowlists - **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 ### String Enum Design Standards - **Always use string enums** for human-readable accessibility when reviewing database entries @@ -169,185 +184,43 @@ This repository contains the **Better Together Community Engine** (an isolated R ``` ## Test Environment Setup -- Configure the host Platform in a before block for controller/request/feature tests. - - Create/set a Platform as host (with community) before requests. - - Toggle requires_invitation and provide invitation_code when needed. - -- **Ruby/Rails** - - 2-space indent, snake_case methods, Rails conventions - - Service objects in `app/services/` - - Concerns for reusable model/controller logic - - Strong params, Pundit/Policy checks (or equivalent) everywhere - - Avoid fat callbacks; keep models lean - - **String enums only**: Always use human-readable string enums following the existing full-word pattern (avg ~7 chars) -- **Views** - - ERB with semantic HTML - - Bootstrap utility classes; respect prefers-reduced-motion & other a11y prefs - - Avoid inline JS; use Stimulus - - External links in `.trix-content` get FA external-link icon unless internal/mailto/tel/pdf - - All user-facing copy must use t("...") and include keys across all locales (add to config/locales/en.yml, es.yml, fr.yml). -- **Hotwire** - - Use Turbo Streams for CRUD updates - - Stimulus controllers in `app/javascript/controllers/` - - No direct DOM manipulation without Stimulus targets/actions -- **Background Jobs** - - Sidekiq jobs under appropriate queues (`:default`, `:mailers`, `:metrics`, etc.) - - Idempotent job design; handle retries - - When generating emails/notifications, localize both subject and body for all locales. -- **Search** - - Update `as_indexed_json` to include translated/plain-text fields as needed -- **Encryption & Privacy** - - Use AR encryption for sensitive columns - - Ensure blobs are encrypted at rest -- **Testing** - - RSpec (if present) or Minitest – follow existing test framework - - **RSpec command usage**: - - Single spec file: `bundle exec rspec spec/path/to/file_spec.rb` - - Specific line: `bundle exec rspec spec/path/to/file_spec.rb:123` - - Multiple files: `bundle exec rspec spec/file1_spec.rb spec/file2_spec.rb` - - Multiple specific lines: `bundle exec rspec spec/file1_spec.rb:123 spec/file2_spec.rb:456` - - **IMPORTANT**: RSpec does NOT support hyphenated line ranges (e.g., `spec/file_spec.rb:123-456` is INVALID) - - Use individual line numbers or run the full spec file instead - - **Test-Driven Development (TDD) Required**: Use stakeholder-focused TDD approach for all features - - **Define acceptance criteria first**: Before writing code, define stakeholder acceptance criteria using `docs/tdd_acceptance_criteria_template.md` as template - - **Red-Green-Refactor cycle**: Write failing tests first (RED), implement minimal code to pass (GREEN), refactor while maintaining tests (REFACTOR) - - **Stakeholder validation**: Validate acceptance criteria with relevant stakeholders (End Users, Community Organizers, Platform Organizers, etc.) - - **Generate comprehensive test coverage for all changes**: Every modification must include RSpec tests covering the new functionality - - All RSpec specs **must use FactoryBot factories** for model instances (do not use `Model.create` or `Model.new` directly in specs). - - **A FactoryBot factory must exist for every model**. When generating a new model, also generate a factory for it. - - **Factories must use the Faker gem** to provide realistic, varied test data for all attributes (e.g., names, emails, addresses, etc.). - - **Test all layers**: models, controllers, mailers, jobs, JavaScript/Stimulus controllers, and integration workflows - - **Feature tests for stakeholder workflows**: End-to-end tests that validate complete stakeholder journeys - - System tests for Turbo flows where possible - - **Session-based testing**: When working on existing code modifications, generate tests that cover all unstaged changes and related functionality - -## Test-Driven Development (TDD) Implementation Process - -### Implementation Plan to Acceptance Criteria Workflow -1. **Receive Confirmed Implementation Plan**: Start with an implementation plan that has completed collaborative review -2. **Generate Acceptance Criteria**: Use `docs/tdd_acceptance_criteria_template.md` to transform the implementation plan into stakeholder-focused acceptance criteria -3. **Identify Stakeholders**: Determine which stakeholders are affected (End Users, Community Organizers, Platform Organizers, Content Moderators, etc.) -4. **Create Testable Criteria**: Write specific criteria using "As a [stakeholder], I want [capability] so that [benefit]" format -5. **Structure Test Coverage**: Define test matrix showing which test types validate which acceptance criteria -6. **Follow Red-Green-Refactor**: Implement each acceptance criteria with TDD cycle -7. **Stakeholder Validation**: Demo completed feature and validate acceptance criteria fulfillment - -### Acceptance Criteria Creation Process -When responding to an implementation plan: -1. **Reference Implementation Plan**: Confirm the plan document and collaborative review completion status -2. **Analyze Stakeholder Impact**: Identify primary and secondary stakeholders affected by the feature -3. **Generate Acceptance Criteria Document**: Create new document using the acceptance criteria template -4. **Define Test Structure**: Specify which test types (model, controller, feature, integration) validate each criteria -5. **Create Implementation Sequence**: Plan Red-Green-Refactor cycles for systematic development - -### TDD Test Categories by Stakeholder -- **End User Tests**: Feature specs validating user experience, safety controls, and interface interactions -- **Community Organizer Tests**: Controller and feature specs validating community management capabilities -- **Platform Organizer Tests**: Integration specs validating platform-wide oversight and configuration -- **Content Moderator Tests**: Controller specs validating moderation tools and workflows -- **Cross-Stakeholder Tests**: Integration specs validating workflows spanning multiple stakeholder types - -### Test Generation Strategy - -### Mandatory Test Creation -When modifying existing code or adding new features, always generate RSpec tests that provide comprehensive coverage: - -1. **Stakeholder Acceptance Tests**: - - Feature tests validating complete stakeholder workflows - - Integration tests covering cross-stakeholder interactions - - Error handling tests for stakeholder edge cases - - Security tests validating stakeholder authorization - -2. **Model Tests**: - - Validations, associations, scopes, callbacks - - Instance methods, class methods, delegations - - Business logic and calculated attributes - - Security-related functionality (encryption, authorization) - -3. **Controller Tests**: - - All CRUD actions and custom endpoints - - Authorization policy checks (Pundit/equivalent) - - Parameter handling and strong params - - Response formats (HTML, JSON, Turbo Stream) - - Error handling and edge cases - -4. **Background Job Tests**: - - Job execution and success scenarios - - Retry logic and error handling - - Side effects and state changes - - Queue assignment and timing - -4. **Mailer Tests**: - - Email content and formatting - - Recipient handling and localization - - Attachment and delivery configurations - - Multi-locale support - -5. **JavaScript/Stimulus Tests**: - - Controller initialization and teardown - - User interaction handlers - - Form state management and dynamic updates - - Target and action mappings - -6. **Integration Tests**: - - Complete user workflows - - Cross-model interactions - - End-to-end feature functionality - - Authentication and authorization flows - -### Session-Specific Test Coverage -For this codebase, ensure tests cover all recent changes including: -- Enhanced LocatableLocation model with polymorphic associations -- Event model with notification callbacks and location integration -- Calendar and CalendarEntry associations -- Event notification system (EventReminderNotifier, EventUpdateNotifier) -- Background jobs for event reminders and scheduling -- EventMailer with localized content -- Dynamic location selector JavaScript controller -- Form enhancements with location type selection - -### Test Quality Standards -- Use descriptive test names that explain the expected behavior -- Follow AAA pattern (Arrange, Act, Assert) in test structure -- Mock external dependencies and network calls -- Test both success and failure scenarios -- Use shared examples for common behavior patterns -- Ensure tests are deterministic and can run independently - -## Project Architecture Notes - -- Engine code is namespaced under `BetterTogether`. -- Host app extends/overrides engine components where needed. -- Content blocks & page builder use configurable relationships (content areas, background images, etc.). -- Journey/Lists features use polymorphic items but with care (or explicit join models). -- Agreements system models participants, roles, terms, and timelines. - -## Specialized Instruction Files - -- `.github/instructions/rails_engine.instructions.md` – Engine isolation & namespacing -- `.github/instructions/hotwire.instructions.md` – Turbo/Stimulus patterns -- `.github/instructions/hotwire-native.instructions.md` – Hotwire Native patterns -- `.github/instructions/sidekiq-redis.instructions.md` – Background jobs & Redis -- `.github/instructions/search-elasticsearch.instructions.md` – Elasticsearch indexing patterns -- `.github/instructions/i18n-mobility.instructions.md` – Translations (Mobility + I18n) -- `.github/instructions/accessibility.instructions.md` – A11y checklist & patterns -- `.github/instructions/notifications-noticed.instructions.md` – Notification patterns -- `.github/instructions/deployment.instructions.md` – Dokku, Cloudflare, backups (S3/MinIO) -- `.github/instructions/security-encryption.instructions.md` – AR encryption, secrets -- `.github/instructions/bootstrap.instructions.md` – Styling, theming, icon usage -- `.github/instructions/importmaps.instructions.md` – JS dependency management -- `.github/instructions/view-helpers.instructions.md` – Consistency in Rails views - ---- - -_If you generate code that touches any of these areas, consult the relevant instruction file and follow it._ - -## Internationalization & Translation Normalization -- Use the `i18n-tasks` gem to: - - Normalize locale files (`i18n-tasks normalize`). - - Identify and add missing keys (`i18n-tasks missing`, `i18n-tasks add-missing`). - - Ensure all user-facing strings are present in all supported locales (en, fr, es, etc.). - - Add new keys in English first, then translate. - - Review translation health regularly (`i18n-tasks health`). -- All new/changed strings must be checked with `i18n-tasks` before merging. -- See `.github/instructions/i18n-mobility.instructions.md` for details. +- **CRITICAL**: Configure the host Platform in a before block for ALL controller/request/feature tests. + - **Use `configure_host_platform`**: Call this helper method which creates/sets a Platform as host (with community) before HTTP requests. + - **Include DeviseSessionHelpers**: Use authentication helpers like `login('user@example.com', 'password')` for authenticated tests. + - **Required Pattern**: + ```ruby + RSpec.describe BetterTogether::SomeController, type: :controller do + include DeviseSessionHelpers + routes { BetterTogether::Engine.routes } + + before do + configure_host_platform # Creates host platform with community + login('user@example.com', 'password') # For authenticated actions + end + end + ``` + - **Engine Routing**: Engine controller tests require `routes { BetterTogether::Engine.routes }` directive. + - **Locale Parameters**: Include `locale: I18n.default_locale` in params for engine routes due to routing constraints. + - **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. + +### 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 +- **Controller Specs Issues**: Require special URL helper configuration in Rails engines and should be avoided +- **Architectural Consistency**: The project follows request spec patterns throughout - maintain this consistency +- **Route Naming Convention**: All engine routes use full resource naming (e.g., `person_blocks_path`, not `blocks_path`) +- **URL Helper Debugging**: If you encounter `default_url_options` errors in a spec while others pass, check if it's a controller spec that should be converted to a request spec + +### Rails Engine Testing Patterns +- **Standard Pattern**: Use request specs for testing engine controllers +- **Path Helpers**: Always use complete, properly namespaced path helpers (`better_together.resource_name_path`) +- **Response Assertions**: For redirects, use pattern matching instead of path helpers in specs: + ```ruby + # Preferred in specs + expect(response.location).to include('/person_blocks') + + # Avoid in controller specs (problematic with engines) + expect(response).to redirect_to(person_blocks_path) + ``` +- **Factory Requirements**: Every Better Together model needs a corresponding FactoryBot factory with proper engine namespace handling diff --git a/AGENTS.md b/AGENTS.md index 574e71e3f..92fdca6ff 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,24 +12,28 @@ 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`) +- **Tests:** `bin/dc-run bin/ci` + (Equivalent: `bin/dc-run bash -c "cd spec/dummy && bundle exec rspec"`) - **Running specific tests:** - - Single spec file: `bundle exec rspec spec/path/to/file_spec.rb` - - Specific line: `bundle exec rspec spec/path/to/file_spec.rb:123` - - Multiple files: `bundle exec rspec spec/file1_spec.rb spec/file2_spec.rb` - - Multiple specific lines: `bundle exec rspec spec/file1_spec.rb:123 spec/file2_spec.rb:456` + - 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) -- **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) +- **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 @@ -37,7 +41,7 @@ Instructions for GitHub Copilot and other automated contributors working in this - **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:** @@ -47,11 +51,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. @@ -156,14 +160,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. @@ -205,6 +209,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('user@example.com', 'password')` for authenticated tests. +- **Platform Setup Pattern**: + ```ruby + RSpec.describe BetterTogether::SomeController do + before do + configure_host_platform # Creates host platform with community + login('user@example.com', '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. @@ -315,3 +335,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 diff --git a/Gemfile b/Gemfile index e886ddb8b..59548c042 100644 --- a/Gemfile +++ b/Gemfile @@ -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' diff --git a/Gemfile.lock b/Gemfile.lock index 3ffb09ed8..42124d149 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -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 @@ -846,6 +850,7 @@ DEPENDENCIES rack-mini-profiler rack-protection rails (= 7.1.5.2) + rails-controller-testing rb-readline rbtrace redis (~> 5.4) diff --git a/app/controllers/better_together/person_blocks_controller.rb b/app/controllers/better_together/person_blocks_controller.rb index 219b5dd84..821ac82b5 100644 --- a/app/controllers/better_together/person_blocks_controller.rb +++ b/app/controllers/better_together/person_blocks_controller.rb @@ -2,39 +2,116 @@ module BetterTogether class PersonBlocksController < ApplicationController # rubocop:todo Style/Documentation + before_action :authenticate_user! before_action :set_person_block, only: :destroy after_action :verify_authorized - def index + def index # rubocop:todo Metrics/AbcSize, Metrics/MethodLength authorize PersonBlock - @blocked_people = current_person.blocked_people + + # AC-2.11: I can search through my blocked users by name + @blocked_people = helpers.current_person.blocked_people + if params[:search].present? + # Search by translated name using includes and references + # Apply policy scope to ensure only authorized people are searchable + search_term = params[:search].strip + authorized_person_ids = policy_scope(BetterTogether::Person).pluck(:id) + + @blocked_people = @blocked_people.where(id: authorized_person_ids) + .includes(:string_translations) + .references(:string_translations) + .where(string_translations: { key: 'name' }) + .where('string_translations.value ILIKE ?', "%#{search_term}%") + .distinct + end + + # AC-2.12: I can see when I blocked each user (provide person_blocks for timestamp info) + @person_blocks = helpers.current_person.person_blocks.includes(:blocked) + if params[:search].present? + # Filter person_blocks by matching blocked person names + # Apply policy scope to ensure only authorized people are searchable + search_term = params[:search].strip + authorized_person_ids = policy_scope(BetterTogether::Person).pluck(:id) + + @person_blocks = @person_blocks.joins(:blocked) + .where(better_together_people: { id: authorized_person_ids }) + .includes(blocked: :string_translations) + .references(:string_translations) + .where(string_translations: { key: 'name' }) + .where('string_translations.value ILIKE ?', "%#{search_term}%") + .distinct + end + + # AC-2.15: I can see how many users I have blocked + @blocked_count = @blocked_people.count end - def create - @person_block = current_person.person_blocks.new(person_block_params) - authorize @person_block + def new + authorize PersonBlock + @person_block = helpers.current_person.person_blocks.build + end - if @person_block.save - redirect_to blocks_path, notice: t('flash.person_block.blocked') - else - redirect_to blocks_path, alert: @person_block.errors.full_messages.to_sentence + def create # rubocop:todo Metrics/AbcSize, Metrics/MethodLength + # rubocop:todo Layout/IndentationWidth + @person_block = helpers.current_person.person_blocks.new(person_block_params) + # rubocop:enable Layout/IndentationWidth + + # AC-2.13: I can block a user by entering their username or email + if params[:person_block][:blocked_identifier].present? # rubocop:todo Layout/IndentationConsistency + target_person = BetterTogether::Person.find_by(identifier: params[:person_block][:blocked_identifier]) || + # rubocop:todo Layout/LineLength + BetterTogether::Person.joins(:user).find_by(better_together_users: { email: params[:person_block][:blocked_identifier] }) + # rubocop:enable Layout/LineLength + @person_block.blocked = target_person if target_person + end + + authorize @person_block # rubocop:todo Layout/IndentationConsistency + + respond_to do |format| # rubocop:todo Layout/IndentationConsistency + if @person_block.save + # AC-2.9: I receive confirmation when blocking/unblocking users + flash[:notice] = t('better_together.person_blocks.notices.blocked') + format.html { redirect_to person_blocks_path } + format.turbo_stream do + render turbo_stream: turbo_stream.replace('blocked_people_list', partial: 'index_content') + end + else + flash[:alert] = @person_block.errors.full_messages.to_sentence + format.html { redirect_to person_blocks_path } + format.turbo_stream do + render turbo_stream: turbo_stream.replace('flash_messages', partial: 'shared/flash_messages') end end + end + end def destroy authorize @person_block @person_block.destroy - redirect_to blocks_path, notice: t('flash.person_block.unblocked') + + respond_to do |format| + # AC-2.9: I receive confirmation when blocking/unblocking users + flash[:notice] = t('better_together.person_blocks.notices.unblocked') + format.html { redirect_to person_blocks_path } + format.turbo_stream do + render turbo_stream: turbo_stream.replace('blocked_people_list', partial: 'index_content') + end + end end private - def current_person - current_user.person + def locale + params[:locale] || I18n.default_locale end def set_person_block + current_person = helpers.current_person + return render_not_found unless current_person + @person_block = current_person.person_blocks.find(params[:id]) + rescue ActiveRecord::RecordNotFound + render_not_found end def person_block_params diff --git a/app/policies/better_together/person_block_policy.rb b/app/policies/better_together/person_block_policy.rb index 385557487..f6760772f 100644 --- a/app/policies/better_together/person_block_policy.rb +++ b/app/policies/better_together/person_block_policy.rb @@ -6,6 +6,10 @@ def index? user.present? end + def new? + user.present? + end + def create? user.present? && record.blocker == agent && !record.blocked.permitted_to?('manage_platform') end diff --git a/app/policies/better_together/person_policy.rb b/app/policies/better_together/person_policy.rb index 66c03fae2..40a01c944 100644 --- a/app/policies/better_together/person_policy.rb +++ b/app/policies/better_together/person_policy.rb @@ -34,9 +34,95 @@ def me? record === user.person # rubocop:todo Style/CaseEquality end - class Scope < Scope # rubocop:todo Style/Documentation - def resolve - scope.with_translations + class Scope < ApplicationPolicy::Scope # rubocop:todo Style/Documentation + def resolve # rubocop:todo Metrics/AbcSize, Metrics/MethodLength + base_scope = scope.with_translations + + # Platform managers can see all people + return base_scope if permitted_to?('manage_platform') + + # Unauthenticated users can only see public profiles + return base_scope.privacy_public unless agent + + # Authenticated users can see: + # 1. Their own profile + # 2. Public profiles + # 3. People in their shared communities (if those people have privacy_community or higher) + # 4. People they have direct interactions with (blocked, conversations, etc.) + + people_table = scope.arel_table + + # Start with public profiles + query = people_table[:privacy].eq('public') + + # Add own profile + query = query.or(people_table[:id].eq(agent.id)) + + # Add people in shared communities with community+ privacy + if shared_community_member_ids.any? + # rubocop:todo Metrics/MethodLength, Lint/MissingCopEnableDirective + community_privacy_query = people_table[:privacy].in(%w[community public]) + .and(people_table[:id].in(shared_community_member_ids)) + query = query.or(community_privacy_query) + end + + # Add people with direct interactions (blocked users, conversation participants, etc.) + query = query.or(people_table[:id].in(interaction_person_ids)) if interaction_person_ids.any? + + base_scope.where(query).distinct + end + + private + + def shared_community_member_ids # rubocop:todo Metrics/MethodLength + return @shared_community_member_ids if defined?(@shared_community_member_ids) + + @shared_community_member_ids = if agent.present? + # Get people who are members of communities that the current person is also a member of # rubocop:disable Layout/LineLength + agent_community_ids = agent.person_community_memberships.pluck(:joinable_id) + if agent_community_ids.any? + BetterTogether::PersonCommunityMembership + .where(joinable_id: agent_community_ids) + .where.not(member_id: agent.id) + .pluck(:member_id) + .uniq + else + [] + end + else + [] + end + end + + def interaction_person_ids # rubocop:todo Metrics/AbcSize, Metrics/MethodLength + return @interaction_person_ids if defined?(@interaction_person_ids) + + @interaction_person_ids = if agent.present? + ids = [] + + # People the current user has blocked or been blocked by + blocked_ids = agent.person_blocks.pluck(:blocked_id) + blocker_ids = BetterTogether::PersonBlock.where(blocked_id: agent.id).pluck(:blocker_id) # rubocop:disable Layout/LineLength + ids.concat(blocked_ids + blocker_ids) + + # People in conversations with the current user + if defined?(BetterTogether::Conversation) && defined?(BetterTogether::ConversationParticipant) + conversation_ids = BetterTogether::ConversationParticipant + .where(person_id: agent.id) + .pluck(:conversation_id) + if conversation_ids.any? + participant_ids = BetterTogether::ConversationParticipant + .where(conversation_id: conversation_ids) + .where.not(person_id: agent.id) + .pluck(:person_id) + ids.concat(participant_ids) + end + end + + ids.uniq + else + [] + end end end end diff --git a/app/views/better_together/person_blocks/_index_content.html.erb b/app/views/better_together/person_blocks/_index_content.html.erb new file mode 100644 index 000000000..15c0dcc03 --- /dev/null +++ b/app/views/better_together/person_blocks/_index_content.html.erb @@ -0,0 +1,46 @@ +<% if @blocked_people.any? %> +
+ <% @person_blocks.each do |person_block| %> +
+
+
+ <% if person_block.blocked.profile_image.attached? %> + <%= image_tag person_block.blocked.profile_image, + class: 'rounded-circle', + size: '40x40', + alt: person_block.blocked.name %> + <% else %> +
+ +
+ <% end %> +
+
+
<%= person_block.blocked.name %>
+ + <%= t('better_together.person_blocks.index.blocked_on', + date: l(person_block.created_at, format: :short)) %> + +
+
+
+ <%= button_to t('better_together.person_blocks.index.unblock'), + person_block_path(person_block, locale: locale), + method: :delete, + data: { + turbo_method: :delete, + turbo_confirm: t('better_together.person_blocks.index.unblock_confirm', name: person_block.blocked.name) + }, + class: 'btn btn-outline-danger btn-sm' %> +
+
+ <% end %> +
+<% else %> +
+ +

<%= t('better_together.person_blocks.index.no_blocked_users') %>

+

<%= t('better_together.person_blocks.index.no_blocked_users_description') %>

+
+<% end %> diff --git a/app/views/better_together/person_blocks/index.html.erb b/app/views/better_together/person_blocks/index.html.erb index 67dc76401..d4994dd24 100644 --- a/app/views/better_together/person_blocks/index.html.erb +++ b/app/views/better_together/person_blocks/index.html.erb @@ -1,6 +1,43 @@ -

Blocked People

- +<% content_for :page_title, t('better_together.person_blocks.index.title') %> + +
+
+
+
+

<%= t('better_together.person_blocks.index.title') %>

+ <%= link_to t('better_together.person_blocks.index.block_user'), + new_person_block_path(locale: locale), + class: 'btn btn-primary' %> +
+ + +
+
+ <%= form_with url: person_blocks_path(locale: locale), method: :get, local: true, class: 'd-flex' do |form| %> + <%= form.text_field :search, + value: params[:search], + placeholder: t('better_together.person_blocks.index.search_placeholder'), + class: 'form-control me-2' %> + <%= form.submit t('better_together.person_blocks.index.search'), + class: 'btn btn-outline-secondary' %> + <% end %> +
+
+ + <%= t('better_together.person_blocks.index.blocked_count', count: @blocked_count) %> + +
+
+ + +
+ <%= render 'shared/flash_messages' if flash.any? %> +
+ + +
+ <%= render 'index_content' %> +
+
+
+
diff --git a/app/views/better_together/person_blocks/new.html.erb b/app/views/better_together/person_blocks/new.html.erb new file mode 100644 index 000000000..d3a5f189e --- /dev/null +++ b/app/views/better_together/person_blocks/new.html.erb @@ -0,0 +1,34 @@ +<% content_for :page_title, t('better_together.person_blocks.new.title') %> + +
+
+
+
+
+

<%= t('better_together.person_blocks.new.title') %>

+
+
+ <%= form_with model: [@person_block], url: person_blocks_path(locale: locale), local: true do |form| %> +
+ <%= form.label :blocked_identifier, + t('better_together.person_blocks.new.blocked_identifier_label'), + class: 'form-label' %> + <%= form.text_field :blocked_identifier, + class: 'form-control', + placeholder: t('better_together.person_blocks.new.blocked_identifier_placeholder'), + required: true %> +
+ <%= t('better_together.person_blocks.new.blocked_identifier_help') %> +
+
+ +
+ <%= link_to t('better_together.person_blocks.new.cancel'), person_blocks_path(locale: locale), class: 'btn btn-outline-secondary' %> + <%= form.submit t('better_together.person_blocks.new.submit'), class: 'btn btn-danger' %> +
+ <% end %> +
+
+
+
+
diff --git a/config/locales/en.yml b/config/locales/en.yml index 68fb1d79a..071da39fa 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1138,6 +1138,36 @@ en: device-permissions: Device-permissions images: Images preferences: Preferences + person_blocks: + index: + title: "Blocked People" + block_user: "Block User" + search: "Search" + search_placeholder: "Search blocked people..." + blocked_count: + one: "1 person blocked" + other: "%{count} people blocked" + no_blocked_people: "You haven't blocked anyone yet." + blocked_at: "Blocked" + actions: "Actions" + unblock: "Unblock" + unblock_confirmation: "Are you sure you want to unblock this person?" + new: + title: "Block User" + form: + identifier_label: "Username or Email" + identifier_placeholder: "Enter username or email address" + reason_label: "Reason (optional)" + reason_placeholder: "Why are you blocking this user?" + submit: "Block User" + cancel: "Cancel" + notices: + blocked: "User has been blocked successfully." + unblocked: "User has been unblocked successfully." + already_blocked: "This user is already blocked." + self_block_error: "You cannot block yourself." + not_found: "User not found." + platform_manager_error: "Platform managers cannot be blocked." phone_numbers: add: Add Phone Number label: Label diff --git a/config/routes.rb b/config/routes.rb index e21f52788..72091e1ed 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -77,7 +77,7 @@ end end - resources :person_blocks, path: :blocks, only: %i[index create destroy] + resources :person_blocks, path: :blocks, only: %i[index new create destroy] resources :reports, only: [:create] namespace :joatu, path: 'exchange' do diff --git a/docs/implementation/current_plans/block_management_interface_acceptance_criteria.md b/docs/implementation/current_plans/block_management_interface_acceptance_criteria.md new file mode 100644 index 000000000..44c4099fd --- /dev/null +++ b/docs/implementation/current_plans/block_management_interface_acceptance_criteria.md @@ -0,0 +1,155 @@ +# Block Management Interface - TDD Acceptance Criteria + +## Overview + +This document defines stakeholder-focused acceptance criteria for implementing the Block Management Interface using Test-Driven Development (TDD). This feature enables users to manage their blocked users list with a comprehensive interface. + +## Stakeholder Roles + +### Primary Stakeholders +- **End Users**: Community members who need to manage their blocked users for safety +- **Community Organizers**: Leaders who need to understand blocking patterns in their communities +- **Platform Organizers**: Staff who need platform-wide blocking oversight + +## Feature: Block Management Interface + +### End User Acceptance Criteria +**As an end user, I want to manage my blocked users so that I can control my social experience.** + +#### Core Blocking Functionality +- [ ] **AC-2.1**: I can block users from their profile page +- [ ] **AC-2.2**: I can block users from any of their content +- [ ] **AC-2.3**: I can view a list of users I have blocked +- [ ] **AC-2.4**: I can unblock users from my block list +- [ ] **AC-2.5**: Blocked users cannot send me messages or interact with my content +- [ ] **AC-2.6**: I cannot see content from blocked users in feeds +- [ ] **AC-2.7**: I cannot block platform administrators +- [ ] **AC-2.8**: I cannot block myself +- [ ] **AC-2.9**: I receive confirmation when blocking/unblocking users + +#### Block Management Interface +- [ ] **AC-2.10**: I can access my blocked users list from my account settings +- [ ] **AC-2.11**: I can search through my blocked users by name +- [ ] **AC-2.12**: I can see when I blocked each user +- [ ] **AC-2.13**: I can block a user by entering their username or email +- [ ] **AC-2.14**: I can quickly unblock users with a confirmation dialog +- [ ] **AC-2.15**: I can see how many users I have blocked +- [ ] **AC-2.16**: The interface works on mobile devices +- [ ] **AC-2.17**: The interface is accessible (WCAG AA compliant) + +### Community Organizer Acceptance Criteria +**As a community organizer, I want to understand blocking patterns so that I can identify problem users.** + +- [ ] **AC-2.18**: I can view users who have been blocked frequently in my community +- [ ] **AC-2.19**: I can see blocking trends in my community dashboard +- [ ] **AC-2.20**: I can intervene appropriately when blocking patterns indicate problems + +### Platform Organizer Acceptance Criteria +**As a platform organizer, I want comprehensive blocking controls so that I can manage platform-wide safety.** + +- [ ] **AC-2.21**: I can view all blocking relationships across the platform +- [ ] **AC-2.22**: I can remove blocks when necessary (for investigations) +- [ ] **AC-2.23**: I can prevent specific users from being blocked (staff protection) + +## TDD Implementation Plan + +### Phase 1: Core Block Management UI (End Users) +**Target Acceptance Criteria: AC-2.3, AC-2.4, AC-2.9 to AC-2.17** + +#### Test Categories + +**Model Tests** (PersonBlock model enhancements) +```ruby +# Test existing validations work with new interface +# Test search and filtering scopes +# Test blocking statistics and counts +``` + +**Controller Tests** (PersonBlocksController enhancements) +```ruby +# Test index action with search and filtering +# Test new action for blocking by username/email +# Test destroy action with proper confirmations +# Test AJAX responses for interactive features +``` + +**Feature Tests** (End-to-end user workflows) +```ruby +# Test complete blocking workflow from profile +# Test block management dashboard functionality +# Test search and filtering features +# Test mobile responsiveness +# Test accessibility compliance +``` + +**JavaScript Tests** (Stimulus controllers) +```ruby +# Test block/unblock confirmation dialogs +# Test search functionality +# Test dynamic UI updates +``` + +### Phase 2: Administrative Features (Community/Platform Organizers) +**Target Acceptance Criteria: AC-2.18 to AC-2.23** + +#### Test Categories + +**Analytics Tests** +```ruby +# Test blocking pattern analysis +# Test community-specific blocking statistics +# Test platform-wide blocking oversight +``` + +**Administrative Interface Tests** +```ruby +# Test community organizer blocking insights +# Test platform organizer blocking management +# Test intervention and override capabilities +``` + +## Success Metrics + +### User Experience Metrics +- [ ] Users can complete block management tasks in under 30 seconds +- [ ] Block/unblock actions provide immediate visual feedback +- [ ] Interface passes WCAG AA accessibility audit +- [ ] Mobile interface maintains full functionality + +### Safety Metrics +- [ ] Zero unauthorized blocks created (security validation) +- [ ] Platform administrators cannot be blocked (safety validation) +- [ ] Self-blocking attempts are prevented (validation working) + +### Performance Metrics +- [ ] Block list loads in under 2 seconds for 1000+ blocked users +- [ ] Search results appear in under 500ms +- [ ] AJAX updates complete without page refresh + +## Implementation Sequence + +### Step 1: Enhance Existing Controller (AC-2.3, AC-2.4) +1. Add search and filtering to PersonBlocksController#index +2. Enhance PersonBlocksController#destroy with confirmations +3. Add AJAX support for dynamic updates + +### Step 2: Create User Interface Views (AC-2.10 to AC-2.17) +1. Create blocked users dashboard (person_blocks/index.html.erb) +2. Create block user form (person_blocks/new.html.erb) +3. Create interactive components (_blocked_person.html.erb) +4. Add mobile-responsive styling and accessibility features + +### Step 3: Add JavaScript Interactivity (AC-2.9, AC-2.14) +1. Create Stimulus controller for block management +2. Add confirmation dialogs for unblocking +3. Implement search and filtering features +4. Add dynamic UI updates + +### Step 4: Administrative Features (Phase 2) +1. Add community organizer blocking insights +2. Add platform organizer blocking management +3. Implement blocking pattern analysis + +--- + +This TDD approach ensures every feature serves clear stakeholder needs with measurable, testable acceptance criteria before any code is written. diff --git a/spec/controllers/better_together/person_blocks_controller_spec.rb b/spec/controllers/better_together/person_blocks_controller_spec.rb new file mode 100644 index 000000000..707c13554 --- /dev/null +++ b/spec/controllers/better_together/person_blocks_controller_spec.rb @@ -0,0 +1,271 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe BetterTogether::PersonBlocksController do + include Devise::Test::ControllerHelpers + include BetterTogether::DeviseSessionHelpers + include Rails.application.routes.url_helpers + + routes { BetterTogether::Engine.routes } + let(:locale) { I18n.default_locale } + let(:user) { create(:better_together_user, :confirmed) } + let(:person) { user.person } + let(:blocked_person) { create(:better_together_person) } + let(:another_person) { create(:better_together_person) } + + before do + configure_host_platform + Rails.application.default_url_options = { locale: locale } + sign_in user + end + + describe 'GET #index' do + context 'when user has blocked people' do # rubocop:todo RSpec/MultipleMemoizedHelpers + let!(:person_block) { create(:person_block, blocker: person, blocked: blocked_person) } + + it 'returns http success' do + get :index, params: { locale: locale } + expect(response).to have_http_status(:success) + end + + it 'assigns @blocked_people' do + get :index, params: { locale: locale } + expect(assigns(:blocked_people)).to include(blocked_person) + end + + # AC-2.3: I can view a list of users I have blocked + it 'shows blocked users in the list' do + get :index, params: { locale: locale } + expect(assigns(:blocked_people)).to contain_exactly(blocked_person) + end + + # AC-2.11: I can search through my blocked users by name + it 'filters blocked users by search query' do + get :index, params: { locale: locale, search: blocked_person.name } + expect(assigns(:blocked_people)).to include(blocked_person) + end + + it 'returns empty results for non-matching search' do + get :index, params: { locale: locale, search: 'nonexistent' } + expect(assigns(:blocked_people)).to be_empty + end + + # AC-2.12: I can see when I blocked each user (FAILING - not implemented yet) + it 'includes blocking timestamp information' do + get :index, params: { locale: locale } + expect(assigns(:person_blocks)).to include(person_block) + end + + # AC-2.15: I can see how many users I have blocked (FAILING - not implemented yet) + it 'provides blocked users count' do + get :index, params: { locale: locale } + expect(assigns(:blocked_count)).to eq(1) + end + end + + context 'when user has no blocked people' do + it 'returns empty blocked_people' do + get :index, params: { locale: locale } + expect(assigns(:blocked_people)).to be_empty + end + + it 'shows zero count' do + get :index, params: { locale: locale } + expect(assigns(:blocked_count)).to eq(0) + end + end + + context 'when not authenticated' do + before { sign_out user } + + it 'redirects to sign in' do # rubocop:todo RSpec/MultipleExpectations + get :index, params: { locale: locale } + expect(response).to have_http_status(:redirect) + expect(response.location).to include('/users/sign-in') + end + end + end + + describe 'GET #new' do + # AC-2.13: I can block a user by entering their username or email (FAILING - not implemented yet) + it 'returns http success' do + get :new, params: { locale: locale } + expect(response).to have_http_status(:success) + end + + it 'assigns a new person_block' do + get :new, params: { locale: locale } + expect(assigns(:person_block)).to be_a_new(BetterTogether::PersonBlock) + end + + context 'when not authenticated' do + before { sign_out user } + + it 'redirects to sign in' do # rubocop:todo RSpec/MultipleExpectations + get :new, params: { locale: locale } + expect(response).to have_http_status(:redirect) + expect(response.location).to include('/users/sign-in') + end + end + end + + describe 'POST #create' do + context 'with valid params' do # rubocop:todo RSpec/MultipleMemoizedHelpers + let(:valid_params) { { locale: locale, person_block: { blocked_id: blocked_person.id } } } + + # AC-2.1: I can block users from their profile page + it 'creates a new PersonBlock' do + expect do + post :create, params: valid_params + end.to change(BetterTogether::PersonBlock, :count).by(1) + end + + # AC-2.9: I receive confirmation when blocking/unblocking users + it 'sets success flash message' do + post :create, params: valid_params + expect(flash[:notice]).to match(/blocked/i) + end + + it 'redirects to blocks path' do + post :create, params: valid_params + expect(response.location).to include('/blocks') + end + + # Test AJAX responses for interactive interface (FAILING - not implemented yet) + # rubocop:todo RSpec/NestedGroups + context 'with AJAX request' do # rubocop:todo RSpec/MultipleMemoizedHelpers, RSpec/NestedGroups + # rubocop:enable RSpec/NestedGroups + it 'responds with turbo_stream' do + post :create, params: valid_params, format: :turbo_stream + expect(response.content_type).to include('text/vnd.turbo-stream.html') + end + end + end + + context 'with invalid params' do + # AC-2.8: I cannot block myself + # rubocop:todo RSpec/NestedGroups + context 'when trying to block self' do # rubocop:todo RSpec/MultipleMemoizedHelpers, RSpec/NestedGroups + # rubocop:enable RSpec/NestedGroups + let(:invalid_params) { { locale: locale, person_block: { blocked_id: person.id } } } + + it 'does not create a PersonBlock' do + expect do + post :create, params: invalid_params + end.not_to change(BetterTogether::PersonBlock, :count) + end + + it 'sets error flash message' do + post :create, params: invalid_params + expect(flash[:alert]).to be_present + end + end + + # AC-2.7: I cannot block platform administrators + # rubocop:todo RSpec/NestedGroups + context 'when trying to block platform administrator' do # rubocop:todo RSpec/MultipleMemoizedHelpers, RSpec/NestedGroups + # rubocop:enable RSpec/NestedGroups + let(:platform_admin) { create(:better_together_person) } + let(:invalid_params) { { locale: locale, person_block: { blocked_identifier: platform_admin.identifier } } } + + before do + platform = BetterTogether::Platform.find_by(host: true) || + create(:better_together_platform, host: true) + role = BetterTogether::Role.find_by(identifier: 'platform_manager') || + create(:better_together_role, + identifier: 'platform_manager', + resource_type: 'BetterTogether::Platform', + name: 'Platform Manager') + + create(:better_together_person_platform_membership, + member: platform_admin, + joinable: platform, + role: role) + end + + it 'does not create a PersonBlock' do + expect do + post :create, params: invalid_params + end.not_to change(BetterTogether::PersonBlock, :count) + end + + it 'sets error flash message' do + post :create, params: invalid_params + expect(flash[:error]).to match(/not authorized/i) + end + end + end + + # Test blocking by username/email (FAILING - not implemented yet) + context 'when blocking by username' do # rubocop:todo RSpec/MultipleMemoizedHelpers + let!(:target_person) { create(:better_together_person, identifier: 'targetuser') } # rubocop:todo RSpec/LetSetup + let(:valid_params) { { locale: locale, person_block: { blocked_identifier: 'targetuser' } } } + + it 'creates a new PersonBlock by username' do + expect do + post :create, params: valid_params + end.to change(BetterTogether::PersonBlock, :count).by(1) + end + end + + context 'when not authenticated' do + before { sign_out user } + + it 'redirects to sign in' do # rubocop:todo RSpec/MultipleExpectations + post :create, params: { locale: locale, person_block: { blocked_id: blocked_person.id } } + expect(response).to have_http_status(:redirect) + expect(response.location).to include('/users/sign-in') + end + end + end + + describe 'DELETE #destroy' do # rubocop:todo RSpec/MultipleMemoizedHelpers + let!(:person_block) { create(:person_block, blocker: person, blocked: blocked_person) } + + # AC-2.4: I can unblock users from my block list + it 'destroys the PersonBlock' do + expect do + delete :destroy, params: { locale: locale, id: person_block.id } + end.to change(BetterTogether::PersonBlock, :count).by(-1) + end + + # AC-2.9: I receive confirmation when blocking/unblocking users + it 'sets success flash message' do + delete :destroy, params: { locale: locale, id: person_block.id } + expect(flash[:notice]).to match(/unblocked/i) + end + + it 'redirects to blocks path' do + delete :destroy, params: { locale: locale, id: person_block.id } + expect(response.location).to include('/blocks') + end + + # Test AJAX responses for interactive interface (FAILING - not implemented yet) + context 'with AJAX request' do # rubocop:todo RSpec/MultipleMemoizedHelpers + it 'responds with turbo_stream' do + delete :destroy, params: { locale: locale, id: person_block.id }, format: :turbo_stream + expect(response.content_type).to include('text/vnd.turbo-stream.html') + end + end + + context 'when trying to destroy someone elses block' do # rubocop:todo RSpec/MultipleMemoizedHelpers + let(:other_persons_block) { create(:person_block, blocker: another_person, blocked: blocked_person) } + + it 'renders not found (404)' do + delete :destroy, params: { locale: locale, id: other_persons_block.id } + expect(response).to have_http_status(:not_found) + end + end + + context 'when not authenticated' do # rubocop:todo RSpec/MultipleMemoizedHelpers + before { sign_out user } + + it 'redirects to sign in' do # rubocop:todo RSpec/MultipleExpectations + delete :destroy, params: { locale: locale, id: person_block.id } + expect(response).to have_http_status(:redirect) + expect(response.location).to include('/users/sign-in') + end + end + end +end diff --git a/spec/factories/better_together/person_platform_memberships.rb b/spec/factories/better_together/person_platform_memberships.rb new file mode 100644 index 000000000..4e8d66f97 --- /dev/null +++ b/spec/factories/better_together/person_platform_memberships.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# spec/factories/person_platform_memberships.rb + +FactoryBot.define do + factory :better_together_person_platform_membership, + class: 'BetterTogether::PersonPlatformMembership', + aliases: %i[person_platform_membership] do + id { SecureRandom.uuid } + association :joinable, factory: :better_together_platform + association :member, factory: :better_together_person + association :role, factory: :better_together_role + end +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index a696f3378..897ecdc92 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -7,6 +7,7 @@ # Prevent database truncation if the environment is production abort('The Rails environment is running in production mode!') if Rails.env.production? require 'rspec/rails' +require 'rails-controller-testing' ActiveJob::Base.queue_adapter = :test From af9a76b9e88dda50d8cbc6040b77fce1ed2f829b Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Sat, 23 Aug 2025 21:37:14 -0230 Subject: [PATCH 03/77] Refactor create? method in PersonBlockPolicy for clarity and correctness; remove redundant permission check for platform managers --- .../better_together/person_block_policy.rb | 18 +++++++++++++++++- app/policies/better_together/person_policy.rb | 1 - 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/policies/better_together/person_block_policy.rb b/app/policies/better_together/person_block_policy.rb index f6760772f..ea641ce07 100644 --- a/app/policies/better_together/person_block_policy.rb +++ b/app/policies/better_together/person_block_policy.rb @@ -11,7 +11,23 @@ def new? end def create? - user.present? && record.blocker == agent && !record.blocked.permitted_to?('manage_platform') + # Must be logged in and be the blocker + return false unless user.present? && record.blocker == agent + + # Must have a valid blocked person + return false unless record.blocked.present? + + # Cannot block platform managers + !blocked_user_is_platform_manager? + end + + private + + def blocked_user_is_platform_manager? + return false unless record.blocked&.user + + # Check if the blocked person's user has platform management permissions + record.blocked.user.permitted_to?('manage_platform') end def destroy? diff --git a/app/policies/better_together/person_policy.rb b/app/policies/better_together/person_policy.rb index 40a01c944..e8150cd2b 100644 --- a/app/policies/better_together/person_policy.rb +++ b/app/policies/better_together/person_policy.rb @@ -60,7 +60,6 @@ def resolve # rubocop:todo Metrics/AbcSize, Metrics/MethodLength # Add people in shared communities with community+ privacy if shared_community_member_ids.any? - # rubocop:todo Metrics/MethodLength, Lint/MissingCopEnableDirective community_privacy_query = people_table[:privacy].in(%w[community public]) .and(people_table[:id].in(shared_community_member_ids)) query = query.or(community_privacy_query) From a603500137fa8926c91f7ca1d016feb74c7ae915 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Sat, 23 Aug 2025 21:37:58 -0230 Subject: [PATCH 04/77] Refactor create? method in PersonBlockPolicy to improve readability; streamline blocked user check --- app/policies/better_together/person_block_policy.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/policies/better_together/person_block_policy.rb b/app/policies/better_together/person_block_policy.rb index ea641ce07..18e625e22 100644 --- a/app/policies/better_together/person_block_policy.rb +++ b/app/policies/better_together/person_block_policy.rb @@ -13,10 +13,10 @@ def new? def create? # Must be logged in and be the blocker return false unless user.present? && record.blocker == agent - + # Must have a valid blocked person return false unless record.blocked.present? - + # Cannot block platform managers !blocked_user_is_platform_manager? end @@ -24,10 +24,10 @@ def create? private def blocked_user_is_platform_manager? - return false unless record.blocked&.user - + return false unless record.blocked + # Check if the blocked person's user has platform management permissions - record.blocked.user.permitted_to?('manage_platform') + record.blocked.permitted_to?('manage_platform') end def destroy? From bff6a411385acc8a5ceedf5cf2d73132f92b8ca9 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Tue, 26 Aug 2025 15:34:59 -0230 Subject: [PATCH 05/77] Refactor destroy? method in PersonBlockPolicy for clarity --- app/policies/better_together/person_block_policy.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/policies/better_together/person_block_policy.rb b/app/policies/better_together/person_block_policy.rb index 18e625e22..4ff951f7a 100644 --- a/app/policies/better_together/person_block_policy.rb +++ b/app/policies/better_together/person_block_policy.rb @@ -21,7 +21,9 @@ def create? !blocked_user_is_platform_manager? end - private + def destroy? + user.present? && record.blocker == agent + end def blocked_user_is_platform_manager? return false unless record.blocked @@ -30,10 +32,6 @@ def blocked_user_is_platform_manager? record.blocked.permitted_to?('manage_platform') end - def destroy? - user.present? && record.blocker == agent - end - class Scope < Scope # rubocop:todo Style/Documentation def resolve scope.where(blocker: agent) From f04a1d8f14c8efa3d39a6d3b2fbfa005df38fd1d Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Tue, 26 Aug 2025 15:35:22 -0230 Subject: [PATCH 06/77] Add blocked people tab and localization support in settings --- .../person_blocks/index.html.erb | 70 ++++++++++--------- .../better_together/settings/index.html.erb | 29 ++++++++ config/locales/en.yml | 4 ++ config/locales/es.yml | 4 ++ config/locales/fr.yml | 4 ++ 5 files changed, 77 insertions(+), 34 deletions(-) diff --git a/app/views/better_together/person_blocks/index.html.erb b/app/views/better_together/person_blocks/index.html.erb index d4994dd24..631829aa5 100644 --- a/app/views/better_together/person_blocks/index.html.erb +++ b/app/views/better_together/person_blocks/index.html.erb @@ -1,43 +1,45 @@ <% content_for :page_title, t('better_together.person_blocks.index.title') %> -
-
-
-
-

<%= t('better_together.person_blocks.index.title') %>

- <%= link_to t('better_together.person_blocks.index.block_user'), - new_person_block_path(locale: locale), - class: 'btn btn-primary' %> -
- - -
-
- <%= form_with url: person_blocks_path(locale: locale), method: :get, local: true, class: 'd-flex' do |form| %> - <%= form.text_field :search, - value: params[:search], - placeholder: t('better_together.person_blocks.index.search_placeholder'), - class: 'form-control me-2' %> - <%= form.submit t('better_together.person_blocks.index.search'), - class: 'btn btn-outline-secondary' %> - <% end %> +<%= turbo_frame_tag "blocked-people-settings" do %> +
+
+
+
+

<%= t('better_together.person_blocks.index.title') %>

+ <%= link_to t('better_together.person_blocks.index.block_user'), + new_person_block_path(locale: locale), + class: 'btn btn-primary btn-sm' %>
-
- - <%= t('better_together.person_blocks.index.blocked_count', count: @blocked_count) %> - + + +
+
+ <%= form_with url: person_blocks_path(locale: locale), method: :get, local: true, class: 'd-flex' do |form| %> + <%= form.text_field :search, + value: params[:search], + placeholder: t('better_together.person_blocks.index.search_placeholder'), + class: 'form-control me-2' %> + <%= form.submit t('better_together.person_blocks.index.search'), + class: 'btn btn-outline-secondary btn-sm' %> + <% end %> +
+
+ + <%= t('better_together.person_blocks.index.blocked_count', count: @blocked_count) %> + +
-
- -
- <%= render 'shared/flash_messages' if flash.any? %> -
+ +
+ <%= render 'shared/flash_messages' if flash.any? %> +
- -
- <%= render 'index_content' %> + +
+ <%= render 'index_content' %> +
-
+<% end %> diff --git a/app/views/better_together/settings/index.html.erb b/app/views/better_together/settings/index.html.erb index 6cb31febb..42194a369 100644 --- a/app/views/better_together/settings/index.html.erb +++ b/app/views/better_together/settings/index.html.erb @@ -29,6 +29,10 @@ <%= t('.tabs.account') %> + + +