-
Notifications
You must be signed in to change notification settings - Fork 2
Add Journal entity support with PSR-3 compatibility (issue #72) #73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
Implemented core Journal bounded context following DDD patterns: - JournalItem entity with PSR-3 factory methods (emergency, alert, critical, error, warning, notice, info, debug) - JournalContext value object with label, payload (JSONB), userId, and IP address fields - LogLevel enum for PSR-3 compatible log levels - JournalItemInterface for SDK contract extraction - JournalItemRepositoryInterface with methods for CRUD operations and cleanup - DoctrineDbalJournalItemRepository implementation with pagination support - Doctrine ORM XML mappings for entity and embeddable objects Features: - UUID identifiers for journal items - Immutable timestamps using Carbon - Reference to ApplicationInstallation via UUID - Support for IP address storage using darsyn/ip library - Repository methods for deletion by installation ID and by date - Indexed fields for optimal query performance
Summary of ChangesHello @mesilov, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive journaling system designed to capture and manage domain business events within the application. By adhering to Domain-Driven Design patterns and PSR-3 logging standards, it provides a structured, persistent, and queryable mechanism for technical support staff to monitor and understand application behavior. The changes encompass new entities, value objects, interfaces, and a Doctrine-based repository implementation, ensuring robust data handling and integration. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a well-designed Journal bounded context, complete with entities, value objects, repositories, and Doctrine mappings, adhering to DDD principles and PSR-3 compatibility. The code is clean and utilizes modern PHP features effectively. My review includes suggestions to optimize database query performance by refining table indexes, to simplify the repository implementation by leveraging inherited methods, and to improve data validation within the JournalContext value object to ensure greater data integrity.
src/Journal/Infrastructure/Doctrine/DoctrineDbalJournalItemRepository.php
Show resolved
Hide resolved
src/Journal/Infrastructure/Doctrine/DoctrineDbalJournalItemRepository.php
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Added dependencies and Messenger integration:
- Updated composer.json with Symfony Messenger, Twig Bundle, Form, Routing, and Validator components
- Created TestJournalEvent for demonstration purposes
- Implemented TestJournalEventHandler to write events to journal with INFO level
- Created ConsumeMessagesCommand console command for message consumption
Implemented admin interface components:
- JournalItemReadRepository for filtered data retrieval with KnpPaginator support
- JournalAdminController with two routes:
* /admin/journal - list view with filters (domain, level, label) and pagination (50 items)
* /admin/journal/{id} - detailed view with payload visualization
Created Twig templates:
- layout.html.twig - base layout for admin interface
- admin/list.html.twig - journal list with filters, pagination, and responsive design
- admin/show.html.twig - detailed record view with syntax-highlighted JSON payload
Features:
- Filter by Bitrix24 portal domain, log level, and custom labels
- Pagination support (50 records per page)
- Color-coded log levels (emergency to debug)
- JSON payload viewer with syntax highlighting
- Responsive and user-friendly interface
- Full PSR-3 compatibility
#72) Removed Messenger integration: - Deleted Events/TestJournalEvent.php - Deleted MessageHandler/TestJournalEventHandler.php - Deleted Console/ConsumeMessagesCommand.php - Removed symfony/messenger and symfony/form from composer.json Added PSR-3 Logger service: - JournalLogger - implements Psr\Log\LoggerInterface * Uses LoggerTrait for all PSR-3 methods * Accepts repository as dependency * Writes directly to journal via repository * Supports all 8 PSR-3 log levels with context - JournalLoggerFactory - creates logger instances per installation Added In-Memory repository implementation: - InMemoryJournalItemRepository - for testing and non-persistent scenarios * Full implementation of JournalItemRepositoryInterface * CRUD operations with filtering and pagination * Clear and findAll methods for testing Comprehensive unit tests (4 test suites, 50+ test cases): - JournalItemTest - Entity creation and validation * All PSR-3 factory methods (emergency to debug) * Context and payload handling * Validation of required fields - LogLevelTest - PSR-3 level conversion * Case-insensitive string conversion * All 8 log levels validation - InMemoryJournalItemRepositoryTest - Repository operations * Save/find/delete operations * Filtering by installation ID and level * Pagination (limit/offset) * Date-based cleanup - JournalLoggerTest - PSR-3 logger functionality * All log methods (info, error, warning, etc.) * Context preservation * Level conversion * EntityManager flush verification Documentation: - tests/Unit/Journal/README.md - test execution guide All tests follow PHPUnit 11 conventions and project code standards.
Database changes: - Renamed table from journal_item to journal - Added composite index (application_installation_id, level, created_at_utc) for optimal query performance - Kept separate index on created_at_utc for deleteOlderThan operation Entity improvements: - Removed readonly from non-constructor fields (applicationInstallationId, level, message, context) - ID now generated inside constructor using Uuid::v7() - Constructor accepts JournalContext object instead of array - Removed PSR-3 factory methods from JournalItemInterface (they remain in implementation) JournalContext improvements: - Made label required (non-nullable) - Removed fromArray() method - Simplified API - construct directly with parameters - Updated Doctrine mapping: label nullable="false" Controller improvements: - Removed Route attributes from JournalAdminController - Developer configures routes in their application - Added documentation comment about route configuration Repository organization: - Moved InMemoryJournalItemRepository from src/ to tests/ - Updated namespace to Bitrix24\Lib\Tests\Unit\Journal\Infrastructure\InMemory - Used in all unit tests for isolation JournalLogger changes: - Updated to create JournalContext from PSR-3 array context - Added default label 'application.log' if not provided - IP address parsing with error handling Documentation: - Moved src/Journal/README.md to src/Journal/Docs/ Updated all unit tests: - Tests now create JournalContext objects explicitly - Updated imports for InMemoryRepository location - All 50+ tests updated and passing - Tests verify label is required All changes maintain backward compatibility at the PSR-3 logger level.
- Replace custom CSS with Bootstrap 5 CDN in layout.html.twig - Update list.html.twig to use Bootstrap components: * Bootstrap forms (form-select, form-label) * Bootstrap table (table-hover, table-striped) * Bootstrap pagination * Bootstrap cards and badges * Bootstrap grid system - Update show.html.twig to use Bootstrap components: * Bootstrap breadcrumb * Bootstrap cards and badges * Bootstrap grid (row, col-md-*) * Bootstrap alerts * Bootstrap utility classes - Keep custom styling only for JSON syntax highlighting and log level badges
| use Doctrine\ORM\EntityRepository; | ||
| use Symfony\Component\Uid\Uuid; | ||
|
|
||
| class DoctrineDbalJournalItemRepository extends EntityRepository implements JournalItemRepositoryInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Мы за композицию, так код чище
| } | ||
|
|
||
| #[\Override] | ||
| public function deleteByApplicationInstallationId(Uuid $applicationInstallationId): int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Только "свертка" удаляет физичиски данные. Или в 0, или куда-то архивирует. Остальное или не надо, или софт-делит.
| /** | ||
| * Delete all journal items by application installation ID | ||
| */ | ||
| public function deleteByApplicationInstallationId(Uuid $applicationInstallationId): int; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
убрать
| /** | ||
| * Count journal items by application installation ID | ||
| */ | ||
| public function countByApplicationInstallationId(Uuid $applicationInstallationId, ?LogLevel $level = null): int; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
убрать
| /** | ||
| * Factory for creating JournalLogger instances | ||
| */ | ||
| readonly class JournalLoggerFactory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если это для тестов, то и унести в тесты
camaxtly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
30-11-2025
Implemented core Journal bounded context following DDD patterns:
Features: