Add Journal entity support with PSR-3 compatibility#83
Open
KarlsonComplete wants to merge 138 commits intomesilov:claude/implement-issue-72-01PyCT5q1ySkhYBu5ehAcnL9from
Conversation
Замапили сущность.
…iry-empty-field Fix/auth token expiry empty field
Inserted `todo` comments with issue references for pending fixes and added static analysis suppressions (`@phpstan-ignore-next-line`) in multiple handlers. Refactored `RenewAuthToken` command to add a default `null` value for `bitrix24UserId`. Removed redundant `#[\Override]` annotations in repository methods. Signed-off-by: mesilov <mesilov.maxim@gmail.com>
Inserted `todo` comments with issue references for pending fixes and added static analysis suppressions (`@phpstan-ignore-next-line`) in multiple handlers. Refactored `RenewAuthToken` command to add a default `null` value for `bitrix24UserId`. Removed redundant `#[\Override]` annotations in repository methods. Signed-off-by: mesilov <mesilov.maxim@gmail.com>
…esh-token Bugfix/58 fix user id refresh token
…iry-empty-field Исправление ошибки в описании типов для СУБД при хранени AuthToken
…esh-token Bugfix/58 fix user id refresh token
Implements full CRUD functionality for application settings storage: Features: - ApplicationSetting entity with UUID v7 ID generation - Key-value storage tied to ApplicationInstallations - Unique constraint on (application_installation_id, key) - Repository with find/save/delete operations - CQRS Use Cases: Set (create/update), Get, Delete - Comprehensive unit and functional tests - Doctrine ORM XML mapping configuration Architecture: - Follows DDD and CQRS patterns - Extends AggregateRoot for event support - Readonly classes for commands - Strict type validation in constructors - Proper exception handling Database: - Table: application_setting - Fields: id, application_installation_id, key, value, created_at_utc, updated_at_utc - Indexes on application_installation_id - Unique constraint ensures no duplicate keys per installation Tests: - Unit tests for entity validation and business logic - Functional tests for repository operations - Functional tests for all use case handlers
Major improvements to ApplicationSettings bounded context: 1. Entity & Interface: - Add ApplicationSettingInterface with TODO to move to b24-php-sdk - Add b24UserId (nullable int) for personal settings - Add b24DepartmentId (nullable int) for departmental settings - Add scope validation (user/department mutually exclusive) - Add isGlobal(), isPersonal(), isDepartmental() methods - Update key validation: only lowercase latin letters and dots 2. Repository & Interface: - Add ApplicationSettingRepositoryInterface with TODO - Add findGlobalByKey() - find setting without user/dept scope - Add findPersonalByKey() - find by key + user ID - Add findDepartmentalByKey() - find by key + department ID - Add findByKey() - flexible search with optional filters - Add findAllGlobal() - all global settings - Add findAllPersonal() - all user settings - Add findAllDepartmental() - all department settings - Refactor findAll() - all settings regardless of scope 3. Database Schema: - Add b24_user_id column (nullable integer) - Add b24_department_id column (nullable integer) - Update unique constraint: (app_id, key, user_id, dept_id) - Add indexes for performance: user_id, dept_id, key 4. Use Cases (CQRS): - Update Set/Command with optional b24UserId, b24DepartmentId - Update Get/Command with scope parameters - Update Delete/Command with scope parameters - All handlers now use interface types - Update validation for new scope parameters 5. Services: - Add InstallSettings service for default settings creation - Support bulk creation of global settings on install - Skip existing settings to avoid duplicates - Provides getRecommendedDefaults() helper 6. CLI Command: - Add ApplicationSettingsListCommand (app:settings:list) - List all settings for portal - Filter by user ID (--user-id) - Filter by department ID (--department-id) - Show only global settings (--global-only) - Table output with key, value, scope, timestamps 7. Tests: - Update unit tests for new validation rules - Add tests for global/personal/departmental scopes - Add tests for scope validation - Test userId/departmentId validation Key validation change: Only lowercase latin letters and dots allowed Example valid keys: app.enabled, user.theme, feature.analytics Settings hierarchy: - Global: applies to entire installation - Departmental: specific to department (overrides global) - Personal: specific to user (overrides departmental & global)
…ssue mesilov#67) Changes: - Remove Get UseCase (UseCases now only for data modification) - Add changedByBitrix24UserId field to track who modified settings - Add isRequired field for frontend validation hints - Create ApplicationSettingChangedEvent with old/new values and change tracking - Move InstallSettings from root Services to ApplicationSettings namespace - Update Set UseCase to support new fields - Update all tests for new Entity constructor signature - Add comprehensive tests for new fields and event emission Entity changes: - changedByBitrix24UserId: nullable int, tracks modifier - isRequired: boolean, indicates required settings for frontend - updateValue() now emits ApplicationSettingChangedEvent Doctrine mapping updated with new fields: - changed_by_b24_user_id (nullable) - is_required (not null)
…v#67) Major improvements to ApplicationSettings: 1. Enum ApplicationSettingStatus - Added enum with Active and Deleted states - Supports soft-delete pattern for data retention 2. Entity changes - Added status field (ApplicationSettingStatus) - Added markAsDeleted() method for soft-delete - Added isActive() method to check status - Updated Doctrine mapping with status field and index 3. Repository improvements - All find* methods now filter by status=Active - Added softDeleteByApplicationInstallationId() method - Soft-deleted records excluded from queries by default - Hard delete methods preserved for admin operations 4. UseCase Delete refactored - Changed from hard delete to soft-delete - Calls markAsDeleted() instead of repository->delete() - Preserves data for audit and recovery 5. New UseCase: OnApplicationDelete - Command and Handler for bulk soft-delete - Triggered when application is uninstalled - Soft-deletes all settings for installation - Maintains data integrity and history 6. Comprehensive tests - Unit tests for status and markAsDeleted() - Functional tests for soft-delete behavior - Tests verify deleted records persist in DB - Full test coverage for OnApplicationDelete 7. Documentation - Complete guide in docs/application-settings.md - Russian language documentation - Detailed examples and best practices - Architecture and concepts explained - CLI commands and API usage Key benefits: - Data retention for compliance and audit - Ability to recover accidentally deleted settings - Historical data analysis capabilities - Safe uninstall with data preservation Database schema: - Added status column (enum: active/deleted) - Added index on status for query performance - Backward compatible with existing data
- Add findByApplicationInstallationIdAndKey() as alias for findGlobalByKey() - Add findByApplicationInstallationId() as alias for findAll() - Update interface with new methods - Fixes test compatibility issues
- Add getEvents() method to retrieve pending events without clearing them - Method is useful for testing event emission - Fixes unit test errors for ApplicationSetting event tests - All 29 unit tests now passing
- Remove event testing methods from ApplicationSettingTest - testUpdateValueEmitsEvent() - testUpdateValueDoesNotEmitEventWhenValueUnchanged() - Revert AggregateRoot to original implementation (remove getEvents method) - Events are emitted via emitEvents() but not directly tested in entity tests - Follows pattern used in Bitrix24Account and other entities - All 27 unit tests passing (55 assertions)
Changes: - Fix PHPStan error in Set/Handler.php - Add intersection type annotation for AggregateRootEventsEmitterInterface - Import ApplicationSettingInterface explicitly - Apply PHP-CS-Fixer formatting to 14 files - Fix doc comment periods - Fix constructor body formatting - Fix fluent interface formatting - Move documentation from docs/ to src/ApplicationSettings/Docs/ - All 27 unit tests passing (55 assertions)
Major changes: 1. Remove getStatus() method - Removed from ApplicationSetting entity and interface - Updated all tests to use isActive() instead - Maintains encapsulation of status field 2. Refactor OnApplicationDelete to use domain methods - Removed softDeleteByApplicationInstallationId() from repository - Handler now fetches all settings and marks each as deleted - Better follows domain-driven design principles - Added deletedCount to logging 3. Remove backward compatibility methods - Removed findByApplicationInstallationIdAndKey() from repository - Updated all tests to use findGlobalByKey() directly - Cleaner repository interface 4. Add PHPDoc annotations - Added @return ApplicationSettingInterface[] to findByApplicationInstallationId() - Improves IDE support and type checking 5. Remove getRecommendedDefaults() static method - Removed from InstallSettings service - Updated documentation to reflect proper usage - Developers should define their own defaults 6. Refactor InstallSettings to use Set UseCase - Now uses Set\Handler instead of direct repository access - Follows CQRS pattern consistently - Removed direct entity instantiation - Simplified constructor (removed repository and flusher dependencies) 7. Add comprehensive unit tests for InstallSettings - Tests for default settings creation - Tests for logging behavior - Tests for global settings scope - Tests for empty settings array handling - 31 unit tests passing (66 assertions) All tests passing: - 31 unit tests with 66 assertions - PHP-CS-Fixer formatting applied - PHPStan errors only in unrelated test base classes
Changes: 1. Simplify Delete UseCase Command - Remove b24UserId and b24DepartmentId parameters - Delete now only works with global settings - Updated Handler to use findGlobalByKey() - Updated all tests 2. Fix repository method naming conflict - Rename findAll() to findAllForInstallation() - Avoids conflict with EntityRepository::findAll() - Updated all usages in UseCases and tests - Updated findByApplicationInstallationId() alias 3. Fix Doctrine XML mapping - Change enumType to enum-type (correct syntax for Doctrine ORM 3) - Fixes mapping validation errors 4. Add comprehensive contract tests for ApplicationSettingRepository - testCanFindPersonalSettingByKey - test personal scope - testCanFindDepartmentalSettingByKey - test departmental scope - testCanFindAllGlobalSettings - test global settings filtering - testCanFindAllPersonalSettings - test personal settings filtering - testCanFindAllDepartmentalSettings - test departmental settings filtering - testSoftDeletedSettingsAreNotReturnedByFindMethods - test soft-delete - testFindByKeySeparatesScopes - test scope separation - Total: 19 repository tests covering all scenarios All unit tests passing: 31 tests, 66 assertions Code style: PHP-CS-Fixer applied
Applied automated code modernization with Rector: - Added #[\Override] attributes to overridden methods - Renamed variables to match method return types - Converted closures to arrow functions where appropriate - Added return types to closures and arrow functions - Simplified boolean comparisons - Improved code readability Applied PHP-CS-Fixer formatting for consistent code style. All tests passing (31 tests, 66 assertions).
- Add PHPDoc annotations for mock types - Remove unused variable in callback - All unit tests passing (31 tests, 66 assertions) - PHPStan errors reduced from 25 to 18
Removed methods: - findAllGlobal(Uuid): array - findAllPersonal(Uuid, int): array - findAllDepartmental(Uuid, int): array - deleteByApplicationInstallationId(Uuid): void - findByApplicationInstallationId(Uuid): array Changes: - Updated ApplicationSettingRepositoryInterface - removed 5 methods - Updated ApplicationSettingRepository - removed implementations - Updated ApplicationSettingsListCommand - use findAllForInstallation with filtering - Updated tests - removed related test methods - Updated documentation - show filtering pattern The API is now simpler with single findAllForInstallation() method. Filtering by scope is done in application code using entity methods (isGlobal(), isPersonal(), isDepartmental()). All tests passing (174 tests, 288 assertions). All linters clean (PHPStan, Rector, PHP-CS-Fixer).
…rvice Major changes: - Removed 6 redundant repository methods, kept only findAllForInstallation() - Added documentation about uniqueness invariant (installation+key+user+department) - Created InMemory repository implementation for fast unit testing - Created SettingsFetcher service with cascading resolution logic (Personal > Departmental > Global) - Added comprehensive tests for InMemory repository (9 tests) - Added comprehensive tests for SettingsFetcher (10 tests) - Applied Rector and PHP-CS-Fixer improvements Files changed: - Modified ApplicationSettingRepositoryInterface: removed findGlobalByKey, findPersonalByKey, findDepartmentalByKey - Modified ApplicationSettingRepository: removed method implementations - Modified UseCase handlers to use findAllForInstallation() with filtering - Updated all functional tests to use new filtering approach - Added documentation section about invariants and uniqueness constraints - Created ApplicationSettingInMemoryRepository with helper methods - Created ApplicationSettingInMemoryRepositoryTest with 9 comprehensive tests - Created SettingsFetcher service with getSetting() and getSettingValue() methods - Created SettingsFetcherTest with 10 tests covering all override scenarios All tests pass (193 unit tests, PHPStan level 5, Rector, PHP-CS-Fixer)
…e method signatures, adjust types in `ContactPerson` entity and builder, and align with new `ContactPersonInterface` contract. Signed-off-by: mesilov <mesilov.maxim@gmail.com>
Add contact persons use case
…es, `ApplicationInstallations` updates, entity changes, and fixes. Signed-off-by: mesilov <mesilov.maxim@gmail.com>
…AcnL9 # Conflicts: # composer.json # rector.php # src/ApplicationInstallations/UseCase/OnAppInstall/Command.php # src/ApplicationInstallations/UseCase/Uninstall/Command.php # tests/EntityManagerFactory.php
…te `Domain` namespace, and refactor main repository logic for `JournalItem`.
… strings directly, refactor `JournalLogger`, and update related tests and documentation.
…ests, and minor improvements (e.g., use of constant for default label).
camaxtly
reviewed
Mar 15, 2026
src/Journal/Infrastructure/Doctrine/DoctrineDbalJournalItemRepository.php
Outdated
Show resolved
Hide resolved
camaxtly
reviewed
Mar 15, 2026
…itory` as it is no longer used.
…sage and replace PSR-3 logger methods, update documentation and tests accordingly.
…erfaces, and replace `InvalidArgumentException` usage in uninstall/install commands with SDK-specific exception
… PSR-3 level validations, update related interfaces, and adjust tests accordingly.
… of `JournalItem`.
camaxtly
reviewed
Mar 22, 2026
camaxtly
requested changes
Mar 22, 2026
src/Journal/Infrastructure/Doctrine/DoctrineDbalJournalItemRepository.php
Outdated
Show resolved
Hide resolved
| public function findByMemberId( | ||
| string $memberId, | ||
| ?string $logLevel = null, | ||
| ?int $limit = null, |
Collaborator
There was a problem hiding this comment.
Постраничка должна быть одинаковой во всех методах ––– берем стандарт из symfony
Collaborator
Author
There was a problem hiding this comment.
У нас findByMemberId и метод findByApplicationInstallationId это обычные репозиторные методы выборки, и для них array + limit/offset выглядит как по мне нормально.
А метод findWithFilters который у нас возвращает пагинацию. По идеи в readonly должен быть.
Collaborator
Author
There was a problem hiding this comment.
добавил везде пагинацию с помощью knpbundle
…tests and documentation accordingly.
…sitories and tests to replace `findById` where strict retrieval is required.
camaxtly
reviewed
Mar 29, 2026
| private string $label, | ||
| private ?array $payload = null, | ||
| private ?int $bitrix24UserId = null, | ||
| private ?IP $ipAddress = null |
camaxtly
reviewed
Mar 29, 2026
|
|
||
| return $this->payload === $other->payload | ||
| && $this->bitrix24UserId === $other->bitrix24UserId | ||
| && ( |
Collaborator
Author
There was a problem hiding this comment.
упростилось
camaxtly
reviewed
Mar 29, 2026
| private JournalContext $context | ||
| private readonly string $memberId, | ||
| private readonly Uuid $applicationInstallationId, | ||
| private readonly string $level, |
Collaborator
There was a problem hiding this comment.
Уровни логирования берем из psr-3, там тут просто нужно 7 уровней
camaxtly
reviewed
Mar 29, 2026
src/Journal/Entity/JournalItem.php
Outdated
| { | ||
| return self::create($applicationInstallationId, LogLevel::critical, $message, $context); | ||
| } | ||
| if ($this === $other) { |
Collaborator
There was a problem hiding this comment.
убираем, это можно сделать и в клиентском коде
camaxtly
reviewed
Mar 29, 2026
src/Journal/Entity/JournalItem.php
Outdated
| } | ||
|
|
||
| public static function warning(Uuid $applicationInstallationId, string $message, JournalContext $context): self | ||
| private function createdAtToMilliseconds(CarbonImmutable $date): string |
Collaborator
Author
There was a problem hiding this comment.
пока сделал функцию normalizeCreatedAt при сравнении убираем миллисекунды
camaxtly
reviewed
Mar 29, 2026
| } | ||
|
|
||
| public static function notice(Uuid $applicationInstallationId, string $message, JournalContext $context): self | ||
| private function validate(): void |
Collaborator
There was a problem hiding this comment.
Подумать ограничения на счет строк
- A-Za-z . - ограничение для лейбла
- Ограничения по хранению - utf-8/16/32
- Валидация/санитайзинг
camaxtly
reviewed
Mar 29, 2026
|
|
||
| /** | ||
| * Journal item interface for SDK contract extraction | ||
| * Journal item interface for SDK contract extraction. |
Collaborator
There was a problem hiding this comment.
изменится в соответствии с правками выше
camaxtly
reviewed
Mar 29, 2026
|
|
||
| $savedItem = $this->repository->getById($journalItem->getId()); | ||
|
|
||
| $this->assertNotNull($savedItem); |
camaxtly
requested changes
Mar 29, 2026
| $this->memberId = 'test-member-id'; | ||
| } | ||
|
|
||
| public function testCreateJournalItemWithInfoLevel(): void |
| $this->assertSame(123, $journalItem->getContext()->getBitrix24UserId()); | ||
| } | ||
|
|
||
| public function testJournalItemHasUniqueId(): void |
Collaborator
There was a problem hiding this comment.
Зачем этот тест? Что он проверяет? Пересмотреть логику
…epositories, tests, and documentation to enforce required `ipAddress`. Migrate repository methods to use pagination parameters.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
.