diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index 2af0c16ffc..93869880d3 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -3,27 +3,17 @@ name: Claude Code on: issue_comment: types: [created] - pull_request_review_comment: - types: [created] - issues: - types: [opened, assigned] - pull_request_review: - types: [submitted] jobs: claude: - if: | - (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) || - (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) || - (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) || - (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude'))) + if: contains(github.event.comment.body, '@claude') runs-on: ubuntu-latest permissions: contents: write pull-requests: write issues: write id-token: write - actions: read # Required for Claude to read CI results on PRs + actions: read steps: - name: Checkout repository uses: actions/checkout@v5 @@ -35,76 +25,14 @@ jobs: with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} - # Enable progress tracking track_progress: true - # Your custom review instructions prompt: | REPO: ${{ github.repository }} PR NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }} - - Review this pull request and provide inline feedback using the GitHub review system. Follow these steps: - - 1. Start a review: Use `mcp__github__create_pending_pull_request_review` to start a pending review. - 2. Get diff information: Use `mcp__github__get_pull_request_diff` to see the code changes and line numbers. - - If the pull request has many changes, paginate the diff using the `page` and `per_page` parameters. - - Example: To get the first 100 lines, call `mcp__github__get_pull_request_diff` with `per_page=100` and `page=1`. For the next 100 lines, use `page=2`, and so on. - - Continue fetching pages until all changes are retrieved. - 3. Add inline comments: Use `mcp__github__add_comment_to_pending_review` to provide feedback on specific lines of code snippets. - - **When suggesting code changes, format them as GitHub suggestion blocks so they can be committed directly:** - - For single-line suggestions, use: - ```suggestion - suggested code here - ``` - - For multi-line suggestions, use: - ```suggestion:-0+1 - suggested code here - ``` - (Adjust the numbers to match how many lines to remove and add) - - Only provide suggestion blocks when you have a concrete, actionable fix - - Include explanation text before or after the suggestion block - 4. Submit for review: Use `mcp__github__submit_pending_pull_request_review` with the event type set to "COMMENT" (not "REQUEST_CHANGES") to publish all comments for non-blocking review. - - Focus on the following focus areas: - - 1. **Code Quality** - - Clean code principles and best practices - - Proper error handling and edge cases - - Code readability and maintainability - - **Provide suggestion blocks for improvements when possible** - - 2. **Security** - - Check for potential security vulnerabilities - - Validate input sanitization - - Review authentication/authorization logic - - **Suggest secure alternatives with suggestion blocks** - - 3. **Performance** - - Identify potential performance bottlenecks - - Review database queries for efficiency - - Check for memory leaks or resource issues - - **Offer optimized code via suggestion blocks** - - 4. **Testing** - - Verify adequate test coverage - - Review test quality and edge cases - - Check for missing test scenarios - - 5. **Documentation** - - Ensure code is properly documented - - Verify README updates for new features - - Check API documentation accuracy - - Provide detailed feedback using inline comments for specific issues. - Use suggestion blocks liberally to make it easy for developers to apply fixes. - Use top-level comments for general observations or praise. - # Tools for comprehensive PR review + Review this pull request following the guidelines in docs/review-guidelines.md. + Provide detailed inline feedback using suggestion blocks where applicable. + claude_args: | --allowedTools "mcp__github__get_pull_request,mcp__github__create_pending_pull_request_review,mcp__github__add_comment_to_pending_review,mcp__github__submit_pending_pull_request_review,mcp__github__get_pull_request_diff" - - # When track_progress is enabled: - # - Creates a tracking comment with progress checkboxes - # - Includes all PR context (comments, attachments, images) - # - Updates progress as the review proceeds - # - Marks as completed when done \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000000..bafc974734 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,15 @@ +# UDB3 Backend + +UiTdatabank 3 core application - an event-sourced backend built with Broadway. + +## Documentation + +- [Domain](docs/domain.md) - Core domain concepts (Event, Place, Organizer, Offer) +- [Architecture](docs/architecture.md) - System design, tech stack, message flow +- [Coding Guidelines](docs/coding-guidelines.md) - Code style, conventions, security +- [Commands](docs/commands.md) - All available make commands +- [Review Guidelines](docs/review-guidelines.md) - PR review process and focus areas + +## Acceptance Tests + +- [General](docs/features/general.md) - Acceptance test overview diff --git a/docs/architecture.md b/docs/architecture.md new file mode 100644 index 0000000000..d6a127e86e --- /dev/null +++ b/docs/architecture.md @@ -0,0 +1,89 @@ +# Architecture + +UiTdatabank 3 (UDB3) is an event-sourced backend built with Broadway. + +## Tech Stack + +- **Language**: PHP 8.1+ +- **Architecture**: Event Sourcing with [Broadway](https://github.com/broadway/broadway) +- **Database**: Doctrine DBAL +- **Message Broker**: RabbitMQ (AMQP) +- **Search**: SAPI3 (external Elasticsearch-based service) +- **Cache**: Redis +- **Testing**: PHPUnit (unit), Behat (acceptance) +- **Quality**: PHPStan (static analysis), PHP-CS-Fixer (code style) +- **Environment**: Docker with docker-compose + +## Project Structure + +``` +├── app/ # Service providers and application wiring +├── src/ # Core domain code +├── tests/ # PHPUnit tests (mirrors src/ structure) +├── features/ # Behat acceptance tests +├── docker/ # Docker configuration +├── docs/ # Documentation +│ └── refactor/ # Refactoring plans and progress +└── vendor/ # Dependencies (never modify) +``` + +## Event Sourcing with Broadway + +- **Aggregates**: Domain objects that emit events (in `src/`) +- **Projectors**: Build read models from events +- **Event Bus**: Distributes events to handlers + +## Message Flow: Domain Events to Search Indexing + +``` +1. API Request (e.g., Create Event) + ↓ +2. Command Handler processes command + ↓ +3. Domain event created (e.g., EventCreated) + ↓ +4. EventBus publishes event to subscribers + ↓ +5. Projectors generate JSON-LD → emit *ProjectedToJSONLD events + ↓ +6. AMQPPublisher sends message to RabbitMQ + ↓ +7. External SAPI3 consumes message and indexes in Elasticsearch +``` + +### AMQP Configuration + +- **Exchange**: `udb3.x.domain-events` +- **Routing**: Messages routed to `api`, `cli`, or `related` queues based on context +- **Message Types**: + - `application/vnd.cultuurnet.udb3-events.event-projected-to-jsonld+json` + - `application/vnd.cultuurnet.udb3-events.place-projected-to-jsonld+json` + - `application/vnd.cultuurnet.udb3-events.organizer-projected-to-jsonld+json` + +### SAPI3 Integration + +SAPI3 is an external search service (Elasticsearch-based) that: +- Consumes messages from RabbitMQ +- Indexes events, places, and organizers +- Provides search endpoints proxied by UDB3 + +**Configuration** (in `config.php`): +- Base URL: `http://search.uitdatabank.local:9000` +- ES URL: `http://search.uitdatabank.local:9200/` +- API Key: configured per environment + +**Search Endpoints** (proxy to SAPI3): +- `/events` - Search events +- `/places` - Search places +- `/organizers` - Search organizers +- `/offers` - Search events and places combined + +## Docker Services + +| Service | Port | Purpose | +|---------|------|---------| +| PHP | 80 | Application | +| MySQL | 3306 | Database | +| Redis | 6379 | Cache | +| RabbitMQ | 5672, 15672 | Message broker (AMQP, Management UI) | +| Mailpit | 1025, 8025 | Email testing (SMTP, Web UI) | diff --git a/docs/coding-guidelines.md b/docs/coding-guidelines.md new file mode 100644 index 0000000000..e31117e4eb --- /dev/null +++ b/docs/coding-guidelines.md @@ -0,0 +1,118 @@ +# Coding Guidelines + +## General + +- Use `final` classes by default +- Follow PSR-4 autoloading under `CultuurNet\UDB3\` namespace +- Keep classes small and focused (Single Responsibility) + +## PHP 8.1 Features + +Use **readonly properties** with constructor promotion for immutable class properties. + +See `src/Doctrine/DBALDatabaseConnectionChecker.php` for an example. + +> **Note**: Do NOT use PHP 8.2 `readonly class` syntax - we're on PHP 8.1. + +## Comments & Self-Documenting Code + +Avoid comments in favor of self-documenting code: + +- **Type hints**: Use typed parameters, typed properties and return types instead of docblock descriptions +- **Method names**: Use descriptive, self-explaining method and function names +- **Variable names**: Choose clear, meaningful variable names that convey purpose +- **Class names**: Use descriptive class names that explain their responsibility +- **File names**: Mirror class names for easy discoverability + +Comments are acceptable when explaining **why** something is done (not **what**), such as workarounds for external limitations or non-obvious business rules. + +## Naming + +### Interfaces + +- Descriptive name without `Interface` suffix (e.g., `DatabaseConnectionChecker`) +- Implementations: Prefix with technology (e.g., `DBALDatabaseConnectionChecker`) + +### Exceptions + +- Descriptive name without `Exception` suffix +- Name should describe what went wrong (e.g., `DocumentDoesNotExist`, `NewsArticleNotFound`) + +## Testing + +- Use `@test` annotation style for test methods +- Mock objects use intersection types: `Connection&MockObject` +- Prefer separate test methods over data providers for clarity +- Test file location mirrors source: `src/Foo/Bar.php` → `tests/Foo/BarTest.php` + +## Gotchas & Pitfalls + +| Issue | Solution | +|-------|----------| +| Commands fail outside Docker | Use `make` commands, not direct `composer` calls | +| `withConsecutive` deprecation | Avoid in new tests - use separate test methods | +| Uppercase `String` type hint | Legacy code - don't "fix" these | +| Tests fail after changes | Run `make ci` before committing | + +## Restrictions + +- **Never** modify files in `vendor/` +- **Never** commit `.env` or credentials files +- **Avoid** creating new service providers - extend existing ones in `app/` +- **Avoid** adding dependencies without team discussion +- **Always** run `make ci` before considering work complete + +## AI Restrictions + +- **Never** create commits - only humans commit code +- **Plan** larger changes but implement step by step, waiting for human review between steps + +## Dependency Injection + +- Service providers in `app/` wire all dependencies +- Use **constructor injection**, not service locators +- Extract testable logic into separate classes with **interfaces** + +See `src/Doctrine/DatabaseConnectionChecker.php` and `src/Doctrine/DBALDatabaseConnectionChecker.php` for an example of interface + implementation pattern. + +## Workflow + +- **Small commits**: One logical change per commit + - A logical change can span multiple files (e.g., source code + tests, or a refactor touching several related files) + - All changes in the commit should belong together and serve a single purpose + - The goal is to make commits easy to review and understand +- **Small pull requests**: Easier to review, faster to merge +- **Feature flags**: Deploy often, enable features when ready + +## Preferences + +When working on this codebase, prefer: + +- Extracting logic into testable classes with interfaces over inline implementation +- Constructor injection over service locator patterns +- Explicit code over clever abstractions +- Integration with existing patterns over introducing new ones + +## Security + +### Never Commit + +- `.env` files or environment-specific configurations +- API keys, tokens, or credentials +- Private keys or certificates +- Database connection strings with passwords +- Any hardcoded secrets + +### Configuration + +- Secrets belong in environment variables, not in code +- Use `.env.example` or `.env.dist` for documenting required env vars (without actual values) +- Configuration files with secrets should be in `.gitignore` + +### Secure Coding + +- No hardcoded credentials or secrets in code +- No sensitive data in log statements +- Validate input on user-provided data +- SQL queries use parameterized statements (handled by Doctrine DBAL) +- No sensitive data exposure in error messages diff --git a/docs/commands.md b/docs/commands.md new file mode 100644 index 0000000000..4a499f6bc8 --- /dev/null +++ b/docs/commands.md @@ -0,0 +1,42 @@ +# Commands + +> **Important**: Always use `make` commands, not direct `composer` or `vendor/bin` calls. The project runs in Docker. + +## Environment Management + +```bash +make up # Start containers +make down # Stop containers +make bash # Shell into PHP container +make install # Install composer dependencies +make migrate # Run database migrations +make init # Install + migrate (fresh setup) +``` + +## Quality Assurance + +```bash +make ci # Run ALL checks (PHPStan + tests + code style) +make stan # PHPStan static analysis only +make test # PHPUnit tests only +make cs # Code style check (dry-run) +make cs-fix # Auto-fix code style issues +``` + +## Unit Testing + +```bash +make test # All unit tests +make test-filter filter=EventBusForwarding # Tests matching filter +make test-group group=integration # Tests in specific group +``` + +## Acceptance Testing (Behat) + +```bash +make feature # All feature tests +make feature-tag tag=@events # Feature tests by tag +make feature-tag tag=@sapi3 # SAPI3 search integration tests +make feature-filter path=features/search/auth.feature # All scenarios in a file +make feature-filter path=features/search/auth.feature:25 # Single scenario by line number +``` diff --git a/docs/domain.md b/docs/domain.md new file mode 100644 index 0000000000..0887a9fff0 --- /dev/null +++ b/docs/domain.md @@ -0,0 +1,15 @@ +# Domain Concepts + +## Resources + +UDB3 manages three main resources: + +| Resource | Description | +|----------|-------------| +| **Event** | An activity happening at a specific time and place | +| **Place** | A location where events can happen | +| **Organizer** | The entity organizing events | + +## Offers + +Event and Place are collectively referred to as **Offer**. This terminology is used throughout the codebase and APIs. \ No newline at end of file diff --git a/docs/features/general.md b/docs/features/general.md new file mode 100644 index 0000000000..f10b4a625b --- /dev/null +++ b/docs/features/general.md @@ -0,0 +1,24 @@ +# Acceptance Tests + +## Overview + +Acceptance tests are written using Behat and located in the `features/` directory. + +## Test Infrastructure + +- `features/bootstrap/FeatureContext.php` - Main Behat context +- `features/Steps/` - Reusable step definitions + +## Running Tests + +See [Commands](../commands.md) for all available test commands. + +## Adding New Tests + +1. **Verify the actual API response** using curl against the local SAPI3 URL +2. **Write the scenario** in the appropriate feature file under `features/search/` +3. **Run the test** using `make feature-filter path=` + +## Features + +- [Search](search.md) - SAPI3 search integration tests diff --git a/docs/features/search.md b/docs/features/search.md new file mode 100644 index 0000000000..c3853184ba --- /dev/null +++ b/docs/features/search.md @@ -0,0 +1,24 @@ +# Search Acceptance Tests + +## Overview + +Acceptance tests for the SAPI3 (Search API 3) integration. + +## Test Location + +- `features/search/` directory +- Tagged with `@sapi3` + +## Test Files + +- `auth.feature` - Authentication tests +- `search-proxy.feature` - Search endpoint proxy tests +- `facets.feature` - Facet response tests +- `contributors.feature` - Contributor filtering +- `default-queries-*.feature` - Default query filtering + +## How Tests Work + +1. Create entity (event/place/organizer) via API +2. Poll SAPI3 endpoint waiting for indexing (max 5 seconds) +3. Assert search results diff --git a/docs/review-guidelines.md b/docs/review-guidelines.md new file mode 100644 index 0000000000..a1291a9faa --- /dev/null +++ b/docs/review-guidelines.md @@ -0,0 +1,43 @@ +# PR Review Guidelines + +Review pull requests and provide inline feedback using the GitHub review system. + +Focus on actionable improvements only. Skip praise and positive feedback. + +Reference relevant files in the `docs/` folder for project standards. + +## Suggestion Blocks + +When suggesting code changes, format them as GitHub suggestion blocks: + +- Single-line: ` ```suggestion ` +- Multi-line: ` ```suggestion:-0+1 ` (adjust numbers for lines to remove/add) + +Only provide suggestion blocks for concrete, actionable fixes. + +## Focus Areas + +### Code Quality +- Clean code principles and best practices +- Proper error handling and edge cases +- Code readability and maintainability + +### Security +- Potential security vulnerabilities +- Input sanitization +- Authentication/authorization logic + +### Performance +- Potential performance bottlenecks +- Database query efficiency +- Memory leaks or resource issues + +### Testing +- Adequate test coverage +- Test quality and edge cases +- Missing test scenarios + +### Documentation +- Code documentation +- README updates for new features +- API documentation accuracy