-
Notifications
You must be signed in to change notification settings - Fork 52
InMemoryApplicationInstallationRepositoryImplementation should resolve installations for pending new master accounts too #387
Description
Problem
InMemoryApplicationInstallationRepositoryImplementation::findByBitrix24AccountMemberId() currently resolves the installation only through a master Bitrix24 account in Bitrix24AccountStatus::active.
That is too narrow for the current installation lifecycle and makes the in-memory repository behave differently from real repository implementations.
Current code path in the in-memory repository:
- calls
Bitrix24AccountRepositoryInterface::findByMemberId(..., Bitrix24AccountStatus::active, ..., true) - returns
nullwhen the only master account is still innew
Why this is a problem
A valid two-step install flow can look like this:
Installcreates a master account innewInstallcreates an application installation innewOnAppInstallfinds that pending installation bymemberIdOnAppInstallfinishes the pair asactive
With the current in-memory implementation, step 3 fails because the lookup ignores new master accounts.
In practice this means:
- use-case/unit tests cannot rely on the SDK in-memory repository for pending install scenarios
- local test suites need custom adapters/shims
- in-memory behavior diverges from DB-backed repository behavior
Reference behavior
In our library, the Doctrine repository resolves installation by memberId for a master account without requiring the account itself to already be active, and excludes only deleted installations.
Expected behavior
findByBitrix24AccountMemberId() should be able to resolve the current installation for a master account in at least these states:
activenew
A broader and safer interpretation would be:
- search across non-deleted master accounts for the member id
- return a non-deleted installation linked to that account
- if needed, prioritize
activeovernew
Suggested fix
Update InMemoryApplicationInstallationRepositoryImplementation::findByBitrix24AccountMemberId() so it is compatible with pending install flows, and add a contract test covering:
- master account
new+ installationnew=> installation is found bymemberId - deleted installation is not returned
- if both
activeandnewexist, selection is deterministic
Impact
This would let downstream libraries use the SDK in-memory repositories directly in unit tests instead of maintaining local compatibility repositories.