Conversation
Collaborator
mesilov
commented
Dec 9, 2025
mesilov
commented
| Q | A |
|---|---|
| Bug fix? | yes/no |
| New feature? | yes |
| Deprecations? | yes |
| Issues | Fix #328 |
| License | MIT |
This commit implements the changes requested in issue #305 to make the Bitrix24 partner ID read-only. The partner ID can now only be set during entity construction and cannot be modified afterwards. Changes: - Removed setBitrix24PartnerId() method from Bitrix24PartnerInterface - Changed getBitrix24PartnerId() return type from ?int to int (non-nullable) - Updated Bitrix24PartnerReferenceEntityImplementation to use readonly int - Added validation in constructor to ensure partnerId is positive - Deprecated Bitrix24PartnerPartnerIdChangedEvent (will be removed in v2.0.0) - Removed testSetBitrix24PartnerId test - Updated all test method signatures to use int instead of ?int Breaking Changes: - partnerId is now required (not nullable) and must be set in constructor - setBitrix24PartnerId() method has been removed - getBitrix24PartnerId() now returns int instead of ?int Fixes #305
Added @phpstan-ignore-next-line annotation to testSetExternalIdWithEmptyString test method. This test intentionally passes an empty string to verify that the setExternalId method throws an InvalidArgumentException, so the PHPStan warning is a false positive.
Signed-off-by: mesilov <mesilov.maxim@gmail.com>
Removed the null check for getBitrix24PartnerId() in the save method since partnerId is now a required non-nullable int (not ?int). The check if ($bitrix24Partner->getBitrix24PartnerId() === null) would always evaluate to false after making partnerId read-only and required. Now the method always validates uniqueness of the partnerId, which is the correct behavior since partnerId is always present.
…3KEY9rHRZdrRGyn feat: Make Bitrix24 partner ID read-only (issue #305)
Add VersionedScope class for managing versioned Scopes with unit tests
Fixes #126 MOVED_TIME field in DealItemResult and LeadItemResult was documented as CarbonImmutable but was actually returning int due to incorrect casting logic in AbstractCrmItem. Changes: - Moved MOVED_TIME from integer casting block to datetime casting block in AbstractCrmItem::__get() - Added unit tests to verify MOVED_TIME returns CarbonImmutable - Tests also cover other datetime fields to ensure consistency The field now correctly returns CarbonImmutable, matching the API documentation and expected behavior.
Applied Rector rules: - NewlineAfterStatementRector: Added blank lines after return statements for better readability - FlipTypeControlToUseExclusiveTypeRector: Improved type checking in constructor (instanceof instead of !== null) - RemoveUselessParamTagRector: Removed redundant @param tags - RenameVariableToMatchNewTypeRector: Renamed $item to $testCrmItem in unit tests for better clarity No functional changes, only code quality improvements.
…pZuJ7BL1j4nWb3oghm Fix MOVED_TIME to return CarbonImmutable instead of int
Signed-off-by: mesilov <mesilov.maxim@gmail.com>
Signed-off-by: mesilov <mesilov.maxim@gmail.com>
…ions Feature/310 add claude instructions
- Add ApplicationSettingsItemInterface entity contract with support for: - Global, user-scoped, and department-scoped settings - CRUD operations and soft delete - Setting status tracking - Add ApplicationSettingStatus enum (active/deleted) - Add events: - ApplicationSettingsItemCreatedEvent - ApplicationSettingsItemChangedEvent - ApplicationSettingsItemDeletedEvent - Add ApplicationSettingsItemNotFoundException exception - Add ApplicationSettingsItemRepositoryInterface with methods for: - Basic CRUD operations - Finding settings by various criteria - Global, personal, and departmental scope filtering - Add comprehensive test coverage: - ApplicationSettingsItemInterfaceTest - ApplicationSettingsItemRepositoryInterfaceTest Closes #304
- Remove useless @param and @return PHPDoc tags (types already declared) - Rename parameters to match type (applicationInstallationId -> uuid) - Rename variables to match return type (item -> applicationSettingsItem) - Add readonly keyword to test class properties - Add newlines after statements for better readability Applied Rector rules: - RemoveUselessParamTagRector - RemoveUselessReturnTagRector - RenameParamToMatchTypeRector - RenameVariableToMatchMethodCallReturnTypeRector - ReadOnlyPropertyRector - NewlineAfterStatementRector
Remove the following methods from ApplicationSettingsItemRepositoryInterface: - findByApplicationInstallationIdAndKeyAndUserId - findByApplicationInstallationIdAndKeyAndDepartmentId - findGlobalByApplicationInstallationIdAndKey These specific finder methods are removed in favor of using the generic findByApplicationInstallationIdAndKey method which returns all settings with the same key across different scopes. The consumer can then filter the results based on their needs. Also removed corresponding tests and data providers: - testFindByApplicationInstallationIdAndKeyAndUserId - testFindByApplicationInstallationIdAndKeyAndDepartmentId - testFindGlobalByApplicationInstallationIdAndKey - personalSettingDataProvider - departmentalSettingDataProvider - globalSettingDataProvider
Rename method in ApplicationSettingsItemRepositoryInterface: - findAllByApplicationInstallationId -> findAllForInstallation Update corresponding test: - testFindAllByApplicationInstallationId -> testFindAllForInstallation This provides a shorter, more concise method name while maintaining clarity.
…nByKey Complete renaming of repository method for consistency: - Update method signature in ApplicationSettingsItemRepositoryInterface - Update all test method names and references - Update TestDox attributes to reflect new method name
Create comprehensive documentation for ApplicationSettings entity: - Entity methods table with descriptions - Scope types explanation (global, personal, departmental) - State diagram for setting lifecycle - Repository methods with use cases - Events timeline diagram
…aceTest - Rename createApplicationSettingsItemRepositoryImplementation to createApplicationSettingsRepositoryImplementation - Reorder abstract methods: createRepositoryFlusherImplementation first - Update all method calls to use new name - Align with naming pattern used in other repository tests
…on in tests - Remove static createTestItem method with anonymous class - Update data providers to return primitive data arrays instead of objects - Modify test methods to create items using createApplicationSettingsItemImplementation - Align with repository test patterns from other entities
Document new ApplicationSettings contracts in Unreleased section: - Entity interface with multi-scope support - Repository interface with CRUD and scope-based queries - Status enum and events for lifecycle tracking - Exception handling and comprehensive tests - Complete documentation
Resolve merge conflict in CHANGELOG.md: - Move ApplicationSettings contracts from Unreleased to 1.9.0 release - Combine with existing 1.9.0 changes (VersionedScope, MCP server, MOVED_TIME fix) - Maintain chronological order of releases
…8c7FAS7sabZc351hfS97J Add ApplicationSettings contracts and tests
Added the missing @property-read bool $active annotation to FlowItemResult.php to match the Bitrix24 API documentation for task.flow.Flow.get method. This fixes invalid typecasting hints reported in issue #275.
…1K5f6oAJNNThYdaxwWx51Qp
This commit comprehensively fixes all type casting issues in FlowItemResult by aligning type annotations with the official Bitrix24 API documentation for task.flow.Flow.get method: - Added missing @property-read bool $active annotation - Corrected 9 incorrectly nullable types to match API spec: * responsibleList: array|null → array * demo: bool|null → bool * responsibleCanChangeDeadline: bool|null → bool * matchWorkTime: bool|null → bool * taskControl: bool|null → bool * notifyAtHalfTime: bool|null → bool * taskCreators: array|null → array * team: array|null → array * trialFeatureEnabled: bool|null → bool Verified against official Bitrix24 API documentation (RU): https://apidocs.bitrix24.ru/api-reference/tasks/flow/tasks-flow-flow-get.html
…NThYdaxwWx51Qp Claude/fix issue 275
Update composer dependency constraint to include version 6.x while maintaining backward compatibility with versions 4.x and 5.x. Version 6.0.0 was released on 2025-02-10 and is compatible with PHP 7.1+, which exceeds the project requirements (PHP 8.2+). All 482 unit tests pass successfully with the new version. Fixes #236
Document the addition of darsyn/ip version 6.x support in the Unreleased section of the changelog. Related to #236
…1HZw Merge latest changes from dev branch including: - ApplicationSettings contracts (issue #310) - MCP server configuration (issue #126) - MOVED_TIME field fix (issue #126) - FlowItemResult type hints fix (issue #275) - VersionedScope class Resolved conflicts in CHANGELOG.md by integrating darsyn/ip v6.x support into the 1.9.0 release section.
…11kx8bSKof8Kor1HZw Add support for darsyn/ip version 6.x
Added specialized exceptions for different OAuth token refresh failure scenarios: - InvalidGrantException - for invalid/expired refresh tokens (requires user re-authorization) - PortalDomainNotFoundException - for non-existent or inaccessible portals Improved ApiClient::getNewAuthToken() method: - Replaced generic error messages with specific exception types based on HTTP status codes - Added detailed error handling for different OAuth error codes (invalid_grant, invalid_client, etc.) - Enhanced error messages with both OAuth error code and description - Added support for HTTP 400, 401, 404, and 5xx status codes - Developers can now distinguish between different failure causes and implement specific recovery logic Added comprehensive unit tests: - Tests for all new exception types - Tests for different HTTP status codes and OAuth error scenarios - Success case test for token refresh Updated CHANGELOG.md with details about the improvements. Resolves: #284
- Apply Rector code quality improvements (UnwrapSprintfOneArgumentRector, RenameVariableToMatchMethodCallReturnTypeRector) - Fix testGetNewAuthTokenSuccess: add required 'expires' field to mock response - Rename test parameter from $expectedException to $throwable for consistency - All unit tests pass (522 tests, 921 assertions)
Centralized OAuth token refresh error handling by moving logic from ApiClient::getNewAuthToken() to a new ApiLevelErrorHandler::handleOAuthError() method. Changes: - Added ApiLevelErrorHandler::handleOAuthError() method for centralized OAuth error handling - Simplified ApiClient::getNewAuthToken() from ~120 lines to ~20 lines - Method now delegates all error handling to ApiLevelErrorHandler - Improved code maintainability and reduced duplication - Added 'never' return type to handleOAuthError() for PHPStan compatibility Benefits: - Single source of truth for OAuth error handling logic - Easier to maintain and test error handling - Consistent error handling patterns across the SDK - Better separation of concerns All tests pass (522 tests, 921 assertions) All linters pass (PHP CS Fixer, PHPStan, Rector) Related: #284
…esh-errors-01Kf3SaMBYEyTB8nPuYokyK2 Fix: Improve error handling during OAuth token refresh (#284)
The test now properly marks the first contact person's email as verified before flushing, ensuring the findByEmail method correctly filters by verified emails in the database. Fixes #316
…elete This ensures the test accurately reflects actual system behavior by persisting the deletion to the database before verifying the exception. Fixes #314
Added entry for the testFindByEmailWithVerifiedEmail test fix that ensures emails are properly marked as verified before flushing to the database.
Resolved conflict in CHANGELOG.md - kept fix for issue #316
…AktUZHagXagBiT Add markEmailAsVerified() call in testFindByEmailWithVerifiedEmail test
- Merged latest changes from dev branch - Added changelog entry for testDelete flush fix
…axbrSKYtrNbdzSRxw7 Fix issue 314 in B24 PHP SDK
Add markMobilePhoneAsVerified() call for the first contact person after save and before flush, ensuring the test correctly validates the findByPhone method with onlyVerified=true flag. Fixes #315
…LN3Ff3P4qVfQkhxLwf Fix testFindByEmailWithVerifiedPhone to mark phone as verified
Signed-off-by: mesilov <mesilov.maxim@gmail.com>
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
2 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.