From 3190b3266ee873a550dd5d173c725840619331f3 Mon Sep 17 00:00:00 2001 From: mesilov Date: Sat, 14 Mar 2026 13:46:04 +0600 Subject: [PATCH 1/8] Add MCP server configuration and documentation: introduce `.mcp.json` for project-level server settings, update `.gitignore`, and enhance project docs with MCP usage instructions. Signed-off-by: mesilov --- .gitignore | 3 ++- .mcp.json | 8 ++++++++ AGENTS.md | 17 +++++++++++++++++ CLAUDE.md | 9 ++++++++- README.md | 18 +++++++++++++++++- 5 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 .mcp.json create mode 100644 AGENTS.md diff --git a/.gitignore b/.gitignore index b5709f1c..de1f5018 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ /.idea* +.DS_Store /vendor /.cache /docker/db @@ -7,4 +8,4 @@ composer.lock .phpunit.result.cache *.log .env.local -*.cache \ No newline at end of file +*.cache diff --git a/.mcp.json b/.mcp.json new file mode 100644 index 00000000..cdef2c45 --- /dev/null +++ b/.mcp.json @@ -0,0 +1,8 @@ +{ + "mcpServers": { + "bitrix24-dev": { + "type": "http", + "url": "https://mcp-dev.bitrix24.tech/mcp" + } + } +} \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..09f246dc --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,17 @@ +# AGENTS + +## MCP Servers + +This repository includes project-level MCP server configuration in `.mcp.json`. + +Developers and agents working with this project must verify the MCP configuration before starting work. + +Configured servers: + +- `bitrix24-dev` - HTTP MCP server at `https://mcp-dev.bitrix24.tech/mcp` + +Checks before work starts: + +- ensure `.mcp.json` is present and contains the expected server list +- restart the client after pulling changes to `.mcp.json` +- verify that the configured MCP servers are available in the current client diff --git a/CLAUDE.md b/CLAUDE.md index 46add905..39ba9f3b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -119,6 +119,13 @@ src/ ## Environment Variables Located in `.env` and `.env.local` files for database configuration. +## MCP Servers +- Project-level MCP configuration is stored in `.mcp.json`. +- Developers using Claude Code or Codex must verify the MCP configuration before starting work in this repository. +- Configured servers: + - `bitrix24-dev` - HTTP MCP server at `https://mcp-dev.bitrix24.tech/mcp` +- If `.mcp.json` changes, restart the client so the updated project MCP configuration is reloaded. + ### Default Configuration for Functional Tests The `.env` file contains default values that work out-of-the-box with Docker Compose: - `DATABASE_HOST=database` (Docker service name) @@ -128,4 +135,4 @@ The `.env` file contains default values that work out-of-the-box with Docker Com - `POSTGRES_VERSION=16` These defaults allow running functional tests immediately after `make up` without additional configuration. -- Always update changelog.md \ No newline at end of file +- Always update changelog.md diff --git a/README.md b/README.md index ff5444bd..d330b023 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,22 @@ src/ - Docker and Docker Compose - Make +### MCP Servers + +The project contains project-level MCP server configuration in `.mcp.json`. + +Developers using Claude Code or Codex must verify the MCP configuration before starting work on the repository. + +Configured servers: + +- `bitrix24-dev` - HTTP MCP server at `https://mcp-dev.bitrix24.tech/mcp` + +Recommended checks: + +- ensure `.mcp.json` is present and contains the expected server list +- restart the client after pulling changes to `.mcp.json` +- verify server availability in the client before work starts + ### Running Tests ```bash @@ -128,4 +144,4 @@ No additional configuration needed for running tests. 2. Library is covered with tests 3. All work is organized through issues 4. Development processes are remote first -5. Think and discuss — then write \ No newline at end of file +5. Think and discuss — then write From 0c207e905650f9286e5a4cf8fa5b356611255535 Mon Sep 17 00:00:00 2001 From: mesilov Date: Sat, 14 Mar 2026 14:22:16 +0600 Subject: [PATCH 2/8] Update developer documentation for onboarding and AI/CLI tooling: add setup steps for `gh` CLI, GitHub MCP, and MCP servers; synchronize `README.md`, `AGENTS.md`, and `docs/tech-stack.md`; introduce environment readiness checklist. Signed-off-by: mesilov --- .tasks/90/developer-docs-update-plan.md | 96 +++++++++++++++++++++++++ CHANGELOG.md | 8 ++- 2 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 .tasks/90/developer-docs-update-plan.md diff --git a/.tasks/90/developer-docs-update-plan.md b/.tasks/90/developer-docs-update-plan.md new file mode 100644 index 00000000..70f56c79 --- /dev/null +++ b/.tasks/90/developer-docs-update-plan.md @@ -0,0 +1,96 @@ +## План обновления документации для разработчика (`gh` CLI, GitHub MCP, Claude Code) + +### Summary +Цель: обновить документацию проекта так, чтобы новый разработчик или агент мог без устных пояснений подготовить рабочее окружение для работы с репозиторием, включая `gh` CLI, GitHub MCP и project MCP `bitrix24-dev`. + +Текущее состояние: +- `README.md` и `AGENTS.md` уже описывают project MCP `bitrix24-dev`. +- В документации пока нет явного требования установить и настроить GitHub MCP. +- В документации пока нет явного требования установить и авторизовать `gh` CLI. +- Нужно отдельно зафиксировать требования для работы через Claude Code. + +### Important Changes / Interfaces +1. Обновить основной онбординг в `README.md` +- Добавить раздел `Developer Setup` или `AI/CLI Tooling`. +- Явно перечислить обязательные инструменты: + - `gh` CLI + - GitHub MCP server + - `bitrix24-dev` MCP server + - Docker / Make +- Добавить короткий smoke-check перед началом работы. + +2. Добавить требования для Claude Code +- Вынести отдельный подраздел `Claude Code`. +- Зафиксировать, что для полноценной работы клиент должен видеть: + - `bitrix24-dev` + - GitHub MCP +- Отдельно указать, что без установленного и авторизованного `gh` CLI GitHub-сценарии будут неполными или недоступными. + +3. Синхронизировать `AGENTS.md` +- Расширить требования к окружению агентов. +- Добавить ожидание наличия GitHub MCP. +- Добавить требование по `gh` CLI там, где агент работает с GitHub-задачами. +- Сохранить правило про проверку `.mcp.json` и перезапуск клиента после его изменения. + +4. Расширить справочную документацию в `docs/tech-stack.md` +- Добавить раздел про developer tooling. +- Кратко описать назначение: + - `gh` CLI для работы с GitHub из терминала + - GitHub MCP для работы агента с GitHub-контекстом + - `bitrix24-dev` MCP для доступа к Bitrix24-документации + +### Implementation Plan +1. Обновить `README.md` как главный документ быстрого старта: +- добавить обязательные prerequisites для developer tooling; +- описать установку и базовую проверку `gh`; +- описать обязательные MCP для Codex / Claude Code; +- добавить checklist перед началом работы. + +2. Обновить `AGENTS.md`: +- дополнить список ожидаемых MCP-серверов; +- зафиксировать, что при работе с GitHub-задачами агент должен иметь доступ к GitHub MCP и `gh` CLI; +- оставить существующие проверки `.mcp.json` и client restart после изменений. + +3. Обновить `docs/tech-stack.md`: +- добавить отдельный раздел `Developer Tooling`; +- перечислить `gh`, GitHub MCP, `bitrix24-dev`; +- кратко пояснить, для каких задач используется каждый инструмент. + +4. Добавить единый checklist готовности окружения: +- `.mcp.json` существует и актуален; +- в клиенте доступен `bitrix24-dev`; +- в клиенте доступен GitHub MCP; +- `gh --version` отрабатывает; +- `gh auth status` показывает успешную авторизацию; +- локальные команды проекта запускаются. + +### Recommended Wording +- `Для полноценной работы с репозиторием в Claude Code, Codex и других AI-клиентах требуется установить и настроить GitHub MCP server и gh CLI.` +- `Помимо project MCP bitrix24-dev, клиент должен видеть GitHub MCP.` +- `После изменения .mcp.json необходимо перезапустить клиент и повторно проверить доступность MCP серверов.` +- `Если gh CLI не установлен или не авторизован, GitHub-сценарии разработки и агентной работы будут ограничены.` + +### Ownership of Documentation +- `README.md` отвечает за быстрый старт и обязательные шаги онбординга. +- `AGENTS.md` отвечает за требования к агентам и MCP-проверки. +- `docs/tech-stack.md` отвечает за справочный контекст и описание используемых инструментов. + +### Test Cases and Scenarios +1. Новый разработчик открывает `README.md` и без дополнительных пояснений: +- устанавливает `gh`; +- проходит `gh auth login`; +- проверяет `gh auth status`; +- проверяет наличие `bitrix24-dev` и GitHub MCP в клиенте; +- запускает базовые команды проекта. + +2. Разработчик в Claude Code: +- видит в документации явное требование по GitHub MCP; +- понимает, что одного project MCP недостаточно; +- знает, что после обновления `.mcp.json` нужен restart клиента. + +3. Агентная работа: +- `AGENTS.md` не расходится с `README.md`; +- требования к MCP и `gh` CLI описаны одинаково и без противоречий. + +### Definition of Done +Документация считается обновлённой, когда новый разработчик может с нуля открыть `README.md`, установить `gh`, подключить GitHub MCP, проверить `bitrix24-dev` и GitHub MCP, после чего начать работу с проектом без дополнительных устных инструкций. diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a15d611..ad0d7756 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,10 @@ -## Unreleased +## Unreleased 0.5.0 + +### Fixed +- Fix start state for Bitrix24 ApplicationAccounts and ApplicationInstall [#90](https://github.com/mesilov/bitrix24-php-lib/issues/90) + + + ## 0.4.0 From 83ad55a1d792af134cd896d5b2e73eaa4eca64db Mon Sep 17 00:00:00 2001 From: mesilov Date: Sun, 15 Mar 2026 01:31:43 +0600 Subject: [PATCH 3/8] Refactor `Install` and `OnAppInstall` use cases to support single-step and two-step application installation flows: adjust status transitions, consolidate finish-step handling, and update developer documentation with user stories, sequence diagrams, and invariants. Signed-off-by: mesilov --- .tasks/90/developer-docs-update-plan.md | 253 ++++++++++++++++++- .tasks/90/install-handler-status-fix-plan.md | 109 ++++++++ 2 files changed, 360 insertions(+), 2 deletions(-) create mode 100644 .tasks/90/install-handler-status-fix-plan.md diff --git a/.tasks/90/developer-docs-update-plan.md b/.tasks/90/developer-docs-update-plan.md index 70f56c79..d076d076 100644 --- a/.tasks/90/developer-docs-update-plan.md +++ b/.tasks/90/developer-docs-update-plan.md @@ -1,13 +1,19 @@ ## План обновления документации для разработчика (`gh` CLI, GitHub MCP, Claude Code) ### Summary -Цель: обновить документацию проекта так, чтобы новый разработчик или агент мог без устных пояснений подготовить рабочее окружение для работы с репозиторием, включая `gh` CLI, GitHub MCP и project MCP `bitrix24-dev`. +Цель: обновить документацию проекта так, чтобы новый разработчик или агент мог без устных пояснений: +- подготовить рабочее окружение для работы с репозиторием, включая `gh` CLI, GitHub MCP и project MCP `bitrix24-dev`; +- понять два поддерживаемых сценария установки приложения в `ApplicationInstallations`; +- получить sequence diagrams и описание use case'ов; +- опереться на unit-тесты, которые фиксируют инварианты доменной модели для одношаговой и двухшаговой установки. Текущее состояние: - `README.md` и `AGENTS.md` уже описывают project MCP `bitrix24-dev`. - В документации пока нет явного требования установить и настроить GitHub MCP. - В документации пока нет явного требования установить и авторизовать `gh` CLI. - Нужно отдельно зафиксировать требования для работы через Claude Code. +- В `ApplicationInstallations` нет локального документа с описанием одношаговой и двухшаговой установки. +- Текущий `Install` handler завершает установку сразу даже без `application_token`, поэтому поведение для двухшагового сценария нужно явно изменить и зафиксировать в документации и тестах. ### Important Changes / Interfaces 1. Обновить основной онбординг в `README.md` @@ -39,6 +45,35 @@ - GitHub MCP для работы агента с GitHub-контекстом - `bitrix24-dev` MCP для доступа к Bitrix24-документации +5. Уточнить контракт `ApplicationInstallations\UseCase\Install` +- Явно разделить два сценария: + - одношаговая установка: `Install` завершает установку сразу, только если `application_token` пришёл в команду; + - двухшаговая установка: `Install` только создаёт агрегаты в статусе `new`, а завершение делает `OnAppInstall`. + +6. Уточнить контракт `ApplicationInstallations\UseCase\OnAppInstall` +- Сделать `OnAppInstall` use case ответственным за завершение установки в двухшаговом сценарии. +- После получения `application_token` use case должен переводить агрегаты из `new` в `active` и сохранять токен. +- Для двухшагового сценария `OnAppInstall` должен искать master account в состоянии `new`, а не `active`, потому что до прихода события установка ещё не завершена. +- Завершение двухшаговой установки обязательно должно происходить через вызов use case `OnAppInstall`; обход этого use case прямыми вызовами методов агрегатов вне сценария запрещён. +- Если событие `ONAPPINSTALL` приходит повторно для уже завершённой установки, use case ничего не делает, пишет `warning` в лог и завершает обработку как `no-op`. +- Если событие `ONAPPINSTALL` не приходит для pending-инсталляции, это не решается в рамках текущей задачи кодом use case; нужно создать GitHub issue на проектирование фонового сборщика битых инсталляций. + +7. Добавить локальную документацию по use case'ам установки +- Создать файл `src/ApplicationInstallations/Docs/application-installations.md`. +- Описать там оба user story. +- Дать ссылки на внешний контракт: + - `https://github.com/bitrix24/b24phpsdk/blob/v3/src/Application/Contracts/ApplicationInstallations/Docs/ApplicationInstallations.md` + - `https://apidocs.bitrix24.com/api-reference/common/events/on-app-install.html` + +8. Добавить sequence diagrams +- В `src/ApplicationInstallations/Docs/application-installations.md` положить две sequence diagram в формате Mermaid: + - `US1: Одношаговая установка` + - `US2: Двухшаговая установка через ONAPPINSTALL` + +9. Добавить unit-тесты на инварианты use case'ов +- Написать unit-тесты для `Install` и `OnAppInstall`, которые работают через in-memory repositories. +- Для `US2` проверять состояние после каждого шага: после `Install` и после `OnAppInstall`. + ### Implementation Plan 1. Обновить `README.md` как главный документ быстрого старта: - добавить обязательные prerequisites для developer tooling; @@ -64,6 +99,205 @@ - `gh auth status` показывает успешную авторизацию; - локальные команды проекта запускаются. +5. Изменить доменное поведение `ApplicationInstallations\UseCase\Install`: +- если `applicationToken` передан, завершать установку сразу; +- если `applicationToken` не передан, создавать `Bitrix24Account` и `ApplicationInstallation` в статусе `new` без финализации установки; +- вариант принудительного завершения установки без `applicationToken` в план не входит и не реализуется. +- если по тому же `memberId` уже существует pending-инсталляция в статусе `new`, повторный вызов `Install` должен переводить старые записи `Bitrix24Account` и `ApplicationInstallation` в статус `deleted`, а затем создавать новую пару записей. + +6. Изменить доменное поведение `ApplicationInstallations\UseCase\OnAppInstall`: +- use case должен находить агрегаты, созданные на первом шаге; +- master account должен искаться среди записей в статусе `new`, чтобы pending-установка могла быть завершена корректно; +- сохранять `applicationToken` в `Bitrix24Account` и `ApplicationInstallation`; +- переводить оба агрегата в финальное состояние установки; +- обновлять `applicationStatus` у `ApplicationInstallation`. +- Этот use case является обязательной точкой входа для завершения `US2`; никакой альтернативный путь финализации установки не допускается. +- при повторном событии для уже завершённой установки use case должен отработать как `no-op` и записать `warning` в лог; +- отсутствие события для зависшей pending-инсталляции не закрывается в этом change set, а выносится в отдельное GitHub-обсуждение. + +7. Инициировать обсуждение по битым инсталляциям в GitHub +- Создать отдельный GitHub issue на проектирование фонового сборщика битых инсталляций, для которых `Install` уже создал записи в статусе `new`, но `ONAPPINSTALL` не был доставлен. +- В issue зафиксировать, что нужно обсудить и выбрать стратегию восстановления. +- Варианты для обсуждения: + - периодический worker, который ищет `new`-инсталляции старше заданного TTL и переводит их в специальный failed/broken сценарий; + - периодический worker, который ищет `new`-инсталляции старше TTL и только создаёт alert/issue/notification без автоматической смены статуса; + - reconciliation job, который пытается повторно сверить состояние установки через доступные внешние признаки и только потом принимает решение; + - ручной operational flow: список зависших инсталляций + консольная команда/админский action для разбора. + +7. Добавить локальную документацию по установке приложения: +- создать `src/ApplicationInstallations/Docs/application-installations.md`; +- описать `US1` и `US2`; +- добавить ссылки на внешний контракт `b24phpsdk` и на документацию события `ONAPPINSTALL`; +- положить в документ две Mermaid sequence diagram. + +8. Подготовить unit-тестовую инфраструктуру: +- добавить in-memory репозиторий для `ApplicationInstallationRepositoryInterface`; +- добавить in-memory репозиторий для `Bitrix24AccountRepositoryInterface`; +- при необходимости добавить test double для `Flusher`, чтобы unit-тесты проверяли только состояние агрегатов. + +9. Написать unit-тесты на user stories и инварианты: +- отдельный тест на `US1` с `applicationToken`; +- отдельный сценарный тест на `US2`, где последовательно вызываются `Install`, затем `OnAppInstall`, и проверяется состояние после каждого шага. + +### ApplicationInstallations User Stories +#### US1. Одношаговая установка +Сценарий: +- вызывается `Install`; +- на событие `ONAPPINSTALL` мы не подписываемся; +- установка финализируется сразу через `Install`, потому что `application_token` уже пришёл в команду; +- это используется в случае, когда приложение ставится без UI и `application_token` прилетает сразу. + +Ожидаемое поведение: +- при наличии `applicationToken` use case завершает установку сразу и сохраняет токен; +- `Bitrix24Account` после `Install` находится в `active`; +- `ApplicationInstallation` после `Install` находится в `active`. + +```mermaid +sequenceDiagram + autonumber + actor Installer as Installer / External Caller + participant Install as UseCase Install + participant AccountRepo as Bitrix24AccountRepository + participant InstallRepo as ApplicationInstallationRepository + + Installer->>Install: Install(command with applicationToken) + Install->>AccountRepo: create Bitrix24Account + Install->>InstallRepo: create ApplicationInstallation + Install->>Install: finalize installation immediately + Install->>AccountRepo: save active account (+ token if present) + Install->>InstallRepo: save active installation (+ token if present) + Install-->>Installer: installation finished in one step +``` + +#### US2. Двухшаговая установка +Сценарий: +- сначала вызывается `Install`; +- use case создаёт агрегаты без токена в статусе `new`; +- приложение подписывается на событие `ONAPPINSTALL`; +- позже на endpoint приходит событие с `application_token`; +- вызывается `OnAppInstall`, который завершает установку. + +Ожидаемое поведение: +- после первого шага `Bitrix24Account` находится в `new`; +- после первого шага `ApplicationInstallation` находится в `new`; +- после первого шага токен отсутствует; +- `OnAppInstall` ищет master account именно в статусе `new`; +- завершение установки выполняется обязательным вызовом `OnAppInstall`; +- если во второй вкладке запускают новую установку до прихода `ONAPPINSTALL` по первой, второй вызов `Install` переводит старые pending-записи в `deleted` и создаёт новые записи для новой попытки установки; +- если `ONAPPINSTALL` не пришёл, инсталляция остаётся pending; дальнейшая стратегия выносится в отдельный GitHub issue про фоновый сборщик битых инсталляций; +- если `ONAPPINSTALL` пришёл повторно после успешного завершения установки, `OnAppInstall` ничего не делает и пишет `warning` в лог; +- после вызова `OnAppInstall` токен сохранён; +- после вызова `OnAppInstall` оба агрегата находятся в `active`. + +```mermaid +sequenceDiagram + autonumber + actor UI as Bitrix24 UI / install.php + participant Install as UseCase Install + participant AccountRepo as Bitrix24AccountRepository + participant InstallRepo as ApplicationInstallationRepository + participant Event as ONAPPINSTALL event + participant OnAppInstall as UseCase OnAppInstall + + UI->>Install: Install(command without applicationToken) + Install->>AccountRepo: save Bitrix24Account(status=new) + Install->>InstallRepo: save ApplicationInstallation(status=new) + Install-->>UI: wait for ONAPPINSTALL + Event->>OnAppInstall: OnAppInstall(memberId, applicationToken, applicationStatus) + OnAppInstall->>AccountRepo: load account by memberId + OnAppInstall->>InstallRepo: load installation by memberId + OnAppInstall->>OnAppInstall: set token + finish installation + OnAppInstall->>AccountRepo: save active account + OnAppInstall->>InstallRepo: save active installation +``` + +### Documentation Deliverable +- Новый локальный документ: `src/ApplicationInstallations/Docs/application-installations.md`. +- Содержимое документа: + - назначение `Install` и `OnAppInstall`; + - описание `US1` и `US2`; + - обе sequence diagram; + - явное различие между одношаговой и двухшаговой установкой; + - явное правило, что для `US2` завершение установки всегда выполняется через use case `OnAppInstall`; + - правило реинсталляции: если pending-установка в статусе `new` уже существует и пользователь запускает новую установку, старые записи переводятся в `deleted`, после чего создаются новые; + - правило обработки повторного `ONAPPINSTALL`: `warning + no-op`; + - ссылка на отдельный GitHub issue по проектированию фонового сборщика битых инсталляций; + - ссылка на контракт в `b24phpsdk`: + `https://github.com/bitrix24/b24phpsdk/blob/v3/src/Application/Contracts/ApplicationInstallations/Docs/ApplicationInstallations.md`; + - ссылка на событие `ONAPPINSTALL`: + `https://apidocs.bitrix24.com/api-reference/common/events/on-app-install.html`. + +### Unit Tests and Invariants +1. Базовые сценарии `Install` +- `US1: Install with applicationToken` +- вызвать `Install`; +- проверить в in-memory repo, что `Bitrix24Account` сохранён в статусе `active`; +- проверить в in-memory repo, что `ApplicationInstallation` сохранён в статусе `active`; +- проверить, что токен сохранён в обоих агрегатах. +- `US2: Install without token` +- вызвать `Install`; +- сразу после вызова проверить, что `Bitrix24Account` в статусе `new`; +- сразу после вызова проверить, что `ApplicationInstallation` в статусе `new`; +- проверить, что токен не сохранён. + +2. Базовые сценарии `OnAppInstall` +- `US2: OnAppInstall finalizes pending installation` +- после предыдущего шага вызвать `OnAppInstall`; +- проверить, что use case находит master account в статусе `new`, а не ожидает `active`; +- проверить, что финализация сценария происходит именно через вызов `OnAppInstall`; +- проверить, что `Bitrix24Account` перешёл в `active`; +- проверить, что `ApplicationInstallation` перешёл в `active`; +- проверить, что токен сохранён; +- проверить, что `applicationStatus` у `ApplicationInstallation` обновился значением из события. + +3. Corner cases `Install` +- `reinstall while previous installation is still new` +- выполнить первый `Install` без токена и убедиться, что созданы записи в статусе `new`; +- выполнить второй `Install` без токена с тем же `memberId`; +- проверить, что первая пара записей `Bitrix24Account` и `ApplicationInstallation` переведена в `deleted`; +- проверить, что создана новая пара записей; +- проверить, что новая пара записей находится в статусе `new`. +- `reinstall while previous installation is already active` +- подготовить завершённую установку по тому же `memberId`; +- выполнить новый `Install`; +- проверить, что предыдущие записи переведены в `deleted`; +- проверить, что создана новая пара записей; +- проверить, что статус новой пары зависит от наличия `applicationToken` в новом вызове. +- `invalid install payload` +- покрыть unit-тестами `Install\Command` критичные невалидные комбинации входных данных, которые влияют на оба сценария установки; +- отдельно проверить пустой `memberId`, невалидный `bitrix24UserId`, невалидный `applicationVersion`, пустой `applicationToken`, если он передан. + +4. Corner cases `OnAppInstall` +- `duplicate ONAPPINSTALL event` +- после успешного завершения `US2` повторно вызвать `OnAppInstall` с тем же событием; +- проверить, что состояние агрегатов не меняется; +- проверить, что use case не падает; +- проверить, что в лог пишется `warning`; +- проверить, что обработка повторного события выполняется как `no-op`. +- `ONAPPINSTALL for missing pending installation` +- вызвать `OnAppInstall`, когда pending installation по `memberId` отсутствует; +- проверить ожидаемое доменное поведение use case: либо контролируемое исключение, либо `warning + no-op`, в зависимости от финального решения по контракту; +- это поведение должно быть явно зафиксировано в документации и тестах, без неявной логики. +- `ONAPPINSTALL when account is not in status new` +- подготовить данные, в которых installation найдена, но master account не находится в `new`; +- проверить, что use case не делает частичного обновления состояния; +- проверить согласованное поведение: controlled exception или `warning + no-op`, в зависимости от финального контракта. +- `ONAPPINSTALL with token mismatch / repeated different token` +- после завершения установки вызвать `OnAppInstall` с другим `applicationToken`; +- проверить, что состояние агрегатов не переписывается бесконтрольно; +- проверить, что use case пишет `warning` и не нарушает инварианты. + +5. Lifecycle cases вне синхронных unit-тестов +- `missing ONAPPINSTALL event` +- отдельным unit-тестом не покрывается, потому что это не поведение синхронного use case, а вопрос lifecycle management; +- вместо этого в deliverables входит создание GitHub issue на проектирование фонового сборщика битых инсталляций и обсуждение вариантов восстановления. + +6. Тестовые артефакты +- unit-тесты разместить рядом с use case'ами в `tests/Unit/ApplicationInstallations/UseCase/...`; +- in-memory repositories разместить в `tests/Helpers/...` по аналогии с существующими test helpers; +- добавить test double для logger, чтобы явно проверять `warning` в no-op сценариях; +- если потребуется, добавить отдельный сценарный unit-тест, покрывающий полный поток `Install -> OnAppInstall`. + ### Recommended Wording - `Для полноценной работы с репозиторием в Claude Code, Codex и других AI-клиентах требуется установить и настроить GitHub MCP server и gh CLI.` - `Помимо project MCP bitrix24-dev, клиент должен видеть GitHub MCP.` @@ -92,5 +326,20 @@ - `AGENTS.md` не расходится с `README.md`; - требования к MCP и `gh` CLI описаны одинаково и без противоречий. +4. Документация `ApplicationInstallations`: +- в репозитории появляется `src/ApplicationInstallations/Docs/application-installations.md`; +- в нём описаны `US1` и `US2`; +- обе диаграммы читаемы и отражают фактическое поведение use case'ов; +- документ ссылается на контракт из `b24phpsdk` и на `ONAPPINSTALL` документацию. + +5. Unit-тесты по user story: +- `US1` проверяет немедленную финализацию установки; +- `US2` проверяет промежуточное состояние после `Install` и финальное состояние после `OnAppInstall`; +- тесты работают без Doctrine и без БД, только через in-memory repositories. + ### Definition of Done -Документация считается обновлённой, когда новый разработчик может с нуля открыть `README.md`, установить `gh`, подключить GitHub MCP, проверить `bitrix24-dev` и GitHub MCP, после чего начать работу с проектом без дополнительных устных инструкций. +План считается реализованным, когда: +- новый разработчик может с нуля открыть `README.md`, установить `gh`, подключить GitHub MCP, проверить `bitrix24-dev` и GitHub MCP, после чего начать работу с проектом без дополнительных устных инструкций; +- в репозитории есть `src/ApplicationInstallations/Docs/application-installations.md` с описанием `US1` и `US2`, двумя sequence diagram и ссылками на внешний контракт; +- `Install` и `OnAppInstall` поддерживают одношаговую и двухшаговую установку как отдельные сценарии; +- unit-тесты фиксируют инварианты состояний агрегатов для обоих сценариев, включая промежуточное состояние `US2` после первого шага. diff --git a/.tasks/90/install-handler-status-fix-plan.md b/.tasks/90/install-handler-status-fix-plan.md new file mode 100644 index 00000000..0a0f949c --- /dev/null +++ b/.tasks/90/install-handler-status-fix-plan.md @@ -0,0 +1,109 @@ +## План исправления преждевременного перехода install-flow в `active` + +### Summary +Issue `#90` описывает баг в `ApplicationInstallations\UseCase\Install\Handler`: сейчас handler всегда вызывает `applicationInstalled($command->applicationToken)` и для `Bitrix24Account`, и для `ApplicationInstallation`, даже если `applicationToken === null`. + +Из-за этого UI install-flow отрабатывает неверно: +- старт установки без `application_token` сразу переводит аккаунт и установку в `active`; +- доменные события завершения установки диспатчатся слишком рано; +- backend считает install-flow завершённым до прихода отдельного webhook / finish-step с `application_token`. + +Проверка текущего кода подтвердила проблему: +- `src/ApplicationInstallations/UseCase/Install/Handler.php` безусловно вызывает `applicationInstalled(...)` для обеих сущностей; +- `Bitrix24Account::applicationInstalled(null)` и `ApplicationInstallation::applicationInstalled(null)` всё равно переводят сущности в `active` и создают finish-events; +- текущие functional tests в `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php` закрепляют именно это преждевременное поведение. + +### Key Design Constraint +Нельзя ограничиться только условием `if (null !== $command->applicationToken)` в `Install\Handler`. + +Причина: +- `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` сейчас ищет master account только в статусе `active`; +- после исправления issue `#90` аккаунт в UI flow должен оставаться в статусе `new`; +- если не скорректировать finish-flow, `OnAppInstall\Handler` перестанет находить аккаунт и установка сломается уже на следующем шаге. + +Дополнительный контекст: +- `src/Bitrix24Accounts/UseCase/InstallFinish/Handler.php` уже работает со статусом `new` и переводит аккаунт в `active`; +- `OnAppInstall\Handler` сейчас только меняет `applicationStatus` и записывает `applicationToken`, но не завершает установку доменно. + +### Important Changes / Interfaces +1. Исправить стартовый install-flow в `src/ApplicationInstallations/UseCase/Install/Handler.php` +- если `applicationToken !== null`, сохранить текущее поведение immediate install-finish; +- если `applicationToken === null`, создать `Bitrix24Account` и `ApplicationInstallation` в статусе `new`; +- не вызывать `applicationInstalled()`; +- не диспатчить `ApplicationInstallationFinishedEvent` и `Bitrix24AccountApplicationInstalledEvent`. + +2. Привести finish-flow к консистентному сценарию +- определить, какой use case считается canonical finish-step для UI flow: + - либо расширить `ApplicationInstallations\UseCase\OnAppInstall\Handler`, чтобы он завершал установку и для account, и для installation; + - либо оставить `OnAppInstall` только для записи токена/статуса установки, но тогда явно связать его с `Bitrix24Accounts\UseCase\InstallFinish\Handler`. +- в любом варианте finish-step должен корректно работать с сущностями в статусе `new`. + +3. Пересмотреть выборку аккаунта в `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` +- убрать жёсткую зависимость от `Bitrix24AccountStatus::active`, если этот handler должен участвовать в finish-flow после старта без токена; +- либо искать master account в `new`, либо разрешить оба статуса (`new` / `active`) в зависимости от финального сценария. + +4. Актуализировать functional tests +- переписать ожидания в `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php` для сценария без токена; +- добавить или адаптировать тесты под двушаговый flow: + - старт установки без токена сохраняет `new`; + - finish-step / webhook с токеном переводит сущности в `active`; + - события завершения диспатчатся только на finish-step. + +### Implementation Plan +1. Зафиксировать целевую модель install-flow в тестах: +- `Install\Handler` с токеном => `active`; +- `Install\Handler` без токена => `new`; +- finish-step после получения `application_token` => `active`. + +2. Изменить `src/ApplicationInstallations/UseCase/Install/Handler.php`: +- обернуть вызовы `applicationInstalled()` условием на `null !== $command->applicationToken`; +- убедиться, что сохранение новых сущностей без токена не порождает finish-events. + +3. Привести следующий шаг install-flow к работе с новыми сущностями: +- обновить `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` и/или `src/Bitrix24Accounts/UseCase/InstallFinish/Handler.php`; +- проверить, где именно должен происходить переход `new -> active` для `ApplicationInstallation`; +- исключить сценарий, при котором токен только записывается, а статус установки навсегда остаётся `new`. + +4. Обновить functional coverage: +- `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php`; +- `tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php`; +- при необходимости `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php`. + +5. Прогнать регрессию по install-flow и смежным сценариям. + +### Test Cases and Scenarios +1. Прямой install с токеном: +- `Install\Handler` создаёт account и installation; +- обе сущности получают статус `active`; +- токен сохраняется; +- диспатчатся finish-events. + +2. UI install-start без токена: +- `Install\Handler` создаёт account и installation; +- обе сущности остаются в статусе `new`; +- токен не сохраняется; +- finish-events не диспатчатся. + +3. Finish-step после получения `application_token`: +- ранее созданные `new` account и installation находятся по `memberId`; +- токен сохраняется; +- обе сущности переходят в `active`; +- диспатчатся `Bitrix24AccountApplicationInstalledEvent` и `ApplicationInstallationFinishedEvent`. + +4. Регрессия на reinstall / repeated install: +- сценарии с существующей активной установкой по тому же `memberId` продолжают корректно деактивировать старые сущности и создавать новый install-flow; +- поведение не должно зависеть от того, был ли токен на первом шаге. + +### Files to Touch +- `src/ApplicationInstallations/UseCase/Install/Handler.php` +- `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` +- возможно `src/Bitrix24Accounts/UseCase/InstallFinish/Handler.php` +- `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php` +- `tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php` +- возможно `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` + +### Definition of Done +Исправление считается завершённым, когда: +- старт установки без `application_token` оставляет `Bitrix24Account` и `ApplicationInstallation` в статусе `new`; +- токен и переход в `active` происходят только на отдельном finish-step; +- functional tests отражают двушаговый install-flow и проходят без закрепления преждевременного `active`. From dde3b53e20b377c741767c69e925e0bf488d8fe0 Mon Sep 17 00:00:00 2001 From: mesilov Date: Sun, 15 Mar 2026 01:49:19 +0600 Subject: [PATCH 4/8] Refactor `Install` and `OnAppInstall` use cases: fix premature status transitions, implement distinct flows for single-step and two-step installations, update developer documentation with new user stories, clear finish-flow steps, sequence diagrams, and corner case handling. Signed-off-by: mesilov --- .tasks/90/developer-docs-update-plan.md | 155 ++++++- .tasks/90/install-handler-status-fix-plan.md | 401 ++++++++++++++----- 2 files changed, 452 insertions(+), 104 deletions(-) diff --git a/.tasks/90/developer-docs-update-plan.md b/.tasks/90/developer-docs-update-plan.md index d076d076..bb71e4d3 100644 --- a/.tasks/90/developer-docs-update-plan.md +++ b/.tasks/90/developer-docs-update-plan.md @@ -52,7 +52,17 @@ 6. Уточнить контракт `ApplicationInstallations\UseCase\OnAppInstall` - Сделать `OnAppInstall` use case ответственным за завершение установки в двухшаговом сценарии. -- После получения `application_token` use case должен переводить агрегаты из `new` в `active` и сохранять токен. +- После получения `application_token` use case должен завершать установку по жёстко заданному flow: + - найти pending `ApplicationInstallation` по `memberId` в статусе `new`; + - найти master `Bitrix24Account` по `memberId` в статусе `new`; + - обновить `applicationStatus` у `ApplicationInstallation` значением из события; + - вызвать `$bitrix24Account->applicationInstalled($applicationToken)`; + - вызвать `$applicationInstallation->applicationInstalled($applicationToken)`; + - сохранить оба агрегата и выполнить `flush`. +- В `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` выборка аккаунта должна быть пересмотрена и жёстко зафиксирована: + - handler ищет master account только среди записей со статусом `Bitrix24AccountStatus::new`; + - выборка по `active` для этого use case недопустима; + - fallback на другие статусы не допускается. - Для двухшагового сценария `OnAppInstall` должен искать master account в состоянии `new`, а не `active`, потому что до прихода события установка ещё не завершена. - Завершение двухшаговой установки обязательно должно происходить через вызов use case `OnAppInstall`; обход этого use case прямыми вызовами методов агрегатов вне сценария запрещён. - Если событие `ONAPPINSTALL` приходит повторно для уже завершённой установки, use case ничего не делает, пишет `warning` в лог и завершает обработку как `no-op`. @@ -74,6 +84,26 @@ - Написать unit-тесты для `Install` и `OnAppInstall`, которые работают через in-memory repositories. - Для `US2` проверять состояние после каждого шага: после `Install` и после `OnAppInstall`. +10. Актуализировать unit tests +- Привести unit-тесты в состояние, где они явно покрывают обе user story: + - `US1` для одношаговой установки; + - `US2` для двухшаговой установки. +- Отдельно покрыть corner cases для этих user story. +- Зафиксировать, что unit-тесты являются основным способом проверки нового контракта `Install` и `OnAppInstall`. + +11. Обновить документацию в `ApplicationInstallations/Docs` +- Добавить user story в папку `Docs` внутри `ApplicationInstallations`. +- Добавить sequence diagrams в папку `Docs` внутри `ApplicationInstallations`. +- Зафиксировать в локальной документации актуальный контракт `Install` и `OnAppInstall` для `US1`, `US2` и corner cases. + +12. Подготовить sequence diagrams для review +- Вынести обе sequence diagrams в план и в локальную документацию как отдельный артефакт для согласования. +- До начала реализации показать диаграммы на review, чтобы проверить: + - порядок вызовов в `US1`; + - порядок вызовов в `US2`; + - точки ветвления для re-install, duplicate `ONAPPINSTALL` и missing-event cases. +- После согласования использовать эти диаграммы как source of truth для реализации и unit-тестов. + ### Implementation Plan 1. Обновить `README.md` как главный документ быстрого старта: - добавить обязательные prerequisites для developer tooling; @@ -108,9 +138,15 @@ 6. Изменить доменное поведение `ApplicationInstallations\UseCase\OnAppInstall`: - use case должен находить агрегаты, созданные на первом шаге; - master account должен искаться среди записей в статусе `new`, чтобы pending-установка могла быть завершена корректно; -- сохранять `applicationToken` в `Bitrix24Account` и `ApplicationInstallation`; -- переводить оба агрегата в финальное состояние установки; -- обновлять `applicationStatus` у `ApplicationInstallation`. +- это правило должно быть реализовано прямо в `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` без альтернативных трактовок; +- finish-flow должен быть детерминированным и одинаковым во всех реализациях: + - загрузить pending `ApplicationInstallation` в статусе `new`; + - загрузить master `Bitrix24Account` в статусе `new`; + - вызвать `changeApplicationStatus(...)` для `ApplicationInstallation`; + - вызвать `applicationInstalled($applicationToken)` для `Bitrix24Account`; + - вызвать `applicationInstalled($applicationToken)` для `ApplicationInstallation`; + - сохранить оба агрегата; + - вызвать `flush`. - Этот use case является обязательной точкой входа для завершения `US2`; никакой альтернативный путь финализации установки не допускается. - при повторном событии для уже завершённой установки use case должен отработать как `no-op` и записать `warning` в лог; - отсутствие события для зависшей pending-инсталляции не закрывается в этом change set, а выносится в отдельное GitHub-обсуждение. @@ -139,6 +175,22 @@ - отдельный тест на `US1` с `applicationToken`; - отдельный сценарный тест на `US2`, где последовательно вызываются `Install`, затем `OnAppInstall`, и проверяется состояние после каждого шага. +10. Актуализировать существующие unit-тесты: +- обновить существующие unit-тесты `Install`, `OnAppInstall`, `Command` и связанные test helpers под новый контракт; +- убедиться, что тестовый набор покрывает обе user story и corner cases по ним; +- убрать устаревшие ожидания, завязанные на старое поведение немедленной финализации без `applicationToken`. + +11. Обновить документацию в `src/ApplicationInstallations/Docs`: +- добавить описание `US1` и `US2`; +- добавить две sequence diagram; +- описать corner cases, которые влияют на поведение `Install` и `OnAppInstall`; +- добавить ссылки на внешний контракт `b24phpsdk` и на документацию `ONAPPINSTALL`. + +12. Подготовить sequence diagrams к отдельному просмотру: +- оформить диаграммы так, чтобы их можно было показать отдельно от остального текста; +- использовать одинаковые названия участников и шагов в плане, документации и тестах; +- при необходимости добавить третью вспомогательную диаграмму для corner case `reinstall while previous installation is still new`. + ### ApplicationInstallations User Stories #### US1. Одношаговая установка Сценарий: @@ -183,6 +235,7 @@ sequenceDiagram - после первого шага токен отсутствует; - `OnAppInstall` ищет master account именно в статусе `new`; - завершение установки выполняется обязательным вызовом `OnAppInstall`; +- `OnAppInstall` завершает установку по фиксированному flow: `changeApplicationStatus` -> `Bitrix24Account::applicationInstalled($token)` -> `ApplicationInstallation::applicationInstalled($token)` -> `save` -> `flush`; - если во второй вкладке запускают новую установку до прихода `ONAPPINSTALL` по первой, второй вызов `Install` переводит старые pending-записи в `deleted` и создаёт новые записи для новой попытки установки; - если `ONAPPINSTALL` не пришёл, инсталляция остаётся pending; дальнейшая стратегия выносится в отдельный GitHub issue про фоновый сборщик битых инсталляций; - если `ONAPPINSTALL` пришёл повторно после успешного завершения установки, `OnAppInstall` ничего не делает и пишет `warning` в лог; @@ -211,14 +264,102 @@ sequenceDiagram OnAppInstall->>InstallRepo: save active installation ``` +### Sequence Diagrams For Review +#### Diagram 1. US1 One-Step Install +```mermaid +sequenceDiagram + autonumber + actor Installer as Installer / External Caller + participant Install as UseCase Install + participant Account as Bitrix24Account + participant Installation as ApplicationInstallation + participant AccountRepo as Bitrix24AccountRepository + participant InstallRepo as ApplicationInstallationRepository + participant Flusher as Flusher + + Installer->>Install: Install(command with applicationToken) + Install->>Account: create account(status=new) + Install->>Installation: create installation(status=new) + Install->>Account: applicationInstalled(applicationToken) + Install->>Installation: applicationInstalled(applicationToken) + Install->>AccountRepo: save(account status=active) + Install->>InstallRepo: save(installation status=active) + Install->>Flusher: flush(account, installation) + Install-->>Installer: done +``` + +#### Diagram 2. US2 Two-Step Install Via ONAPPINSTALL +```mermaid +sequenceDiagram + autonumber + actor UI as Bitrix24 UI / install.php + participant Install as UseCase Install + participant Account as Bitrix24Account + participant Installation as ApplicationInstallation + participant AccountRepo as Bitrix24AccountRepository + participant InstallRepo as ApplicationInstallationRepository + participant Event as ONAPPINSTALL + participant OnAppInstall as UseCase OnAppInstall + participant Flusher as Flusher + + UI->>Install: Install(command without applicationToken) + Install->>Account: create account(status=new) + Install->>Installation: create installation(status=new) + Install->>AccountRepo: save(account status=new) + Install->>InstallRepo: save(installation status=new) + Install->>Flusher: flush(account, installation) + Install-->>UI: pending installation created + + Event->>OnAppInstall: OnAppInstall(memberId, applicationToken, applicationStatus) + OnAppInstall->>InstallRepo: load pending installation(status=new) + OnAppInstall->>AccountRepo: load master account(status=new) + OnAppInstall->>Installation: changeApplicationStatus(applicationStatus) + OnAppInstall->>Account: applicationInstalled(applicationToken) + OnAppInstall->>Installation: applicationInstalled(applicationToken) + OnAppInstall->>AccountRepo: save(account status=active) + OnAppInstall->>InstallRepo: save(installation status=active) + OnAppInstall->>Flusher: flush(account, installation) + OnAppInstall-->>Event: done +``` + +#### Diagram 3. Reinstall While Previous Installation Is Still New +```mermaid +sequenceDiagram + autonumber + actor User as User / Second Tab + participant Install as UseCase Install + participant ExistingAccount as Existing Bitrix24Account(status=new) + participant ExistingInstallation as Existing ApplicationInstallation(status=new) + participant NewAccount as New Bitrix24Account + participant NewInstallation as New ApplicationInstallation + participant AccountRepo as Bitrix24AccountRepository + participant InstallRepo as ApplicationInstallationRepository + participant Flusher as Flusher + + User->>Install: Install(command without applicationToken, same memberId) + Install->>InstallRepo: load existing installation by memberId + Install->>AccountRepo: load existing accounts by memberId + Install->>ExistingAccount: applicationUninstalled(null) + Install->>ExistingInstallation: move to deleted according to new contract + Install->>AccountRepo: save(existing account status=deleted) + Install->>InstallRepo: save(existing installation status=deleted) + Install->>NewAccount: create account(status=new) + Install->>NewInstallation: create installation(status=new) + Install->>AccountRepo: save(new account status=new) + Install->>InstallRepo: save(new installation status=new) + Install->>Flusher: flush(deleted old entities, new entities) + Install-->>User: new pending installation created +``` + ### Documentation Deliverable - Новый локальный документ: `src/ApplicationInstallations/Docs/application-installations.md`. - Содержимое документа: - назначение `Install` и `OnAppInstall`; - описание `US1` и `US2`; - - обе sequence diagram; + - sequence diagrams из раздела `Sequence Diagrams For Review`; - явное различие между одношаговой и двухшаговой установкой; - явное правило, что для `US2` завершение установки всегда выполняется через use case `OnAppInstall`; + - точный finish-flow для `US2` с перечислением вызовов `changeApplicationStatus`, `Bitrix24Account::applicationInstalled($token)` и `ApplicationInstallation::applicationInstalled($token)`; - правило реинсталляции: если pending-установка в статусе `new` уже существует и пользователь запускает новую установку, старые записи переводятся в `deleted`, после чего создаются новые; - правило обработки повторного `ONAPPINSTALL`: `warning + no-op`; - ссылка на отдельный GitHub issue по проектированию фонового сборщика битых инсталляций; @@ -244,7 +385,9 @@ sequenceDiagram - `US2: OnAppInstall finalizes pending installation` - после предыдущего шага вызвать `OnAppInstall`; - проверить, что use case находит master account в статусе `new`, а не ожидает `active`; +- проверить, что выборка в `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` выполняется именно по `Bitrix24AccountStatus::new`; - проверить, что финализация сценария происходит именно через вызов `OnAppInstall`; +- проверить точный finish-flow: `ApplicationInstallation::changeApplicationStatus(...)`, затем `Bitrix24Account::applicationInstalled($token)`, затем `ApplicationInstallation::applicationInstalled($token)`; - проверить, что `Bitrix24Account` перешёл в `active`; - проверить, что `ApplicationInstallation` перешёл в `active`; - проверить, что токен сохранён; @@ -281,7 +424,7 @@ sequenceDiagram - `ONAPPINSTALL when account is not in status new` - подготовить данные, в которых installation найдена, но master account не находится в `new`; - проверить, что use case не делает частичного обновления состояния; -- проверить согласованное поведение: controlled exception или `warning + no-op`, в зависимости от финального контракта. +- проверить жёстко зафиксированное поведение: handler не завершает установку и завершает обработку с контролируемой ошибкой, потому что выборка аккаунта разрешена только по статусу `new`. - `ONAPPINSTALL with token mismatch / repeated different token` - после завершения установки вызвать `OnAppInstall` с другим `applicationToken`; - проверить, что состояние агрегатов не переписывается бесконтрольно; diff --git a/.tasks/90/install-handler-status-fix-plan.md b/.tasks/90/install-handler-status-fix-plan.md index 0a0f949c..6ca05d5a 100644 --- a/.tasks/90/install-handler-status-fix-plan.md +++ b/.tasks/90/install-handler-status-fix-plan.md @@ -1,109 +1,314 @@ -## План исправления преждевременного перехода install-flow в `active` +## План исправления install-flow и статусов в `ApplicationInstallations` ### Summary -Issue `#90` описывает баг в `ApplicationInstallations\UseCase\Install\Handler`: сейчас handler всегда вызывает `applicationInstalled($command->applicationToken)` и для `Bitrix24Account`, и для `ApplicationInstallation`, даже если `applicationToken === null`. - -Из-за этого UI install-flow отрабатывает неверно: -- старт установки без `application_token` сразу переводит аккаунт и установку в `active`; -- доменные события завершения установки диспатчатся слишком рано; -- backend считает install-flow завершённым до прихода отдельного webhook / finish-step с `application_token`. - -Проверка текущего кода подтвердила проблему: -- `src/ApplicationInstallations/UseCase/Install/Handler.php` безусловно вызывает `applicationInstalled(...)` для обеих сущностей; -- `Bitrix24Account::applicationInstalled(null)` и `ApplicationInstallation::applicationInstalled(null)` всё равно переводят сущности в `active` и создают finish-events; -- текущие functional tests в `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php` закрепляют именно это преждевременное поведение. - -### Key Design Constraint -Нельзя ограничиться только условием `if (null !== $command->applicationToken)` в `Install\Handler`. - -Причина: -- `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` сейчас ищет master account только в статусе `active`; -- после исправления issue `#90` аккаунт в UI flow должен оставаться в статусе `new`; -- если не скорректировать finish-flow, `OnAppInstall\Handler` перестанет находить аккаунт и установка сломается уже на следующем шаге. - -Дополнительный контекст: -- `src/Bitrix24Accounts/UseCase/InstallFinish/Handler.php` уже работает со статусом `new` и переводит аккаунт в `active`; -- `OnAppInstall\Handler` сейчас только меняет `applicationStatus` и записывает `applicationToken`, но не завершает установку доменно. - -### Important Changes / Interfaces -1. Исправить стартовый install-flow в `src/ApplicationInstallations/UseCase/Install/Handler.php` -- если `applicationToken !== null`, сохранить текущее поведение immediate install-finish; -- если `applicationToken === null`, создать `Bitrix24Account` и `ApplicationInstallation` в статусе `new`; -- не вызывать `applicationInstalled()`; -- не диспатчить `ApplicationInstallationFinishedEvent` и `Bitrix24AccountApplicationInstalledEvent`. - -2. Привести finish-flow к консистентному сценарию -- определить, какой use case считается canonical finish-step для UI flow: - - либо расширить `ApplicationInstallations\UseCase\OnAppInstall\Handler`, чтобы он завершал установку и для account, и для installation; - - либо оставить `OnAppInstall` только для записи токена/статуса установки, но тогда явно связать его с `Bitrix24Accounts\UseCase\InstallFinish\Handler`. -- в любом варианте finish-step должен корректно работать с сущностями в статусе `new`. - -3. Пересмотреть выборку аккаунта в `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` -- убрать жёсткую зависимость от `Bitrix24AccountStatus::active`, если этот handler должен участвовать в finish-flow после старта без токена; -- либо искать master account в `new`, либо разрешить оба статуса (`new` / `active`) в зависимости от финального сценария. - -4. Актуализировать functional tests -- переписать ожидания в `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php` для сценария без токена; -- добавить или адаптировать тесты под двушаговый flow: - - старт установки без токена сохраняет `new`; - - finish-step / webhook с токеном переводит сущности в `active`; - - события завершения диспатчатся только на finish-step. +Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/Install/Handler.php`: сейчас handler всегда завершает установку сразу, даже если `applicationToken === null`. + +Из-за этого ломается двухшаговый install-flow: +- первый шаг UI-установки без `application_token` преждевременно переводит `Bitrix24Account` и `ApplicationInstallation` в `active`; +- finish-события диспатчатся слишком рано; +- `ONAPPINSTALL` перестаёт быть реальным finish-step. + +Целевой контракт после исправления: +- `US1`: если `applicationToken` пришёл сразу, `Install` завершает установку в один шаг; +- `US2`: если `applicationToken` не пришёл, `Install` создаёт pending-сущности в статусе `new`, а finish-step выполняет `OnAppInstall`. + +### Scope +В рамках этого change set нужно: +- исправить `Install\Handler`; +- исправить `OnAppInstall\Handler`; +- актуализировать unit tests; +- обновить документацию в `src/ApplicationInstallations/Docs`; +- добавить sequence diagrams; +- зафиксировать отдельный follow-up issue для битых pending-инсталляций. + +### Target Contract +1. `Install` with token +- Если `Install\Command::$applicationToken !== null`, handler: + - создаёт `Bitrix24Account` в `new`; + - создаёт `ApplicationInstallation` в `new`; + - вызывает `Bitrix24Account::applicationInstalled($applicationToken)`; + - вызывает `ApplicationInstallation::applicationInstalled($applicationToken)`; + - сохраняет обе сущности; + - вызывает `flush`. +- Результат: + - обе сущности в `active`; + - токен сохранён; + - finish-events диспатчатся на этом шаге. + +2. `Install` without token +- Если `Install\Command::$applicationToken === null`, handler: + - создаёт `Bitrix24Account` в `new`; + - создаёт `ApplicationInstallation` в `new`; + - сохраняет обе сущности; + - вызывает `flush`; + - не вызывает `applicationInstalled(...)`. +- Результат: + - обе сущности остаются в `new`; + - токен не сохранён; + - finish-events не диспатчатся. + +3. `OnAppInstall` as canonical finish-step +- Для двухшаговой установки finish-step выполняется только через `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php`. +- Никакой альтернативный finish-flow через другие use case или прямые вызовы методов агрегатов вне `OnAppInstall` не допускается. + +4. Exact `OnAppInstall` finish-flow +- `OnAppInstall\Handler` работает по жёсткому алгоритму: + - найти pending `ApplicationInstallation` по `memberId` в статусе `new`; + - найти master `Bitrix24Account` по `memberId` только в статусе `Bitrix24AccountStatus::new`; + - вызвать `ApplicationInstallation::changeApplicationStatus($applicationStatus)`; + - вызвать `Bitrix24Account::applicationInstalled($applicationToken)`; + - вызвать `ApplicationInstallation::applicationInstalled($applicationToken)`; + - сохранить обе сущности; + - вызвать `flush`. + +5. Exact account lookup rule in `OnAppInstall` +- В `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` выборка аккаунта должна идти только по `Bitrix24AccountStatus::new`. +- Выборка по `active` для основного finish-path запрещена. +- Fallback на другие статусы для finish-path запрещён. + +### Explicit Behaviour for Corner Cases +1. Duplicate `ONAPPINSTALL` +- Если pending installation в статусе `new` не найдена, но по `memberId` уже существует завершённая `active` installation и `active` master account с тем же `applicationToken`, handler: + - ничего не меняет; + - пишет `warning` в лог; + - завершает обработку как `no-op`. + +2. Missing pending installation +- Если pending installation в статусе `new` не найдена и сценарий duplicate-event не подтверждается, `OnAppInstall` завершает обработку с контролируемым исключением. +- Для этого сценария source of truth: + - `ApplicationInstallationNotFoundException`, если installation не найдена; + - `Bitrix24AccountNotFoundException`, если installation найдена, а master account в статусе `new` не найден. + +3. Token mismatch on repeated event +- Если `ONAPPINSTALL` пришёл для уже завершённой установки, но `applicationToken` отличается от уже сохранённого токена: + - handler ничего не меняет; + - пишет `warning` в лог; + - завершает обработку с контролируемым исключением. + +4. Reinstall while previous installation is still `new` +- Если по тому же `memberId` уже есть pending installation в статусе `new`, второй вызов `Install` должен: + - перевести старый `Bitrix24Account` в `deleted`; + - перевести старую `ApplicationInstallation` в `deleted`; + - создать новую пару сущностей. +- Для `ApplicationInstallation` нужно добавить явный доменный переход для удаления pending-installation из статуса `new`. +- Использовать для этого `applicationUninstalled()` нельзя, потому что сейчас он разрешён только из `active|blocked`. + +5. Reinstall while previous installation is already `active` +- Поведение остаётся совместимым с текущим reinstall-flow: + - старые сущности переводятся в `deleted`; + - создаётся новая пара сущностей; + - статус новой пары зависит от наличия `applicationToken` в новом вызове. + +6. Missing `ONAPPINSTALL` event +- Pending installation может зависнуть в `new`. +- Это не лечится в рамках синхронного use case. +- В рамках задачи создаётся отдельный GitHub issue на проектирование фонового сборщика битых pending-инсталляций. ### Implementation Plan -1. Зафиксировать целевую модель install-flow в тестах: -- `Install\Handler` с токеном => `active`; -- `Install\Handler` без токена => `new`; -- finish-step после получения `application_token` => `active`. - -2. Изменить `src/ApplicationInstallations/UseCase/Install/Handler.php`: -- обернуть вызовы `applicationInstalled()` условием на `null !== $command->applicationToken`; -- убедиться, что сохранение новых сущностей без токена не порождает finish-events. - -3. Привести следующий шаг install-flow к работе с новыми сущностями: -- обновить `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` и/или `src/Bitrix24Accounts/UseCase/InstallFinish/Handler.php`; -- проверить, где именно должен происходить переход `new -> active` для `ApplicationInstallation`; -- исключить сценарий, при котором токен только записывается, а статус установки навсегда остаётся `new`. - -4. Обновить functional coverage: -- `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php`; -- `tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php`; -- при необходимости `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php`. - -5. Прогнать регрессию по install-flow и смежным сценариям. - -### Test Cases and Scenarios -1. Прямой install с токеном: -- `Install\Handler` создаёт account и installation; -- обе сущности получают статус `active`; -- токен сохраняется; -- диспатчатся finish-events. - -2. UI install-start без токена: -- `Install\Handler` создаёт account и installation; -- обе сущности остаются в статусе `new`; -- токен не сохраняется; -- finish-events не диспатчатся. - -3. Finish-step после получения `application_token`: -- ранее созданные `new` account и installation находятся по `memberId`; -- токен сохраняется; -- обе сущности переходят в `active`; -- диспатчатся `Bitrix24AccountApplicationInstalledEvent` и `ApplicationInstallationFinishedEvent`. - -4. Регрессия на reinstall / repeated install: -- сценарии с существующей активной установкой по тому же `memberId` продолжают корректно деактивировать старые сущности и создавать новый install-flow; -- поведение не должно зависеть от того, был ли токен на первом шаге. +1. Исправить `src/ApplicationInstallations/UseCase/Install/Handler.php` +- ветка с токеном: оставить one-step finish; +- ветка без токена: не вызывать `applicationInstalled(...)`; +- сохранять `Bitrix24Account` и `ApplicationInstallation` в `new`; +- не диспатчить finish-events на первом шаге без токена. + +2. Исправить `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` +- сделать `OnAppInstall` единственной точкой finish-step для `US2`; +- пересмотреть выборку аккаунта: + - finish-path ищет master account только в `Bitrix24AccountStatus::new`; + - duplicate-event path отдельно проверяет already finished `active` records; +- реализовать exact finish-flow в фиксированном порядке: + - `changeApplicationStatus(...)`; + - `Bitrix24Account::applicationInstalled($token)`; + - `ApplicationInstallation::applicationInstalled($token)`; + - `save`; + - `flush`. + +3. Добавить явный доменный переход для удаления pending installation +- В `ApplicationInstallation` нужно добавить explicit method для перевода сущности из `new` в `deleted` при re-install. +- Название и API метода должны прямо отражать назначение: + - удаление незавершённой installation; + - без требования `applicationToken`. +- Этот переход должен использоваться в `Install\Handler` при re-install поверх `new`. + +4. Актуализировать unit tests +- Обновить существующие unit tests `Install`, `OnAppInstall`, `Command` и test helpers. +- Unit tests должны покрывать: + - `US1`; + - `US2`; + - corner cases для `Install`; + - corner cases для `OnAppInstall`. +- Unit tests являются основным способом фиксации нового контракта. + +5. Обновить документацию +- В `src/ApplicationInstallations/Docs` добавить: + - описание `US1`; + - описание `US2`; + - corner cases; + - sequence diagrams; + - ссылки на внешний контракт: + - `https://github.com/bitrix24/b24phpsdk/blob/v3/src/Application/Contracts/ApplicationInstallations/Docs/ApplicationInstallations.md` + - `https://apidocs.bitrix24.com/api-reference/common/events/on-app-install.html` + +6. Создать follow-up GitHub issue +- Отдельно создать issue на проектирование фонового сборщика битых pending-инсталляций. +- В issue предложить варианты: + - worker с TTL и переводом зависших `new`-installations в broken/failed сценарий; + - worker с TTL и alert-only поведением; + - reconciliation job; + - ручной operational flow. + +### Sequence Diagrams For Review +#### Diagram 1. US1 One-Step Install +```mermaid +sequenceDiagram + autonumber + actor Installer as Installer / External Caller + participant Install as UseCase Install + participant Account as Bitrix24Account + participant Installation as ApplicationInstallation + participant AccountRepo as Bitrix24AccountRepository + participant InstallRepo as ApplicationInstallationRepository + participant Flusher as Flusher + + Installer->>Install: Install(command with applicationToken) + Install->>Account: create account(status=new) + Install->>Installation: create installation(status=new) + Install->>Account: applicationInstalled(applicationToken) + Install->>Installation: applicationInstalled(applicationToken) + Install->>AccountRepo: save(account status=active) + Install->>InstallRepo: save(installation status=active) + Install->>Flusher: flush(account, installation) + Install-->>Installer: done +``` + +#### Diagram 2. US2 Two-Step Install Via ONAPPINSTALL +```mermaid +sequenceDiagram + autonumber + actor UI as Bitrix24 UI / install.php + participant Install as UseCase Install + participant Account as Bitrix24Account + participant Installation as ApplicationInstallation + participant AccountRepo as Bitrix24AccountRepository + participant InstallRepo as ApplicationInstallationRepository + participant Event as ONAPPINSTALL + participant OnAppInstall as UseCase OnAppInstall + participant Flusher as Flusher + + UI->>Install: Install(command without applicationToken) + Install->>Account: create account(status=new) + Install->>Installation: create installation(status=new) + Install->>AccountRepo: save(account status=new) + Install->>InstallRepo: save(installation status=new) + Install->>Flusher: flush(account, installation) + Install-->>UI: pending installation created + + Event->>OnAppInstall: OnAppInstall(memberId, applicationToken, applicationStatus) + OnAppInstall->>InstallRepo: load pending installation(status=new) + OnAppInstall->>AccountRepo: load master account(status=new) + OnAppInstall->>Installation: changeApplicationStatus(applicationStatus) + OnAppInstall->>Account: applicationInstalled(applicationToken) + OnAppInstall->>Installation: applicationInstalled(applicationToken) + OnAppInstall->>AccountRepo: save(account status=active) + OnAppInstall->>InstallRepo: save(installation status=active) + OnAppInstall->>Flusher: flush(account, installation) + OnAppInstall-->>Event: done +``` + +#### Diagram 3. Reinstall While Previous Installation Is Still New +```mermaid +sequenceDiagram + autonumber + actor User as User / Second Tab + participant Install as UseCase Install + participant ExistingAccount as Existing Bitrix24Account(status=new) + participant ExistingInstallation as Existing ApplicationInstallation(status=new) + participant NewAccount as New Bitrix24Account + participant NewInstallation as New ApplicationInstallation + participant AccountRepo as Bitrix24AccountRepository + participant InstallRepo as ApplicationInstallationRepository + participant Flusher as Flusher + + User->>Install: Install(command without applicationToken, same memberId) + Install->>InstallRepo: load existing installation by memberId + Install->>AccountRepo: load existing accounts by memberId + Install->>ExistingAccount: applicationUninstalled(null) + Install->>ExistingInstallation: delete pending installation via new explicit method + Install->>AccountRepo: save(existing account status=deleted) + Install->>InstallRepo: save(existing installation status=deleted) + Install->>NewAccount: create account(status=new) + Install->>NewInstallation: create installation(status=new) + Install->>AccountRepo: save(new account status=new) + Install->>InstallRepo: save(new installation status=new) + Install->>Flusher: flush(old deleted entities, new pending entities) + Install-->>User: new pending installation created +``` + +### Unit Tests and Invariants +1. Base scenarios `Install` +- `US1: Install with applicationToken` + - после `handle()` account и installation находятся в `active`; + - токен сохранён; + - finish-events появились. +- `US2: Install without token` + - после `handle()` account и installation находятся в `new`; + - токен не сохранён; + - finish-events не появились. + +2. Base scenarios `OnAppInstall` +- `US2: OnAppInstall finalizes pending installation` + - handler ищет installation по `memberId` в `new`; + - handler ищет master account по `memberId` только в `new`; + - выполняется exact finish-flow; + - после `handle()` обе сущности в `active`; + - токен сохранён; + - `applicationStatus` обновлён. + +3. Corner cases `Install` +- `reinstall while previous installation is still new` + - старая пара переводится в `deleted`; + - новая пара создаётся в `new`. +- `reinstall while previous installation is already active` + - старая активная пара переводится в `deleted`; + - новая пара создаётся корректно. +- `invalid install payload` + - покрыть невалидные комбинации для `Install\Command`. + +4. Corner cases `OnAppInstall` +- `duplicate ONAPPINSTALL event with same token` + - `warning + no-op`. +- `ONAPPINSTALL for missing pending installation` + - controlled exception. +- `ONAPPINSTALL when account is not in status new` + - controlled exception; + - partial update state отсутствует. +- `ONAPPINSTALL with different token for already finished installation` + - `warning`; + - controlled exception; + - state does not change. + +5. Test infrastructure +- использовать in-memory repositories для `ApplicationInstallationRepositoryInterface` и `Bitrix24AccountRepositoryInterface`; +- добавить logger test double для проверки `warning`; +- при необходимости добавить flusher test double. ### Files to Touch - `src/ApplicationInstallations/UseCase/Install/Handler.php` - `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` -- возможно `src/Bitrix24Accounts/UseCase/InstallFinish/Handler.php` -- `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php` -- `tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php` -- возможно `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` +- `src/ApplicationInstallations/Entity/ApplicationInstallation.php` +- `tests/Unit/ApplicationInstallations/UseCase/Install/...` +- `tests/Unit/ApplicationInstallations/UseCase/OnAppInstall/...` +- `tests/Helpers/...` +- `src/ApplicationInstallations/Docs/application-installations.md` ### Definition of Done Исправление считается завершённым, когда: -- старт установки без `application_token` оставляет `Bitrix24Account` и `ApplicationInstallation` в статусе `new`; -- токен и переход в `active` происходят только на отдельном finish-step; -- functional tests отражают двушаговый install-flow и проходят без закрепления преждевременного `active`. +- `Install` с токеном завершает установку в один шаг; +- `Install` без токена создаёт pending-сущности в `new`; +- `OnAppInstall` является единственным finish-step для двухшаговой установки; +- выборка master account в finish-path идёт только по `Bitrix24AccountStatus::new`; +- duplicate `ONAPPINSTALL` с тем же токеном обрабатывается как `warning + no-op`; +- missing pending installation приводит к контролируемой ошибке; +- re-install поверх pending `new` переводит старые записи в `deleted` и создаёт новые; +- unit tests покрывают обе user story и corner cases; +- документация в `src/ApplicationInstallations/Docs` обновлена и содержит sequence diagrams. From 13fe39a3920758d2dcc30d0571a0a2ebdd5581c3 Mon Sep 17 00:00:00 2001 From: mesilov Date: Sun, 15 Mar 2026 02:02:07 +0600 Subject: [PATCH 5/8] Refactor `Install` and `OnAppInstall` use cases: fix `new`-status handling for pending installations, implement SDK-compatible `reinstall` flow, update functional and unit tests, and align developer documentation with new contracts and finish-flows. Signed-off-by: mesilov --- .tasks/90/install-handler-status-fix-plan.md | 64 +++++++++++++++----- 1 file changed, 50 insertions(+), 14 deletions(-) diff --git a/.tasks/90/install-handler-status-fix-plan.md b/.tasks/90/install-handler-status-fix-plan.md index 6ca05d5a..d51af438 100644 --- a/.tasks/90/install-handler-status-fix-plan.md +++ b/.tasks/90/install-handler-status-fix-plan.md @@ -17,6 +17,7 @@ Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/I - исправить `Install\Handler`; - исправить `OnAppInstall\Handler`; - актуализировать unit tests; +- актуализировать functional tests, которые закрепляют старое поведение; - обновить документацию в `src/ApplicationInstallations/Docs`; - добавить sequence diagrams; - зафиксировать отдельный follow-up issue для битых pending-инсталляций. @@ -83,15 +84,19 @@ Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/I - Если `ONAPPINSTALL` пришёл для уже завершённой установки, но `applicationToken` отличается от уже сохранённого токена: - handler ничего не меняет; - пишет `warning` в лог; - - завершает обработку с контролируемым исключением. + - события не эмитятся; + - стейт агрегатов не меняется; + - обработка завершается без исключения. 4. Reinstall while previous installation is still `new` - Если по тому же `memberId` уже есть pending installation в статусе `new`, второй вызов `Install` должен: - перевести старый `Bitrix24Account` в `deleted`; - перевести старую `ApplicationInstallation` в `deleted`; - создать новую пару сущностей. -- Для `ApplicationInstallation` нужно добавить явный доменный переход для удаления pending-installation из статуса `new`. -- Использовать для этого `applicationUninstalled()` нельзя, потому что сейчас он разрешён только из `active|blocked`. +- Для `ApplicationInstallation` используется SDK-совместимый путь без добавления нового метода в контракт: + - сначала `markAsBlocked('reinstall before finish')`; + - затем `applicationUninstalled(null)`. +- Этот путь является обязательным для `lib`, если правим только реализацию `lib` и не меняем интерфейс из SDK. 5. Reinstall while previous installation is already `active` - Поведение остаётся совместимым с текущим reinstall-flow: @@ -123,12 +128,12 @@ Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/I - `save`; - `flush`. -3. Добавить явный доменный переход для удаления pending installation -- В `ApplicationInstallation` нужно добавить explicit method для перевода сущности из `new` в `deleted` при re-install. -- Название и API метода должны прямо отражать назначение: - - удаление незавершённой installation; - - без требования `applicationToken`. -- Этот переход должен использоваться в `Install\Handler` при re-install поверх `new`. +3. Использовать SDK-совместимый delete-flow для pending installation +- В `Install\Handler` при re-install поверх `new` для `ApplicationInstallation` нужно использовать последовательность: + - `markAsBlocked('reinstall before finish')`; + - `applicationUninstalled(null)`. +- Новый `lib`-only метод в `ApplicationInstallation` не добавляется. +- Контракт SDK не меняется. 4. Актуализировать unit tests - Обновить существующие unit tests `Install`, `OnAppInstall`, `Command` и test helpers. @@ -139,7 +144,25 @@ Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/I - corner cases для `OnAppInstall`. - Unit tests являются основным способом фиксации нового контракта. -5. Обновить документацию +5. Актуализировать functional tests +- Исправить следующие functional tests: + - `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php` + - `tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php` + - `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` только если после правок он пересекается по контракту с новым finish-flow. +- В `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php`: + - сценарий `Install` без токена должен ожидать статус `new`, а не `active`; + - сценарий `Install` с токеном должен по-прежнему ожидать `active`; + - reinstall поверх `new` должен проверять перевод старых записей в `deleted` и создание новых; + - reinstall поверх `new` должен явно покрывать путь `markAsBlocked('reinstall before finish') -> applicationUninstalled(null)` для `ApplicationInstallation`. +- В `tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php`: + - `OnAppInstall` должен завершать pending installation из `new`; + - account должен искаться через новый finish-path по статусу `new`; + - нужно проверить duplicate-event path; + - нужно проверить missing pending installation path; + - нужно проверить repeated-event path с другим токеном: только `warning`, без исключения, без изменения состояния и без новых событий. +- Если `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` остаётся в проекте без изменений, это нужно явно проверить и зафиксировать, чтобы не было двух конкурирующих finish-flow. + +6. Обновить документацию - В `src/ApplicationInstallations/Docs` добавить: - описание `US1`; - описание `US2`; @@ -149,7 +172,7 @@ Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/I - `https://github.com/bitrix24/b24phpsdk/blob/v3/src/Application/Contracts/ApplicationInstallations/Docs/ApplicationInstallations.md` - `https://apidocs.bitrix24.com/api-reference/common/events/on-app-install.html` -6. Создать follow-up GitHub issue +7. Создать follow-up GitHub issue - Отдельно создать issue на проектирование фонового сборщика битых pending-инсталляций. - В issue предложить варианты: - worker с TTL и переводом зависших `new`-installations в broken/failed сценарий; @@ -233,7 +256,8 @@ sequenceDiagram Install->>InstallRepo: load existing installation by memberId Install->>AccountRepo: load existing accounts by memberId Install->>ExistingAccount: applicationUninstalled(null) - Install->>ExistingInstallation: delete pending installation via new explicit method + Install->>ExistingInstallation: markAsBlocked("reinstall before finish") + Install->>ExistingInstallation: applicationUninstalled(null) Install->>AccountRepo: save(existing account status=deleted) Install->>InstallRepo: save(existing installation status=deleted) Install->>NewAccount: create account(status=new) @@ -266,6 +290,7 @@ sequenceDiagram 3. Corner cases `Install` - `reinstall while previous installation is still new` + - старая `ApplicationInstallation` проходит путь `markAsBlocked('reinstall before finish') -> applicationUninstalled(null)`; - старая пара переводится в `deleted`; - новая пара создаётся в `new`. - `reinstall while previous installation is already active` @@ -284,18 +309,27 @@ sequenceDiagram - partial update state отсутствует. - `ONAPPINSTALL with different token for already finished installation` - `warning`; - - controlled exception; - - state does not change. + - события не эмитятся; + - state does not change; + - исключение не выбрасывается. 5. Test infrastructure - использовать in-memory repositories для `ApplicationInstallationRepositoryInterface` и `Bitrix24AccountRepositoryInterface`; - добавить logger test double для проверки `warning`; - при необходимости добавить flusher test double. +6. Functional test coverage to update +- `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php` +- `tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php` +- `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` при необходимости, если новый контракт делает его устаревшим или конфликтующим + ### Files to Touch - `src/ApplicationInstallations/UseCase/Install/Handler.php` - `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` - `src/ApplicationInstallations/Entity/ApplicationInstallation.php` +- `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php` +- `tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php` +- `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` при необходимости - `tests/Unit/ApplicationInstallations/UseCase/Install/...` - `tests/Unit/ApplicationInstallations/UseCase/OnAppInstall/...` - `tests/Helpers/...` @@ -309,6 +343,8 @@ sequenceDiagram - выборка master account в finish-path идёт только по `Bitrix24AccountStatus::new`; - duplicate `ONAPPINSTALL` с тем же токеном обрабатывается как `warning + no-op`; - missing pending installation приводит к контролируемой ошибке; +- repeated `ONAPPINSTALL` с другим токеном обрабатывается как `warning + no-op` без исключения, без новых событий и без изменения состояния; - re-install поверх pending `new` переводит старые записи в `deleted` и создаёт новые; +- для `ApplicationInstallation` при re-install поверх `new` используется путь `markAsBlocked('reinstall before finish') -> applicationUninstalled(null)`; - unit tests покрывают обе user story и corner cases; - документация в `src/ApplicationInstallations/Docs` обновлена и содержит sequence diagrams. From 1555d04661cd83e4e8af8c25de7fc7b345567d3e Mon Sep 17 00:00:00 2001 From: mesilov Date: Sun, 15 Mar 2026 02:12:22 +0600 Subject: [PATCH 6/8] Refactor `Install` and `OnAppInstall` use cases: enhance `new`-status handling for pending installations, establish strict finish-flow sequencing, implement controlled exception paths, update duplicate/mismatch handling, fix `reinstall` flows, revise tests, and update documentation with new canonical flows and sequence diagrams. Signed-off-by: mesilov --- .tasks/90/install-handler-status-fix-plan.md | 51 +++++++++++++++----- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/.tasks/90/install-handler-status-fix-plan.md b/.tasks/90/install-handler-status-fix-plan.md index d51af438..9b273738 100644 --- a/.tasks/90/install-handler-status-fix-plan.md +++ b/.tasks/90/install-handler-status-fix-plan.md @@ -54,13 +54,19 @@ Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/I 4. Exact `OnAppInstall` finish-flow - `OnAppInstall\Handler` работает по жёсткому алгоритму: - - найти pending `ApplicationInstallation` по `memberId` в статусе `new`; - - найти master `Bitrix24Account` по `memberId` только в статусе `Bitrix24AccountStatus::new`; - - вызвать `ApplicationInstallation::changeApplicationStatus($applicationStatus)`; - - вызвать `Bitrix24Account::applicationInstalled($applicationToken)`; - - вызвать `ApplicationInstallation::applicationInstalled($applicationToken)`; - - сохранить обе сущности; - - вызвать `flush`. + - сначала найти pending `ApplicationInstallation` по `memberId` в статусе `new`; + - если pending installation найдена: + - найти master `Bitrix24Account` по `memberId` только в статусе `Bitrix24AccountStatus::new`; + - вызвать `ApplicationInstallation::changeApplicationStatus($applicationStatus)`; + - вызвать `Bitrix24Account::applicationInstalled($applicationToken)`; + - вызвать `ApplicationInstallation::applicationInstalled($applicationToken)`; + - сохранить обе сущности; + - вызвать `flush`; + - если pending installation не найдена: + - проверить сценарий already finished installation по `memberId`; + - если найден finished pair с тем же `applicationToken`, обработать как `warning + no-op`; + - если найден finished pair с другим `applicationToken`, обработать как `warning + no-op`; + - если finished pair не найден, завершить обработку контролируемым исключением. 5. Exact account lookup rule in `OnAppInstall` - В `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php` выборка аккаунта должна идти только по `Bitrix24AccountStatus::new`. @@ -71,7 +77,10 @@ Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/I 1. Duplicate `ONAPPINSTALL` - Если pending installation в статусе `new` не найдена, но по `memberId` уже существует завершённая `active` installation и `active` master account с тем же `applicationToken`, handler: - ничего не меняет; + - `ApplicationInstallation::changeApplicationStatus(...)` не вызывается; - пишет `warning` в лог; + - события не эмитятся; + - стейт агрегатов не меняется; - завершает обработку как `no-op`. 2. Missing pending installation @@ -83,6 +92,7 @@ Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/I 3. Token mismatch on repeated event - Если `ONAPPINSTALL` пришёл для уже завершённой установки, но `applicationToken` отличается от уже сохранённого токена: - handler ничего не меняет; + - `ApplicationInstallation::changeApplicationStatus(...)` не вызывается; - пишет `warning` в лог; - события не эмитятся; - стейт агрегатов не меняется; @@ -97,6 +107,9 @@ Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/I - сначала `markAsBlocked('reinstall before finish')`; - затем `applicationUninstalled(null)`. - Этот путь является обязательным для `lib`, если правим только реализацию `lib` и не меняем интерфейс из SDK. +- Порядок сохранения и `flush` фиксируется жёстко: + - сначала перевести старые сущности в `deleted` и сделать отдельный `flush`; + - только после этого создавать и сохранять новую пару сущностей. 5. Reinstall while previous installation is already `active` - Поведение остаётся совместимым с текущим reinstall-flow: @@ -121,12 +134,17 @@ Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/I - пересмотреть выборку аккаунта: - finish-path ищет master account только в `Bitrix24AccountStatus::new`; - duplicate-event path отдельно проверяет already finished `active` records; +- порядок ветвления должен быть жёстко зафиксирован: + - сначала проверяется pending `new` installation; + - затем duplicate/mismatch path по already finished `active` records; + - только потом controlled exception path; - реализовать exact finish-flow в фиксированном порядке: - `changeApplicationStatus(...)`; - `Bitrix24Account::applicationInstalled($token)`; - `ApplicationInstallation::applicationInstalled($token)`; - `save`; - `flush`. +- в duplicate/mismatch path `changeApplicationStatus(...)` не вызывается. 3. Использовать SDK-совместимый delete-flow для pending installation - В `Install\Handler` при re-install поверх `new` для `ApplicationInstallation` нужно использовать последовательность: @@ -134,6 +152,7 @@ Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/I - `applicationUninstalled(null)`. - Новый `lib`-only метод в `ApplicationInstallation` не добавляется. - Контракт SDK не меняется. +- Старые сущности должны быть `flush`-нуты отдельно до создания новой пары. 4. Актуализировать unit tests - Обновить существующие unit tests `Install`, `OnAppInstall`, `Command` и test helpers. @@ -148,7 +167,7 @@ Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/I - Исправить следующие functional tests: - `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php` - `tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php` - - `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` только если после правок он пересекается по контракту с новым finish-flow. + - `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` - В `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php`: - сценарий `Install` без токена должен ожидать статус `new`, а не `active`; - сценарий `Install` с токеном должен по-прежнему ожидать `active`; @@ -161,6 +180,7 @@ Issue `#90` описывает баг в `src/ApplicationInstallations/UseCase/I - нужно проверить missing pending installation path; - нужно проверить repeated-event path с другим токеном: только `warning`, без исключения, без изменения состояния и без новых событий. - Если `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` остаётся в проекте без изменений, это нужно явно проверить и зафиксировать, чтобы не было двух конкурирующих finish-flow. +- `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` нужно обновить явно, чтобы он не конфликтовал с новым canonical finish-flow через `OnAppInstall`. 6. Обновить документацию - В `src/ApplicationInstallations/Docs` добавить: @@ -260,11 +280,12 @@ sequenceDiagram Install->>ExistingInstallation: applicationUninstalled(null) Install->>AccountRepo: save(existing account status=deleted) Install->>InstallRepo: save(existing installation status=deleted) + Install->>Flusher: flush(old deleted entities) Install->>NewAccount: create account(status=new) Install->>NewInstallation: create installation(status=new) Install->>AccountRepo: save(new account status=new) Install->>InstallRepo: save(new installation status=new) - Install->>Flusher: flush(old deleted entities, new pending entities) + Install->>Flusher: flush(new pending entities) Install-->>User: new pending installation created ``` @@ -283,6 +304,7 @@ sequenceDiagram - `US2: OnAppInstall finalizes pending installation` - handler ищет installation по `memberId` в `new`; - handler ищет master account по `memberId` только в `new`; + - duplicate/mismatch path не выполняется, если найден pending `new`; - выполняется exact finish-flow; - после `handle()` обе сущности в `active`; - токен сохранён; @@ -302,6 +324,9 @@ sequenceDiagram 4. Corner cases `OnAppInstall` - `duplicate ONAPPINSTALL event with same token` - `warning + no-op`. + - `changeApplicationStatus(...)` не вызывается. + - события не эмитятся. + - state does not change. - `ONAPPINSTALL for missing pending installation` - controlled exception. - `ONAPPINSTALL when account is not in status new` @@ -309,6 +334,7 @@ sequenceDiagram - partial update state отсутствует. - `ONAPPINSTALL with different token for already finished installation` - `warning`; + - `changeApplicationStatus(...)` не вызывается; - события не эмитятся; - state does not change; - исключение не выбрасывается. @@ -321,7 +347,7 @@ sequenceDiagram 6. Functional test coverage to update - `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php` - `tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php` -- `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` при необходимости, если новый контракт делает его устаревшим или конфликтующим +- `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` ### Files to Touch - `src/ApplicationInstallations/UseCase/Install/Handler.php` @@ -329,7 +355,7 @@ sequenceDiagram - `src/ApplicationInstallations/Entity/ApplicationInstallation.php` - `tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php` - `tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php` -- `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` при необходимости +- `tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php` - `tests/Unit/ApplicationInstallations/UseCase/Install/...` - `tests/Unit/ApplicationInstallations/UseCase/OnAppInstall/...` - `tests/Helpers/...` @@ -342,9 +368,12 @@ sequenceDiagram - `OnAppInstall` является единственным finish-step для двухшаговой установки; - выборка master account в finish-path идёт только по `Bitrix24AccountStatus::new`; - duplicate `ONAPPINSTALL` с тем же токеном обрабатывается как `warning + no-op`; +- duplicate `ONAPPINSTALL` с тем же токеном не эмитит события и не меняет состояние; - missing pending installation приводит к контролируемой ошибке; - repeated `ONAPPINSTALL` с другим токеном обрабатывается как `warning + no-op` без исключения, без новых событий и без изменения состояния; - re-install поверх pending `new` переводит старые записи в `deleted` и создаёт новые; - для `ApplicationInstallation` при re-install поверх `new` используется путь `markAsBlocked('reinstall before finish') -> applicationUninstalled(null)`; +- при re-install старые сущности `flush`-ятся отдельно до создания новой пары; +- в duplicate/mismatch path `applicationStatus` не меняется; - unit tests покрывают обе user story и corner cases; - документация в `src/ApplicationInstallations/Docs` обновлена и содержит sequence diagrams. From 573985a98a47e08e0c5fd4f18acb188ead66de3f Mon Sep 17 00:00:00 2001 From: mesilov Date: Sun, 15 Mar 2026 02:37:08 +0600 Subject: [PATCH 7/8] Refactor `Install` and `OnAppInstall` use cases: consolidate updated installation handling, canonicalize finish-step flows, add `reinstall` and corner-case logic, improve tests, and extend documentation with detailed user stories and sequence diagrams. Signed-off-by: mesilov --- AGENTS.md | 13 + .../Docs/application-installations.md | 179 ++++++++ .../UseCase/Install/Handler.php | 76 ++-- .../UseCase/OnAppInstall/Handler.php | 86 +++- .../UseCase/Install/HandlerTest.php | 411 ++++++------------ .../UseCase/OnAppInstall/HandlerTest.php | 299 +++++++++---- .../UseCase/InstallFinish/HandlerTest.php | 4 +- .../UseCase/Install/HandlerTest.php | 242 +++++++++++ .../UseCase/OnAppInstall/HandlerTest.php | 278 ++++++++++++ 9 files changed, 1184 insertions(+), 404 deletions(-) create mode 100644 src/ApplicationInstallations/Docs/application-installations.md create mode 100644 tests/Unit/ApplicationInstallations/UseCase/Install/HandlerTest.php create mode 100644 tests/Unit/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php diff --git a/AGENTS.md b/AGENTS.md index 09f246dc..db5d40ff 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,3 +15,16 @@ Checks before work starts: - ensure `.mcp.json` is present and contains the expected server list - restart the client after pulling changes to `.mcp.json` - verify that the configured MCP servers are available in the current client + +## Tests And Linters + +Agents working in this repository must run linters and tests only through `Makefile` entrypoints. + +Do not call tool binaries directly when an equivalent `make` target exists. + +Use: + +- `make lint-all` for the full linter pass +- `make test-unit` for the unit test suite +- `make test-functional` for the functional test suite +- `make lint-cs-fixer-fix` and `make lint-rector-fix` only when an autofix pass is needed diff --git a/src/ApplicationInstallations/Docs/application-installations.md b/src/ApplicationInstallations/Docs/application-installations.md new file mode 100644 index 00000000..734bfa71 --- /dev/null +++ b/src/ApplicationInstallations/Docs/application-installations.md @@ -0,0 +1,179 @@ +# ApplicationInstallations Install Flow + +## Overview + +`ApplicationInstallations` stores the installation state for a Bitrix24 portal and coordinates it with the master `Bitrix24Account`. + +The install flow is intentionally split into two contracts: + +- `US1`: one-step install when the initial `Install` command already contains `applicationToken` +- `US2`: two-step install when `Install` starts the flow in `new`, and `OnAppInstall` performs the canonical finish-step + +External references: + +- SDK contract: https://github.com/bitrix24/b24phpsdk/blob/v3/src/Application/Contracts/ApplicationInstallations/Docs/ApplicationInstallations.md +- Bitrix24 `ONAPPINSTALL`: https://apidocs.bitrix24.com/api-reference/common/events/on-app-install.html +- Bitrix24 install finish behavior: https://apidocs.bitrix24.ru/api-reference/app-installation/installation-finish.html + +## US1: Install With Token + +If `Install\Command::$applicationToken !== null`, `src/ApplicationInstallations/UseCase/Install/Handler.php` finishes the installation in one step: + +1. Create master `Bitrix24Account` in `new` +2. Create `ApplicationInstallation` in `new` +3. Call `Bitrix24Account::applicationInstalled($applicationToken)` +4. Call `ApplicationInstallation::applicationInstalled($applicationToken)` +5. Persist both aggregates and flush once + +Result: + +- both aggregates become `active` +- application token is stored immediately +- finish events are emitted on `Install` + +```mermaid +sequenceDiagram + autonumber + actor Caller as Installer + participant Install as Install Handler + participant Account as Bitrix24Account + participant Installation as ApplicationInstallation + participant Flusher as Flusher + + Caller->>Install: handle(command with applicationToken) + Install->>Account: create(status=new) + Install->>Installation: create(status=new) + Install->>Account: applicationInstalled(token) + Install->>Installation: applicationInstalled(token) + Install->>Flusher: flush(installation, account) +``` + +## US2: Install Without Token + +If `Install\Command::$applicationToken === null`, `Install` only starts the installation: + +1. Create master `Bitrix24Account` in `new` +2. Create `ApplicationInstallation` in `new` +3. Persist both aggregates and flush once +4. Do not call `applicationInstalled(...)` + +Result: + +- both aggregates stay in `new` +- token is still unknown +- finish events are not emitted on `Install` + +The finish-step must then happen only in `src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php`. + +```mermaid +sequenceDiagram + autonumber + actor Caller as Installer + participant Install as Install Handler + participant Account as Bitrix24Account + participant Installation as ApplicationInstallation + participant Flusher as Flusher + + Caller->>Install: handle(command without applicationToken) + Install->>Account: create(status=new) + Install->>Installation: create(status=new) + Install->>Flusher: flush(installation, account) +``` + +## Canonical Finish-Step + +`OnAppInstall` is the canonical finish-step for the two-step install flow. + +Algorithm: + +1. Load current non-deleted installation by `memberId` +2. If installation is `new`: + - load master `Bitrix24Account` by `memberId` only in status `new` + - call `changeApplicationStatus(...)` + - call `Bitrix24Account::applicationInstalled($applicationToken)` + - call `ApplicationInstallation::applicationInstalled($applicationToken)` + - persist both aggregates and flush once +3. If installation is already `active`: + - load master `Bitrix24Account` by `memberId` only in status `active` + - log warning and return `no-op` +4. Otherwise: + - throw a controlled exception + +`Bitrix24Accounts\UseCase\InstallFinish\Handler` remains an account-only use case and is not the canonical finish path for `ApplicationInstallations`. + +```mermaid +sequenceDiagram + autonumber + actor B24 as Bitrix24 ONAPPINSTALL + participant Handler as OnAppInstall Handler + participant Repo as Installation Repository + participant AccountRepo as Account Repository + participant Account as Bitrix24Account + participant Installation as ApplicationInstallation + participant Flusher as Flusher + + B24->>Handler: handle(memberId, applicationToken, applicationStatus) + Handler->>Repo: findByBitrix24AccountMemberId(memberId) + alt pending installation in status new + Handler->>AccountRepo: findByMemberId(memberId, status=new) + Handler->>Installation: changeApplicationStatus(status) + Handler->>Account: applicationInstalled(token) + Handler->>Installation: applicationInstalled(token) + Handler->>Flusher: flush(installation, account) + else already active installation + Handler->>AccountRepo: findByMemberId(memberId, status=active) + Handler-->>B24: warning + no-op + else no pending installation + Handler-->>B24: controlled exception + end +``` + +## Corner Cases + +- Duplicate `ONAPPINSTALL` with the same token: + `OnAppInstall` does not mutate state, does not emit events, and writes a warning log entry. +- Repeated `ONAPPINSTALL` with a different token: + `OnAppInstall` still does not mutate state, does not emit events, and writes a warning log entry. +- Missing pending installation: + `OnAppInstall` throws `ApplicationInstallationNotFoundException`. +- Pending installation exists but master account in status `new` is missing: + `OnAppInstall` throws `Bitrix24AccountNotFoundException`. +- Reinstall over pending installation: + `Install` moves the previous installation through `markAsBlocked('reinstall before finish') -> applicationUninstalled(null)`, deletes related accounts, flushes deletions, and only then creates a new pair. +- Reinstall over active installation: + previous non-deleted aggregates are moved to `deleted`, then a new pair is created. + +## Reinstall Before Finish + +```mermaid +sequenceDiagram + autonumber + actor Caller as Installer + participant Install as Install Handler + participant OldInstallation as Old Installation + participant OldAccount as Old Master Account + participant Flusher as Flusher + participant NewAccount as New Account + participant NewInstallation as New Installation + + Caller->>Install: handle(new Install command) + Install->>OldInstallation: markAsBlocked('reinstall before finish') + Install->>OldInstallation: applicationUninstalled(null) + Install->>OldAccount: applicationUninstalled(null) + Install->>Flusher: flush(old installation, old account...) + Install->>NewAccount: create(status=new or active by token) + Install->>NewInstallation: create(status=new or active by token) + Install->>Flusher: flush(new installation, new account) +``` + +## Follow-Up + +Stale `new` installations remain possible when Bitrix24 never delivers the finish-step. This library does not auto-heal them synchronously. + +The follow-up issue tracks background cleanup options such as: + +- GitHub issue: https://github.com/mesilov/bitrix24-php-lib/issues/92 +- TTL worker that marks stale `new` installations as failed +- TTL worker with alert-only behavior +- reconciliation job for portal state re-check +- manual operational recovery flow diff --git a/src/ApplicationInstallations/UseCase/Install/Handler.php b/src/ApplicationInstallations/UseCase/Install/Handler.php index 3025bc4c..b441fbe7 100644 --- a/src/ApplicationInstallations/UseCase/Install/Handler.php +++ b/src/ApplicationInstallations/UseCase/Install/Handler.php @@ -8,8 +8,10 @@ use Bitrix24\Lib\Bitrix24Accounts\Entity\Bitrix24Account; use Bitrix24\Lib\Services\Flusher; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationInterface; +use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationStatus; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Repository\ApplicationInstallationRepositoryInterface; use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Entity\Bitrix24AccountInterface; +use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Entity\Bitrix24AccountStatus; use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Repository\Bitrix24AccountRepositoryInterface; use Bitrix24\SDK\Application\Contracts\Events\AggregateRootEventsEmitterInterface; use Bitrix24\SDK\Core\Exceptions\InvalidArgumentException; @@ -48,31 +50,7 @@ public function handle(Command $command): void $activeInstallation = $this->applicationInstallationRepository->findByBitrix24AccountMemberId($command->memberId); if (null !== $activeInstallation) { - $entitiesToFlush = []; - - $activeInstallation->applicationUninstalled(); - - $this->applicationInstallationRepository->save($activeInstallation); - - $entitiesToFlush[] = $activeInstallation; - - /** @var AggregateRootEventsEmitterInterface|Bitrix24AccountInterface[] $b24Accounts */ - $b24Accounts = $this->bitrix24AccountRepository->findByMemberId($command->memberId); - - if ([] !== $b24Accounts) { - foreach ($b24Accounts as $b24Account) { - $b24Account->applicationUninstalled(null); - $this->bitrix24AccountRepository->save($b24Account); - $entitiesToFlush[] = $b24Account; - } - } - - /* - Here flush immediately here, since this condition does not always work, - and it was better to at first to deal with accounts and installers - which need to be deactivated, and then we are already working with new entities. - */ - $this->flusher->flush(...array_filter($entitiesToFlush, fn ($entity): bool => $entity instanceof AggregateRootEventsEmitterInterface)); + $this->deactivateCurrentInstallation($command->memberId, $activeInstallation); } $uuidV7 = Uuid::v7(); @@ -91,7 +69,9 @@ public function handle(Command $command): void true ); - $bitrix24Account->applicationInstalled($command->applicationToken); + if (null !== $command->applicationToken) { + $bitrix24Account->applicationInstalled($command->applicationToken); + } $this->bitrix24AccountRepository->save($bitrix24Account); @@ -109,7 +89,9 @@ public function handle(Command $command): void true ); - $applicationInstallation->applicationInstalled($command->applicationToken); + if (null !== $command->applicationToken) { + $applicationInstallation->applicationInstalled($command->applicationToken); + } $this->applicationInstallationRepository->save($applicationInstallation); @@ -123,4 +105,44 @@ public function handle(Command $command): void ] ); } + + /** + * Flush deleted entities before creating a new installation pair for the same member id. + * + * @param AggregateRootEventsEmitterInterface&ApplicationInstallationInterface $applicationInstallation + * + * @throws InvalidArgumentException + */ + private function deactivateCurrentInstallation( + string $memberId, + ApplicationInstallationInterface $applicationInstallation + ): void { + $entitiesToFlush = []; + + if (ApplicationInstallationStatus::new === $applicationInstallation->getStatus()) { + $applicationInstallation->markAsBlocked('reinstall before finish'); + } + + $applicationInstallation->applicationUninstalled(null); + $this->applicationInstallationRepository->save($applicationInstallation); + $entitiesToFlush[] = $applicationInstallation; + + /** @var AggregateRootEventsEmitterInterface|Bitrix24AccountInterface[] $b24Accounts */ + $b24Accounts = $this->bitrix24AccountRepository->findByMemberId($memberId); + + foreach ($b24Accounts as $b24Account) { + if (Bitrix24AccountStatus::deleted === $b24Account->getStatus()) { + continue; + } + + $b24Account->applicationUninstalled(null); + $this->bitrix24AccountRepository->save($b24Account); + $entitiesToFlush[] = $b24Account; + } + + $this->flusher->flush(...array_filter( + $entitiesToFlush, + static fn ($entity): bool => $entity instanceof AggregateRootEventsEmitterInterface + )); + } } diff --git a/src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php b/src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php index 6c9b043b..b2626801 100644 --- a/src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php +++ b/src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php @@ -6,6 +6,7 @@ use Bitrix24\Lib\Services\Flusher; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationInterface; +use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationStatus; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Exceptions\ApplicationInstallationNotFoundException; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Repository\ApplicationInstallationRepositoryInterface; use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Entity\Bitrix24AccountInterface; @@ -43,34 +44,80 @@ public function handle(Command $command): void $applicationInstallation = $this->applicationInstallationRepository->findByBitrix24AccountMemberId($command->memberId); if (null === $applicationInstallation) { - throw new ApplicationInstallationNotFoundException( - sprintf('Application installation not found for member ID %s', $command->memberId) - ); + throw $this->buildInstallationNotFoundException($command->memberId); } - $applicationInstallation->changeApplicationStatus($command->applicationStatus); + if (ApplicationInstallationStatus::new === $applicationInstallation->getStatus()) { + $this->finishPendingInstallation($command, $applicationInstallation); - $applicationInstallation->setApplicationToken($command->applicationToken); + $this->logger->info('ApplicationInstallation.OnAppInstall.finish'); - $this->applicationInstallationRepository->save($applicationInstallation); + return; + } + if (ApplicationInstallationStatus::active === $applicationInstallation->getStatus()) { + $this->handleRepeatedEvent($command, $applicationInstallation); + + return; + } + + throw $this->buildInstallationNotFoundException($command->memberId); + } + + /** + * @param AggregateRootEventsEmitterInterface&ApplicationInstallationInterface $applicationInstallation + * + * @throws Bitrix24AccountNotFoundException|InvalidArgumentException|MultipleBitrix24AccountsFoundException + */ + private function finishPendingInstallation( + Command $command, + ApplicationInstallationInterface $applicationInstallation + ): void { /** @var AggregateRootEventsEmitterInterface|Bitrix24AccountInterface $bitrix24Account */ - $bitrix24Account = $this->findMasterAccountByMemberId($command->memberId); + $bitrix24Account = $this->findMasterAccountByMemberId($command->memberId, Bitrix24AccountStatus::new); - $bitrix24Account->setApplicationToken($command->applicationToken); + $applicationInstallation->changeApplicationStatus($command->applicationStatus); + $bitrix24Account->applicationInstalled($command->applicationToken); + $applicationInstallation->applicationInstalled($command->applicationToken); + $this->applicationInstallationRepository->save($applicationInstallation); $this->bitrix24AccountRepository->save($bitrix24Account); $this->flusher->flush($applicationInstallation, $bitrix24Account); - - $this->logger->info('ApplicationInstallation.OnAppInstall.finish'); } - private function findMasterAccountByMemberId(string $memberId): Bitrix24AccountInterface + /** + * @param AggregateRootEventsEmitterInterface&ApplicationInstallationInterface $applicationInstallation + * + * @throws Bitrix24AccountNotFoundException|MultipleBitrix24AccountsFoundException + */ + private function handleRepeatedEvent(Command $command, ApplicationInstallationInterface $applicationInstallation): void { + /** @var AggregateRootEventsEmitterInterface|Bitrix24AccountInterface $bitrix24Account */ + $bitrix24Account = $this->findMasterAccountByMemberId($command->memberId, Bitrix24AccountStatus::active); + + $sameToken = $applicationInstallation->isApplicationTokenValid($command->applicationToken) + && $bitrix24Account->isApplicationTokenValid($command->applicationToken); + + $this->logger->warning('ApplicationInstallation.OnAppInstall.duplicate', [ + 'memberId' => $command->memberId, + 'domain' => $command->domainUrl->value, + 'applicationToken' => $command->applicationToken, + 'tokenMatch' => $sameToken, + ]); + } + + /** + * @throws Bitrix24AccountNotFoundException + * @throws MultipleBitrix24AccountsFoundException + */ + private function findMasterAccountByMemberId( + string $memberId, + Bitrix24AccountStatus $bitrix24AccountStatus + ): Bitrix24AccountInterface { $bitrix24Accounts = $this->bitrix24AccountRepository->findByMemberId( $memberId, - Bitrix24AccountStatus::active, + $bitrix24AccountStatus, null, null ); @@ -82,13 +129,24 @@ private function findMasterAccountByMemberId(string $memberId): Bitrix24AccountI ); if ([] === $masterAccounts) { - throw new Bitrix24AccountNotFoundException('Bitrix24 account not found for member ID '.$memberId); + throw new Bitrix24AccountNotFoundException( + sprintf('Bitrix24 account with status %s not found for member ID %s', $bitrix24AccountStatus->value, $memberId) + ); } if (1 !== count($masterAccounts)) { - throw new MultipleBitrix24AccountsFoundException('Multiple Bitrix24 accounts found for member ID '.$memberId); + throw new MultipleBitrix24AccountsFoundException( + sprintf('Multiple Bitrix24 accounts with status %s found for member ID %s', $bitrix24AccountStatus->value, $memberId) + ); } return reset($masterAccounts); } + + private function buildInstallationNotFoundException(string $memberId): ApplicationInstallationNotFoundException + { + return new ApplicationInstallationNotFoundException( + sprintf('Pending application installation not found for member ID %s', $memberId) + ); + } } diff --git a/tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php b/tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php index 45846d33..2aa034f4 100644 --- a/tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php +++ b/tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php @@ -1,353 +1,204 @@ - * - * For the full copyright and license information, please view the MIT-LICENSE.txt - * file that was distributed with this source code. - */ - declare(strict_types=1); namespace Bitrix24\Lib\Tests\Functional\ApplicationInstallations\UseCase\Install; - -use Bitrix24\Lib\Bitrix24Accounts; - +use Bitrix24\Lib\ApplicationInstallations\Entity\ApplicationInstallation; +use Bitrix24\Lib\ApplicationInstallations\Infrastructure\Doctrine\ApplicationInstallationRepository; +use Bitrix24\Lib\ApplicationInstallations\UseCase\Install\Command; +use Bitrix24\Lib\ApplicationInstallations\UseCase\Install\Handler; +use Bitrix24\Lib\Bitrix24Accounts\Entity\Bitrix24Account; +use Bitrix24\Lib\Bitrix24Accounts\Infrastructure\Doctrine\Bitrix24AccountRepository; use Bitrix24\Lib\Bitrix24Accounts\ValueObjects\Domain; use Bitrix24\Lib\Services\Flusher; -use Bitrix24\Lib\ApplicationInstallations; use Bitrix24\Lib\Tests\EntityManagerFactory; - -use Bitrix24\Lib\Tests\Functional\ApplicationInstallations\Builders\ApplicationInstallationBuilder; -use Bitrix24\Lib\Tests\Functional\Bitrix24Accounts\Builders\Bitrix24AccountBuilder; use Bitrix24\SDK\Application\ApplicationStatus; -use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationInterface; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationStatus; +use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationBlockedEvent; +use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationCreatedEvent; +use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationFinishedEvent; +use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationUninstalledEvent; use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Entity\Bitrix24AccountStatus; -use Bitrix24\SDK\Application\Contracts\Events\AggregateRootEventsEmitterInterface; +use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Events\Bitrix24AccountApplicationInstalledEvent; +use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Events\Bitrix24AccountApplicationUninstalledEvent; +use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Events\Bitrix24AccountCreatedEvent; use Bitrix24\SDK\Application\PortalLicenseFamily; +use Bitrix24\SDK\Core\Credentials\AuthToken; use Bitrix24\SDK\Core\Credentials\Scope; -use Bitrix24\SDK\Core\Exceptions\InvalidArgumentException; - +use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; - use Symfony\Component\EventDispatcher\Debug\TraceableEventDispatcher; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\Stopwatch\Stopwatch; - -use Bitrix24\Lib\ApplicationInstallations\Infrastructure\Doctrine\ApplicationInstallationRepository; -use Bitrix24\Lib\ApplicationInstallations\UseCase\Install\Handler; - -use Bitrix24\Lib\Bitrix24Accounts\Infrastructure\Doctrine\Bitrix24AccountRepository; use Symfony\Component\Uid\Uuid; /** * @internal */ -#[CoversClass(ApplicationInstallations\UseCase\Install\Handler::class)] +#[CoversClass(Handler::class)] class HandlerTest extends TestCase { - private Handler $handler; + private EntityManagerInterface $entityManager; - private Flusher $flusher; + private Handler $handler; - private ApplicationInstallationRepository $repository; + private ApplicationInstallationRepository $installationRepository; - private Bitrix24AccountRepository $bitrix24accountRepository; + private Bitrix24AccountRepository $accountRepository; private TraceableEventDispatcher $eventDispatcher; #[\Override] protected function setUp(): void { - $entityManager = EntityManagerFactory::get(); + $this->entityManager = EntityManagerFactory::get(); $this->eventDispatcher = new TraceableEventDispatcher(new EventDispatcher(), new Stopwatch()); - $this->repository = new ApplicationInstallationRepository($entityManager); - $this->flusher = new Flusher($entityManager, $this->eventDispatcher); - $this->bitrix24accountRepository = new Bitrix24AccountRepository($entityManager); + $this->installationRepository = new ApplicationInstallationRepository($this->entityManager); + $this->accountRepository = new Bitrix24AccountRepository($this->entityManager); + $this->handler = new Handler( - $this->bitrix24accountRepository, - $this->repository, - $this->flusher, + $this->accountRepository, + $this->installationRepository, + new Flusher($this->entityManager, $this->eventDispatcher), new NullLogger() ); - } - /** - * @throws InvalidArgumentException - */ #[Test] - public function testNewInstallation(): void + public function testInstallWithoutTokenCreatesPendingEntities(): void { - $bitrix24AccountBuilder = (new Bitrix24AccountBuilder()) - ->withApplicationScope(new Scope(['crm'])) - ->build(); - + $command = $this->createCommand(); - $applicationInstallationBuilder = (new ApplicationInstallationBuilder()) - ->withApplicationStatus(new ApplicationStatus('F')) - ->withPortalLicenseFamily(PortalLicenseFamily::free) - ->build(); + $this->handler->handle($command); + $this->entityManager->clear(); + $installation = $this->installationRepository->findByBitrix24AccountMemberId($command->memberId); + $accounts = $this->accountRepository->findByMemberId($command->memberId); + $account = $accounts[0]; - $this->handler->handle( - new ApplicationInstallations\UseCase\Install\Command( - $bitrix24AccountBuilder->getMemberId(), - new Domain($bitrix24AccountBuilder->getDomainUrl()), - $bitrix24AccountBuilder->getAuthToken(), - $bitrix24AccountBuilder->getApplicationVersion(), - $bitrix24AccountBuilder->getApplicationScope(), - $bitrix24AccountBuilder->getBitrix24UserId(), - $bitrix24AccountBuilder->isBitrix24UserAdmin(), - $applicationInstallationBuilder->getApplicationStatus(), - $applicationInstallationBuilder->getPortalLicenseFamily(), - null, // applicationToken - $applicationInstallationBuilder->getPortalUsersCount(), - $applicationInstallationBuilder->getContactPersonId(), - $applicationInstallationBuilder->getBitrix24PartnerContactPersonId(), - $applicationInstallationBuilder->getBitrix24PartnerId(), - $applicationInstallationBuilder->getExternalId(), - $applicationInstallationBuilder->getComment() - ) - ); - - $dispatchedEvents = $this->eventDispatcher->getOrphanedEvents(); + self::assertNotNull($installation); + self::assertSame(ApplicationInstallationStatus::new, $installation->getStatus()); + self::assertSame(Bitrix24AccountStatus::new, $account->getStatus()); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Events\Bitrix24AccountCreatedEvent::class, $dispatchedEvents); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationCreatedEvent::class, $dispatchedEvents); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Events\Bitrix24AccountApplicationInstalledEvent::class, $dispatchedEvents); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationFinishedEvent::class, $dispatchedEvents); + $events = $this->eventDispatcher->getOrphanedEvents(); + self::assertContains(Bitrix24AccountCreatedEvent::class, $events); + self::assertContains(ApplicationInstallationCreatedEvent::class, $events); + self::assertNotContains(Bitrix24AccountApplicationInstalledEvent::class, $events); + self::assertNotContains(ApplicationInstallationFinishedEvent::class, $events); } - /** - * @throws InvalidArgumentException - */ #[Test] - public function testNewInstallationWithToken(): void + public function testInstallWithTokenFinishesInstallationInOneStep(): void { - $bitrix24AccountBuilder = (new Bitrix24AccountBuilder()) - ->withApplicationScope(new Scope(['crm'])) - ->build(); - - - $applicationInstallationBuilder = (new ApplicationInstallationBuilder()) - ->withApplicationStatus(new ApplicationStatus('F')) - ->withPortalLicenseFamily(PortalLicenseFamily::free) - ->build(); - - $newApplicationToken = Uuid::v7()->toRfc4122(); - - $this->handler->handle( - new ApplicationInstallations\UseCase\Install\Command( - $bitrix24AccountBuilder->getMemberId(), - new Domain($bitrix24AccountBuilder->getDomainUrl()), - $bitrix24AccountBuilder->getAuthToken(), - $bitrix24AccountBuilder->getApplicationVersion(), - $bitrix24AccountBuilder->getApplicationScope(), - $bitrix24AccountBuilder->getBitrix24UserId(), - $bitrix24AccountBuilder->isBitrix24UserAdmin(), - $applicationInstallationBuilder->getApplicationStatus(), - $applicationInstallationBuilder->getPortalLicenseFamily(), - $newApplicationToken, // applicationToken - $applicationInstallationBuilder->getPortalUsersCount(), - $applicationInstallationBuilder->getContactPersonId(), - $applicationInstallationBuilder->getBitrix24PartnerContactPersonId(), - $applicationInstallationBuilder->getBitrix24PartnerId(), - $applicationInstallationBuilder->getExternalId(), - $applicationInstallationBuilder->getComment() - ) - ); + $applicationToken = Uuid::v7()->toRfc4122(); + $command = $this->createCommand($applicationToken); + $this->handler->handle($command); + $this->entityManager->clear(); - $activeInstallation = $this->repository->findByApplicationToken($newApplicationToken); - $this->assertNotNull($activeInstallation); + $installation = $this->installationRepository->findByApplicationToken($applicationToken); + $accounts = $this->accountRepository->findByApplicationToken($applicationToken); - $dispatchedEvents = $this->eventDispatcher->getOrphanedEvents(); + self::assertNotNull($installation); + self::assertCount(1, $accounts); + self::assertSame(ApplicationInstallationStatus::active, $installation->getStatus()); + self::assertSame(Bitrix24AccountStatus::active, $accounts[0]->getStatus()); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Events\Bitrix24AccountCreatedEvent::class, $dispatchedEvents); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationCreatedEvent::class, $dispatchedEvents); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Events\Bitrix24AccountApplicationInstalledEvent::class, $dispatchedEvents); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationFinishedEvent::class, $dispatchedEvents); + $events = $this->eventDispatcher->getOrphanedEvents(); + self::assertContains(Bitrix24AccountCreatedEvent::class, $events); + self::assertContains(ApplicationInstallationCreatedEvent::class, $events); + self::assertContains(Bitrix24AccountApplicationInstalledEvent::class, $events); + self::assertContains(ApplicationInstallationFinishedEvent::class, $events); } - /** - * @throws InvalidArgumentException - */ #[Test] - public function testNewInstallationWithEmptyToken(): void + public function testReinstallOverPendingInstallationDeletesOldEntitiesAndCreatesNewPendingPair(): void { - $bitrix24AccountBuilder = (new Bitrix24AccountBuilder()) - ->withApplicationScope(new Scope(['crm'])) - ->build(); - - - $applicationInstallationBuilder = (new ApplicationInstallationBuilder()) - ->withApplicationStatus(new ApplicationStatus('F')) - ->withPortalLicenseFamily(PortalLicenseFamily::free) - ->build(); + $memberId = Uuid::v4()->toRfc4122(); + $bitrix24Account = $this->createAccount($memberId); + $applicationInstallation = $this->createInstallation($bitrix24Account->getId()); + + $this->entityManager->persist($bitrix24Account); + $this->entityManager->persist($applicationInstallation); + $this->entityManager->flush(); + + $this->handler->handle($this->createCommand(null, $memberId)); + $this->entityManager->clear(); + + /** @var Bitrix24Account $deletedAccount */ + $deletedAccount = $this->entityManager->find(Bitrix24Account::class, $bitrix24Account->getId()); + /** @var ApplicationInstallation $deletedInstallation */ + $deletedInstallation = $this->entityManager->find(ApplicationInstallation::class, $applicationInstallation->getId()); + $currentInstallation = $this->installationRepository->findByBitrix24AccountMemberId($memberId); + + self::assertNotNull($currentInstallation); + self::assertSame(Bitrix24AccountStatus::deleted, $deletedAccount->getStatus()); + self::assertSame(ApplicationInstallationStatus::deleted, $deletedInstallation->getStatus()); + self::assertSame('reinstall before finish', $deletedInstallation->getComment()); + self::assertSame(ApplicationInstallationStatus::new, $currentInstallation->getStatus()); + self::assertNotSame($applicationInstallation->getId()->toRfc4122(), $currentInstallation->getId()->toRfc4122()); + + $events = $this->eventDispatcher->getOrphanedEvents(); + self::assertContains(ApplicationInstallationBlockedEvent::class, $events); + self::assertContains(ApplicationInstallationUninstalledEvent::class, $events); + self::assertContains(Bitrix24AccountApplicationUninstalledEvent::class, $events); + self::assertContains(Bitrix24AccountCreatedEvent::class, $events); + self::assertContains(ApplicationInstallationCreatedEvent::class, $events); + self::assertNotContains(Bitrix24AccountApplicationInstalledEvent::class, $events); + self::assertNotContains(ApplicationInstallationFinishedEvent::class, $events); + } - $this->expectException(InvalidArgumentException::class); - $this->handler->handle( - new ApplicationInstallations\UseCase\Install\Command( - $bitrix24AccountBuilder->getMemberId(), - new Domain($bitrix24AccountBuilder->getDomainUrl()), - $bitrix24AccountBuilder->getAuthToken(), - $bitrix24AccountBuilder->getApplicationVersion(), - $bitrix24AccountBuilder->getApplicationScope(), - $bitrix24AccountBuilder->getBitrix24UserId(), - $bitrix24AccountBuilder->isBitrix24UserAdmin(), - $applicationInstallationBuilder->getApplicationStatus(), - $applicationInstallationBuilder->getPortalLicenseFamily(), - '', // applicationToken - $applicationInstallationBuilder->getPortalUsersCount(), - $applicationInstallationBuilder->getContactPersonId(), - $applicationInstallationBuilder->getBitrix24PartnerContactPersonId(), - $applicationInstallationBuilder->getBitrix24PartnerId(), - $applicationInstallationBuilder->getExternalId(), - $applicationInstallationBuilder->getComment() - ) + private function createCommand(?string $applicationToken = null, ?string $memberId = null): Command + { + return new Command( + memberId: $memberId ?? Uuid::v4()->toRfc4122(), + domain: new Domain('example.bitrix24.test'), + authToken: new AuthToken('access', 'refresh', 3600, time() + 3600), + applicationVersion: 1, + applicationScope: new Scope(['crm']), + bitrix24UserId: 1, + isBitrix24UserAdmin: true, + applicationStatus: new ApplicationStatus('F'), + portalLicenseFamily: PortalLicenseFamily::free, + applicationToken: $applicationToken, + portalUsersCount: 10, + externalId: 'lead-1', + comment: 'install' ); } - /** - * @throws InvalidArgumentException - */ - #[Test] - public function testReinstallInstallation(): void + private function createAccount(string $memberId): Bitrix24Account { - - $memberId = Uuid::v4()->toRfc4122(); - - // Load account and application installation into database for reinstallation testing. - $applicationToken = Uuid::v7()->toRfc4122(); - $currentBitrix24Account = (new Bitrix24AccountBuilder()) - ->withApplicationScope(new Scope(['crm'])) - ->withStatus(Bitrix24AccountStatus::new) - ->withApplicationToken($applicationToken) - ->withMemberId($memberId) - ->withMaster(true) - ->build(); - - - $applicationInstallation = (new ApplicationInstallationBuilder()) - ->withApplicationStatus(new ApplicationStatus('F')) - ->withPortalLicenseFamily(PortalLicenseFamily::free) - ->withBitrix24AccountId($currentBitrix24Account->getId()) - ->withApplicationStatusInstallation(ApplicationInstallationStatus::active) - ->build(); - - $this->bitrix24accountRepository->save($currentBitrix24Account); - $this->repository->save($applicationInstallation); - $this->flusher->flush(); - - $bitrix24AccountBuilder = (new Bitrix24AccountBuilder()) - ->withApplicationScope(new Scope(['crm'])) - ->withMemberId($memberId) - ->build(); - - $applicationInstallationBuilder = (new ApplicationInstallationBuilder()) - ->withApplicationStatus(new ApplicationStatus('F')) - ->withPortalLicenseFamily(PortalLicenseFamily::free) - ->build(); - - $this->handler->handle( - new ApplicationInstallations\UseCase\Install\Command( - $bitrix24AccountBuilder->getMemberId(), - new Domain($bitrix24AccountBuilder->getDomainUrl()), - $bitrix24AccountBuilder->getAuthToken(), - $bitrix24AccountBuilder->getApplicationVersion(), - $bitrix24AccountBuilder->getApplicationScope(), - $bitrix24AccountBuilder->getBitrix24UserId(), - $bitrix24AccountBuilder->isBitrix24UserAdmin(), - $applicationInstallationBuilder->getApplicationStatus(), - $applicationInstallationBuilder->getPortalLicenseFamily(), - null, // applicationToken - $applicationInstallationBuilder->getPortalUsersCount(), - $applicationInstallationBuilder->getContactPersonId(), - $applicationInstallationBuilder->getBitrix24PartnerContactPersonId(), - $applicationInstallationBuilder->getBitrix24PartnerId(), - $applicationInstallationBuilder->getExternalId(), - $applicationInstallationBuilder->getComment() - ) + return new Bitrix24Account( + Uuid::v7(), + 1, + true, + $memberId, + 'example.bitrix24.test', + new AuthToken('access', 'refresh', 3600, time() + 3600), + 1, + new Scope(['crm']), + true ); - - $dispatchedEvents = $this->eventDispatcher->getOrphanedEvents(); - - $this->assertContains(\Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Events\Bitrix24AccountCreatedEvent::class, $dispatchedEvents); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationCreatedEvent::class, $dispatchedEvents); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Events\Bitrix24AccountApplicationInstalledEvent::class, $dispatchedEvents); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationFinishedEvent::class, $dispatchedEvents); } - /** - * @throws InvalidArgumentException - */ - #[Test] - public function testFewInstallationsOnOneAccount(): void + private function createInstallation(Uuid $uuid): ApplicationInstallation { - - $memberId = Uuid::v4()->toRfc4122(); - - // Load account and application installation into database for reinstallation testing. - $applicationToken = Uuid::v7()->toRfc4122(); - $currentBitrix24Account = (new Bitrix24AccountBuilder()) - ->withApplicationScope(new Scope(['crm'])) - ->withStatus(Bitrix24AccountStatus::new) - ->withApplicationToken($applicationToken) - ->withMemberId($memberId) - ->build(); - - - $applicationInstallation = (new ApplicationInstallationBuilder()) - ->withApplicationStatus(new ApplicationStatus('F')) - ->withPortalLicenseFamily(PortalLicenseFamily::free) - ->withBitrix24AccountId($currentBitrix24Account->getId()) - ->withApplicationStatusInstallation(ApplicationInstallationStatus::active) - ->build(); - - $this->bitrix24accountRepository->save($currentBitrix24Account); - $this->repository->save($applicationInstallation); - $this->flusher->flush(); - - $applicationInstallationBuilder = (new ApplicationInstallationBuilder()) - ->withApplicationStatus(new ApplicationStatus('F')) - ->withPortalLicenseFamily(PortalLicenseFamily::free) - ->build(); - - $this->handler->handle( - new ApplicationInstallations\UseCase\Install\Command( - $currentBitrix24Account->getMemberId(), - new Domain($currentBitrix24Account->getDomainUrl()), - $currentBitrix24Account->getAuthToken(), - $currentBitrix24Account->getApplicationVersion(), - $currentBitrix24Account->getApplicationScope(), - $currentBitrix24Account->getBitrix24UserId(), - $currentBitrix24Account->isBitrix24UserAdmin(), - $applicationInstallationBuilder->getApplicationStatus(), - $applicationInstallationBuilder->getPortalLicenseFamily(), - null, // applicationToken - $applicationInstallationBuilder->getPortalUsersCount(), - $applicationInstallationBuilder->getContactPersonId(), - $applicationInstallationBuilder->getBitrix24PartnerContactPersonId(), - $applicationInstallationBuilder->getBitrix24PartnerId(), - $applicationInstallationBuilder->getExternalId(), - $applicationInstallationBuilder->getComment() - ) + return new ApplicationInstallation( + Uuid::v7(), + $uuid, + new ApplicationStatus('F'), + PortalLicenseFamily::free, + 10, + null, + null, + null, + 'lead-1', + 'install' ); - - $dispatchedEvents = $this->eventDispatcher->getOrphanedEvents(); - - $this->assertContains(\Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Events\Bitrix24AccountCreatedEvent::class, $dispatchedEvents); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationCreatedEvent::class, $dispatchedEvents); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Events\Bitrix24AccountApplicationInstalledEvent::class, $dispatchedEvents); - $this->assertContains(\Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationFinishedEvent::class, $dispatchedEvents); } - } diff --git a/tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php b/tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php index 01e8c2f2..85074f49 100644 --- a/tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php +++ b/tests/Functional/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php @@ -1,134 +1,271 @@ - * - * For the full copyright and license information, please view the MIT-LICENSE.txt - * file that was distributed with this source code. - */ - declare(strict_types=1); namespace Bitrix24\Lib\Tests\Functional\ApplicationInstallations\UseCase\OnAppInstall; - -use Bitrix24\Lib\Bitrix24Accounts; - +use Bitrix24\Lib\ApplicationInstallations\Entity\ApplicationInstallation; +use Bitrix24\Lib\ApplicationInstallations\Infrastructure\Doctrine\ApplicationInstallationRepository; +use Bitrix24\Lib\ApplicationInstallations\UseCase\OnAppInstall\Command; +use Bitrix24\Lib\ApplicationInstallations\UseCase\OnAppInstall\Handler; +use Bitrix24\Lib\Bitrix24Accounts\Entity\Bitrix24Account; +use Bitrix24\Lib\Bitrix24Accounts\Infrastructure\Doctrine\Bitrix24AccountRepository; use Bitrix24\Lib\Bitrix24Accounts\ValueObjects\Domain; use Bitrix24\Lib\Services\Flusher; -use Bitrix24\Lib\ApplicationInstallations; use Bitrix24\Lib\Tests\EntityManagerFactory; - -use Bitrix24\Lib\Tests\Functional\ApplicationInstallations\Builders\ApplicationInstallationBuilder; -use Bitrix24\Lib\Tests\Functional\Bitrix24Accounts\Builders\Bitrix24AccountBuilder; use Bitrix24\SDK\Application\ApplicationStatus; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationStatus; +use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationFinishedEvent; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Exceptions\ApplicationInstallationNotFoundException; use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Entity\Bitrix24AccountStatus; +use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Events\Bitrix24AccountApplicationInstalledEvent; use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Exceptions\Bitrix24AccountNotFoundException; use Bitrix24\SDK\Application\PortalLicenseFamily; +use Bitrix24\SDK\Core\Credentials\AuthToken; use Bitrix24\SDK\Core\Credentials\Scope; -use Bitrix24\SDK\Core\Exceptions\InvalidArgumentException; - +use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -use Psr\Log\NullLogger; - +use Psr\Log\AbstractLogger; use Symfony\Component\EventDispatcher\Debug\TraceableEventDispatcher; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\Stopwatch\Stopwatch; - -use Bitrix24\Lib\ApplicationInstallations\Infrastructure\Doctrine\ApplicationInstallationRepository; -use Bitrix24\Lib\ApplicationInstallations\UseCase\OnAppInstall\Handler; - -use Bitrix24\Lib\Bitrix24Accounts\Infrastructure\Doctrine\Bitrix24AccountRepository; use Symfony\Component\Uid\Uuid; /** * @internal */ -#[CoversClass(ApplicationInstallations\UseCase\OnAppInstall\Handler::class)] +#[CoversClass(Handler::class)] class HandlerTest extends TestCase { - private Handler $handler; - - private Flusher $flusher; + private EntityManagerInterface $entityManager; - private ApplicationInstallationRepository $applicationInstallationRepository; + private ApplicationInstallationRepository $installationRepository; - private Bitrix24AccountRepository $bitrix24accountRepository; + private Bitrix24AccountRepository $accountRepository; private TraceableEventDispatcher $eventDispatcher; + private TestLogger $logger; + + private Handler $handler; + #[\Override] protected function setUp(): void { - $entityManager = EntityManagerFactory::get(); + $this->entityManager = EntityManagerFactory::get(); + $this->installationRepository = new ApplicationInstallationRepository($this->entityManager); + $this->accountRepository = new Bitrix24AccountRepository($this->entityManager); $this->eventDispatcher = new TraceableEventDispatcher(new EventDispatcher(), new Stopwatch()); - $this->applicationInstallationRepository = new ApplicationInstallationRepository($entityManager); - $this->flusher = new Flusher($entityManager, $this->eventDispatcher); - $this->bitrix24accountRepository = new Bitrix24AccountRepository($entityManager); + $this->logger = new TestLogger(); + $this->handler = new Handler( - $this->bitrix24accountRepository, - $this->applicationInstallationRepository, - $this->flusher, - new NullLogger() + $this->accountRepository, + $this->installationRepository, + new Flusher($this->entityManager, $this->eventDispatcher), + $this->logger ); + } + + #[Test] + public function testOnAppInstallFinishesPendingInstallation(): void + { + $memberId = Uuid::v4()->toRfc4122(); + $applicationToken = Uuid::v7()->toRfc4122(); + $bitrix24Account = $this->createAccount($memberId); + $applicationInstallation = $this->createInstallation($bitrix24Account->getId(), new ApplicationStatus('F')); + + $this->entityManager->persist($bitrix24Account); + $this->entityManager->persist($applicationInstallation); + $this->entityManager->flush(); + + $this->handler->handle(new Command( + memberId: $memberId, + domainUrl: new Domain('example.bitrix24.test'), + applicationToken: $applicationToken, + applicationStatus: new ApplicationStatus('T') + )); + $this->entityManager->clear(); + + $updatedInstallation = $this->installationRepository->findByBitrix24AccountMemberId($memberId); + $updatedAccount = $this->accountRepository->findByMemberId($memberId, Bitrix24AccountStatus::active, null, null, true)[0]; + + self::assertNotNull($updatedInstallation); + self::assertSame(ApplicationInstallationStatus::active, $updatedInstallation->getStatus()); + self::assertSame(Bitrix24AccountStatus::active, $updatedAccount->getStatus()); + self::assertSame('trial', $updatedInstallation->getApplicationStatus()->getStatusCode()); + self::assertTrue($updatedInstallation->isApplicationTokenValid($applicationToken)); + self::assertTrue($updatedAccount->isApplicationTokenValid($applicationToken)); + self::assertContains(ApplicationInstallationFinishedEvent::class, $this->eventDispatcher->getOrphanedEvents()); + self::assertContains(Bitrix24AccountApplicationInstalledEvent::class, $this->eventDispatcher->getOrphanedEvents()); } - /** - * @throws InvalidArgumentException|Bitrix24AccountNotFoundException|ApplicationInstallationNotFoundException - */ #[Test] - public function testEventOnAppInstall(): void + public function testDuplicateOnAppInstallWithSameTokenIsNoop(): void { $memberId = Uuid::v4()->toRfc4122(); - $domainUrl = Uuid::v4()->toRfc4122().'-example.com'; $applicationToken = Uuid::v7()->toRfc4122(); - $applicationStatus = new ApplicationStatus('T'); - - $bitrix24Account = (new Bitrix24AccountBuilder()) - ->withApplicationScope(new Scope(['crm'])) - ->withStatus(Bitrix24AccountStatus::new) - ->withMemberId($memberId) - ->withDomainUrl($domainUrl) - ->withInstalled() - ->withMaster(true) - ->build(); - - $applicationInstallationBuilder = (new ApplicationInstallationBuilder()) - ->withApplicationStatus(new ApplicationStatus('F')) - ->withPortalLicenseFamily(PortalLicenseFamily::free) - ->withBitrix24AccountId($bitrix24Account->getId()) - ->withApplicationStatusInstallation(ApplicationInstallationStatus::active) - ->build(); - - $this->bitrix24accountRepository->save($bitrix24Account); - $this->applicationInstallationRepository->save($applicationInstallationBuilder); - $this->flusher->flush(); - - $this->handler->handle( - new ApplicationInstallations\UseCase\OnAppInstall\Command( - $memberId, - new Domain($domainUrl), - $applicationToken, - $applicationStatus - ) + $bitrix24Account = $this->createActiveAccount($memberId, $applicationToken); + $applicationInstallation = $this->createActiveInstallation($bitrix24Account->getId(), $applicationToken, new ApplicationStatus('F')); + + $this->entityManager->persist($bitrix24Account); + $this->entityManager->persist($applicationInstallation); + $this->entityManager->flush(); + + $this->handler->handle(new Command( + memberId: $memberId, + domainUrl: new Domain('example.bitrix24.test'), + applicationToken: $applicationToken, + applicationStatus: new ApplicationStatus('T') + )); + + $this->entityManager->clear(); + + $updatedInstallation = $this->installationRepository->findByBitrix24AccountMemberId($memberId); + $updatedAccount = $this->accountRepository->findByMemberId($memberId, Bitrix24AccountStatus::active, null, null, true)[0]; + + self::assertSame(ApplicationInstallationStatus::active, $updatedInstallation->getStatus()); + self::assertSame('free', $updatedInstallation->getApplicationStatus()->getStatusCode()); + self::assertTrue($updatedInstallation->isApplicationTokenValid($applicationToken)); + self::assertTrue($updatedAccount->isApplicationTokenValid($applicationToken)); + self::assertSame([], $this->eventDispatcher->getOrphanedEvents()); + self::assertCount(1, $this->logger->warnings); + self::assertTrue($this->logger->warnings[0]['context']['tokenMatch']); + } + + #[Test] + public function testRepeatedOnAppInstallWithDifferentTokenIsNoop(): void + { + $memberId = Uuid::v4()->toRfc4122(); + $storedToken = Uuid::v7()->toRfc4122(); + $replayedToken = Uuid::v7()->toRfc4122(); + $bitrix24Account = $this->createActiveAccount($memberId, $storedToken); + $applicationInstallation = $this->createActiveInstallation($bitrix24Account->getId(), $storedToken, new ApplicationStatus('F')); + + $this->entityManager->persist($bitrix24Account); + $this->entityManager->persist($applicationInstallation); + $this->entityManager->flush(); + + $this->handler->handle(new Command( + memberId: $memberId, + domainUrl: new Domain('example.bitrix24.test'), + applicationToken: $replayedToken, + applicationStatus: new ApplicationStatus('T') + )); + + $this->entityManager->clear(); + + $updatedInstallation = $this->installationRepository->findByBitrix24AccountMemberId($memberId); + $updatedAccount = $this->accountRepository->findByMemberId($memberId, Bitrix24AccountStatus::active, null, null, true)[0]; + + self::assertSame('free', $updatedInstallation->getApplicationStatus()->getStatusCode()); + self::assertTrue($updatedInstallation->isApplicationTokenValid($storedToken)); + self::assertTrue($updatedAccount->isApplicationTokenValid($storedToken)); + self::assertSame([], $this->eventDispatcher->getOrphanedEvents()); + self::assertCount(1, $this->logger->warnings); + self::assertFalse($this->logger->warnings[0]['context']['tokenMatch']); + } + + #[Test] + public function testOnAppInstallThrowsWhenPendingInstallationMissing(): void + { + $this->expectException(ApplicationInstallationNotFoundException::class); + + $this->handler->handle(new Command( + memberId: Uuid::v4()->toRfc4122(), + domainUrl: new Domain('example.bitrix24.test'), + applicationToken: Uuid::v7()->toRfc4122(), + applicationStatus: new ApplicationStatus('T') + )); + } + + #[Test] + public function testOnAppInstallThrowsWhenPendingInstallationExistsButNewMasterAccountMissing(): void + { + $memberId = Uuid::v4()->toRfc4122(); + $bitrix24Account = $this->createActiveAccount($memberId, Uuid::v7()->toRfc4122()); + $applicationInstallation = $this->createInstallation($bitrix24Account->getId(), new ApplicationStatus('F')); + + $this->entityManager->persist($bitrix24Account); + $this->entityManager->persist($applicationInstallation); + $this->entityManager->flush(); + + $this->expectException(Bitrix24AccountNotFoundException::class); + + $this->handler->handle(new Command( + memberId: $memberId, + domainUrl: new Domain('example.bitrix24.test'), + applicationToken: Uuid::v7()->toRfc4122(), + applicationStatus: new ApplicationStatus('T') + )); + } + + private function createAccount(string $memberId): Bitrix24Account + { + return new Bitrix24Account( + Uuid::v7(), + 1, + true, + $memberId, + 'example.bitrix24.test', + new AuthToken('access', 'refresh', 3600, time() + 3600), + 1, + new Scope(['crm']), + true ); + } - $updated = $this->bitrix24accountRepository->getById($bitrix24Account->getId()); + private function createActiveAccount(string $memberId, string $applicationToken): Bitrix24Account + { + $bitrix24Account = $this->createAccount($memberId); + $bitrix24Account->applicationInstalled($applicationToken); + + return $bitrix24Account; + } - $this->assertTrue( - $updated->isApplicationTokenValid($applicationToken), - sprintf( - 'failed application token «%s» validation for bitrix24 account with id «%s»', - $applicationToken, - $bitrix24Account->getId()->toString() - ) + private function createInstallation(Uuid $uuid, ApplicationStatus $applicationStatus): ApplicationInstallation + { + return new ApplicationInstallation( + Uuid::v7(), + $uuid, + $applicationStatus, + PortalLicenseFamily::free, + 10, + null, + null, + null, + 'lead-1', + 'install' ); } + + private function createActiveInstallation( + Uuid $uuid, + string $applicationToken, + ApplicationStatus $applicationStatus + ): ApplicationInstallation { + $applicationInstallation = $this->createInstallation($uuid, $applicationStatus); + $applicationInstallation->applicationInstalled($applicationToken); + + return $applicationInstallation; + } +} + +final class TestLogger extends AbstractLogger +{ + /** @var array}> */ + public array $warnings = []; + + #[\Override] + public function log($level, string|\Stringable $message, array $context = []): void + { + if ('warning' === $level) { + $this->warnings[] = [ + 'message' => (string) $message, + 'context' => $context, + ]; + } + } } diff --git a/tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php b/tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php index 5d84d7e2..12721dd0 100644 --- a/tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php +++ b/tests/Functional/Bitrix24Accounts/UseCase/InstallFinish/HandlerTest.php @@ -64,8 +64,8 @@ protected function setUp(): void } #[Test] - #[TestDox('test finish installation application')] - public function testFinishInstallationApplication(): void + #[TestDox('test installFinish changes only account state and stays separate from OnAppInstall flow')] + public function testInstallFinishActivatesPendingAccountOnly(): void { $bitrix24Account = (new Bitrix24AccountBuilder()) ->withStatus(Bitrix24AccountStatus::new) diff --git a/tests/Unit/ApplicationInstallations/UseCase/Install/HandlerTest.php b/tests/Unit/ApplicationInstallations/UseCase/Install/HandlerTest.php new file mode 100644 index 00000000..5e05de00 --- /dev/null +++ b/tests/Unit/ApplicationInstallations/UseCase/Install/HandlerTest.php @@ -0,0 +1,242 @@ +bitrix24AccountRepository = $this->createMock(Bitrix24AccountRepositoryInterface::class); + $this->applicationInstallationRepository = $this->createMock(ApplicationInstallationRepositoryInterface::class); + $this->flusher = $this->createMock(Flusher::class); + + $this->handler = new Handler( + $this->bitrix24AccountRepository, + $this->applicationInstallationRepository, + $this->flusher, + new NullLogger() + ); + } + + #[Test] + public function testHandleWithoutTokenCreatesPendingEntities(): void + { + $command = $this->createCommand(); + $savedEntities = []; + + $this->applicationInstallationRepository + ->expects($this->once()) + ->method('findByBitrix24AccountMemberId') + ->with($command->memberId) + ->willReturn(null); + + $this->bitrix24AccountRepository + ->expects($this->once()) + ->method('save') + ->willReturnCallback(static function (Bitrix24Account $bitrix24Account) use (&$savedEntities): void { + $savedEntities['account'] = $bitrix24Account; + }); + + $this->applicationInstallationRepository + ->expects($this->once()) + ->method('save') + ->willReturnCallback(static function (ApplicationInstallation $applicationInstallation) use (&$savedEntities): void { + $savedEntities['installation'] = $applicationInstallation; + }); + + $this->flusher + ->expects($this->once()) + ->method('flush') + ->with( + $this->callback(static fn (ApplicationInstallation $applicationInstallation): bool => $applicationInstallation->getStatus() === ApplicationInstallationStatus::new), + $this->callback(static fn (Bitrix24Account $bitrix24Account): bool => $bitrix24Account->getStatus() === Bitrix24AccountStatus::new) + ); + + $this->handler->handle($command); + + self::assertArrayHasKey('account', $savedEntities); + self::assertArrayHasKey('installation', $savedEntities); + self::assertSame(Bitrix24AccountStatus::new, $savedEntities['account']->getStatus()); + self::assertSame(ApplicationInstallationStatus::new, $savedEntities['installation']->getStatus()); + } + + #[Test] + public function testHandleWithTokenFinishesInstallationInOneStep(): void + { + $applicationToken = Uuid::v7()->toRfc4122(); + $command = $this->createCommand($applicationToken); + $savedEntities = []; + + $this->applicationInstallationRepository + ->expects($this->once()) + ->method('findByBitrix24AccountMemberId') + ->with($command->memberId) + ->willReturn(null); + + $this->bitrix24AccountRepository + ->expects($this->once()) + ->method('save') + ->willReturnCallback(static function (Bitrix24Account $bitrix24Account) use (&$savedEntities): void { + $savedEntities['account'] = $bitrix24Account; + }); + + $this->applicationInstallationRepository + ->expects($this->once()) + ->method('save') + ->willReturnCallback(static function (ApplicationInstallation $applicationInstallation) use (&$savedEntities): void { + $savedEntities['installation'] = $applicationInstallation; + }); + + $this->flusher + ->expects($this->once()) + ->method('flush') + ->with( + $this->callback(static fn(ApplicationInstallation $applicationInstallation): bool => $applicationInstallation->getStatus() === ApplicationInstallationStatus::active + && $applicationInstallation->isApplicationTokenValid($applicationToken)), + $this->callback(static fn(Bitrix24Account $bitrix24Account): bool => $bitrix24Account->getStatus() === Bitrix24AccountStatus::active + && $bitrix24Account->isApplicationTokenValid($applicationToken)) + ); + + $this->handler->handle($command); + + self::assertSame(Bitrix24AccountStatus::active, $savedEntities['account']->getStatus()); + self::assertSame(ApplicationInstallationStatus::active, $savedEntities['installation']->getStatus()); + self::assertTrue($savedEntities['account']->isApplicationTokenValid($applicationToken)); + self::assertTrue($savedEntities['installation']->isApplicationTokenValid($applicationToken)); + } + + #[Test] + public function testHandleReinstallOverPendingInstallationDeletesOldPairBeforeCreatingNewOne(): void + { + $command = $this->createCommand(); + $bitrix24Account = $this->createAccount($command->memberId, true); + $applicationInstallation = $this->createInstallation($bitrix24Account->getId()); + $flushCalls = []; + + $this->applicationInstallationRepository + ->expects($this->exactly(2)) + ->method('save') + ->willReturnCallback(static function (ApplicationInstallation $applicationInstallation): void {}); + + $this->bitrix24AccountRepository + ->expects($this->exactly(2)) + ->method('save') + ->willReturnCallback(static function (Bitrix24Account $bitrix24Account): void {}); + + $this->applicationInstallationRepository + ->expects($this->once()) + ->method('findByBitrix24AccountMemberId') + ->with($command->memberId) + ->willReturn($applicationInstallation); + + $this->bitrix24AccountRepository + ->expects($this->once()) + ->method('findByMemberId') + ->with($command->memberId) + ->willReturn([$bitrix24Account]); + + $this->flusher + ->expects($this->exactly(2)) + ->method('flush') + ->willReturnCallback(static function (...$entities) use (&$flushCalls): void { + $flushCalls[] = $entities; + }); + + $this->handler->handle($command); + + self::assertCount(2, $flushCalls); + self::assertSame(ApplicationInstallationStatus::deleted, $applicationInstallation->getStatus()); + self::assertSame('reinstall before finish', $applicationInstallation->getComment()); + self::assertSame(Bitrix24AccountStatus::deleted, $bitrix24Account->getStatus()); + self::assertSame($applicationInstallation, $flushCalls[0][0]); + self::assertSame($bitrix24Account, $flushCalls[0][1]); + self::assertSame(ApplicationInstallationStatus::new, $flushCalls[1][0]->getStatus()); + self::assertSame(Bitrix24AccountStatus::new, $flushCalls[1][1]->getStatus()); + } + + private function createCommand(?string $applicationToken = null): Command + { + return new Command( + memberId: Uuid::v4()->toRfc4122(), + domain: new Domain('example.bitrix24.test'), + authToken: new AuthToken('access', 'refresh', 3600, time() + 3600), + applicationVersion: 1, + applicationScope: new Scope(['crm']), + bitrix24UserId: 1, + isBitrix24UserAdmin: true, + applicationStatus: new ApplicationStatus('F'), + portalLicenseFamily: PortalLicenseFamily::free, + applicationToken: $applicationToken, + portalUsersCount: 10, + externalId: 'lead-1', + comment: 'install' + ); + } + + private function createAccount(string $memberId, bool $isMaster): Bitrix24Account + { + return new Bitrix24Account( + Uuid::v7(), + 1, + true, + $memberId, + 'example.bitrix24.test', + new AuthToken('access', 'refresh', 3600, time() + 3600), + 1, + new Scope(['crm']), + $isMaster + ); + } + + private function createInstallation(Uuid $uuid): ApplicationInstallation + { + return new ApplicationInstallation( + Uuid::v7(), + $uuid, + new ApplicationStatus('F'), + PortalLicenseFamily::free, + 10, + null, + null, + null, + 'lead-1', + 'install' + ); + } +} diff --git a/tests/Unit/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php b/tests/Unit/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php new file mode 100644 index 00000000..ae541d83 --- /dev/null +++ b/tests/Unit/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php @@ -0,0 +1,278 @@ +bitrix24AccountRepository = $this->createMock(Bitrix24AccountRepositoryInterface::class); + $this->applicationInstallationRepository = $this->createMock(ApplicationInstallationRepositoryInterface::class); + $this->flusher = $this->createMock(Flusher::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->handler = new Handler( + $this->bitrix24AccountRepository, + $this->applicationInstallationRepository, + $this->flusher, + $this->logger + ); + } + + #[Test] + public function testHandleFinishesPendingInstallation(): void + { + $command = $this->createCommand(); + $bitrix24Account = $this->createAccount($command->memberId, true); + $applicationInstallation = $this->createInstallation($bitrix24Account->getId()); + + $this->applicationInstallationRepository + ->expects($this->once()) + ->method('findByBitrix24AccountMemberId') + ->with($command->memberId) + ->willReturn($applicationInstallation); + + $this->bitrix24AccountRepository + ->expects($this->once()) + ->method('findByMemberId') + ->with($command->memberId, Bitrix24AccountStatus::new, null, null) + ->willReturn([$bitrix24Account]); + + $this->applicationInstallationRepository->expects($this->once())->method('save')->with($applicationInstallation); + $this->bitrix24AccountRepository->expects($this->once())->method('save')->with($bitrix24Account); + + $this->flusher + ->expects($this->once()) + ->method('flush') + ->with($applicationInstallation, $bitrix24Account); + + $this->handler->handle($command); + + self::assertSame(ApplicationInstallationStatus::active, $applicationInstallation->getStatus()); + self::assertSame(Bitrix24AccountStatus::active, $bitrix24Account->getStatus()); + self::assertTrue($applicationInstallation->isApplicationTokenValid($command->applicationToken)); + self::assertTrue($bitrix24Account->isApplicationTokenValid($command->applicationToken)); + } + + #[Test] + public function testHandleDuplicateEventWithSameTokenIsNoop(): void + { + $applicationToken = Uuid::v7()->toRfc4122(); + $command = $this->createCommand($applicationToken, new ApplicationStatus('T')); + $bitrix24Account = $this->createActiveAccount($command->memberId, $applicationToken, true); + $applicationInstallation = $this->createActiveInstallation($bitrix24Account->getId(), $applicationToken, new ApplicationStatus('F')); + + $this->applicationInstallationRepository + ->expects($this->once()) + ->method('findByBitrix24AccountMemberId') + ->with($command->memberId) + ->willReturn($applicationInstallation); + + $this->bitrix24AccountRepository + ->expects($this->once()) + ->method('findByMemberId') + ->with($command->memberId, Bitrix24AccountStatus::active, null, null) + ->willReturn([$bitrix24Account]); + + $this->applicationInstallationRepository->expects($this->never())->method('save'); + $this->bitrix24AccountRepository->expects($this->never())->method('save'); + $this->flusher->expects($this->never())->method('flush'); + $this->logger->expects($this->once())->method('warning'); + + $this->handler->handle($command); + + self::assertSame(ApplicationInstallationStatus::active, $applicationInstallation->getStatus()); + self::assertSame('free', $applicationInstallation->getApplicationStatus()->getStatusCode()); + self::assertTrue($applicationInstallation->isApplicationTokenValid($applicationToken)); + self::assertTrue($bitrix24Account->isApplicationTokenValid($applicationToken)); + } + + #[Test] + public function testHandleRepeatedEventWithDifferentTokenIsNoop(): void + { + $storedToken = Uuid::v7()->toRfc4122(); + $command = $this->createCommand(Uuid::v7()->toRfc4122(), new ApplicationStatus('T')); + $bitrix24Account = $this->createActiveAccount($command->memberId, $storedToken, true); + $applicationInstallation = $this->createActiveInstallation($bitrix24Account->getId(), $storedToken, new ApplicationStatus('F')); + + $this->applicationInstallationRepository + ->expects($this->once()) + ->method('findByBitrix24AccountMemberId') + ->with($command->memberId) + ->willReturn($applicationInstallation); + + $this->bitrix24AccountRepository + ->expects($this->once()) + ->method('findByMemberId') + ->with($command->memberId, Bitrix24AccountStatus::active, null, null) + ->willReturn([$bitrix24Account]); + + $this->applicationInstallationRepository->expects($this->never())->method('save'); + $this->bitrix24AccountRepository->expects($this->never())->method('save'); + $this->flusher->expects($this->never())->method('flush'); + $this->logger->expects($this->once())->method('warning'); + + $this->handler->handle($command); + + self::assertTrue($applicationInstallation->isApplicationTokenValid($storedToken)); + self::assertTrue($bitrix24Account->isApplicationTokenValid($storedToken)); + self::assertSame('free', $applicationInstallation->getApplicationStatus()->getStatusCode()); + } + + #[Test] + public function testHandleThrowsWhenPendingInstallationNotFound(): void + { + $command = $this->createCommand(); + + $this->applicationInstallationRepository + ->expects($this->once()) + ->method('findByBitrix24AccountMemberId') + ->with($command->memberId) + ->willReturn(null); + + $this->bitrix24AccountRepository->expects($this->never())->method('findByMemberId'); + $this->flusher->expects($this->never())->method('flush'); + + $this->expectException(ApplicationInstallationNotFoundException::class); + + $this->handler->handle($command); + } + + #[Test] + public function testHandleThrowsWhenPendingInstallationExistsButNewMasterAccountMissing(): void + { + $command = $this->createCommand(); + $applicationInstallation = $this->createInstallation(Uuid::v7()); + + $this->applicationInstallationRepository + ->expects($this->once()) + ->method('findByBitrix24AccountMemberId') + ->with($command->memberId) + ->willReturn($applicationInstallation); + + $this->bitrix24AccountRepository + ->expects($this->once()) + ->method('findByMemberId') + ->with($command->memberId, Bitrix24AccountStatus::new, null, null) + ->willReturn([]); + + $this->applicationInstallationRepository->expects($this->never())->method('save'); + $this->bitrix24AccountRepository->expects($this->never())->method('save'); + $this->flusher->expects($this->never())->method('flush'); + + $this->expectException(Bitrix24AccountNotFoundException::class); + + $this->handler->handle($command); + } + + private function createCommand(?string $applicationToken = null, ?ApplicationStatus $applicationStatus = null): Command + { + return new Command( + memberId: Uuid::v4()->toRfc4122(), + domainUrl: new Domain('example.bitrix24.test'), + applicationToken: $applicationToken ?? Uuid::v7()->toRfc4122(), + applicationStatus: $applicationStatus ?? new ApplicationStatus('T') + ); + } + + private function createAccount(string $memberId, bool $isMaster): Bitrix24Account + { + return new Bitrix24Account( + Uuid::v7(), + 1, + true, + $memberId, + 'example.bitrix24.test', + new AuthToken('access', 'refresh', 3600, time() + 3600), + 1, + new Scope(['crm']), + $isMaster + ); + } + + private function createActiveAccount(string $memberId, string $applicationToken, bool $isMaster): Bitrix24Account + { + $bitrix24Account = $this->createAccount($memberId, $isMaster); + $bitrix24Account->applicationInstalled($applicationToken); + + return $bitrix24Account; + } + + private function createInstallation(Uuid $uuid): ApplicationInstallation + { + return new ApplicationInstallation( + Uuid::v7(), + $uuid, + new ApplicationStatus('F'), + PortalLicenseFamily::free, + 10, + null, + null, + null, + 'lead-1', + 'install' + ); + } + + private function createActiveInstallation( + Uuid $uuid, + string $applicationToken, + ApplicationStatus $applicationStatus + ): ApplicationInstallation { + $applicationInstallation = new ApplicationInstallation( + Uuid::v7(), + $uuid, + $applicationStatus, + PortalLicenseFamily::free, + 10, + null, + null, + null, + 'lead-1', + 'install' + ); + $applicationInstallation->applicationInstalled($applicationToken); + + return $applicationInstallation; + } +} From e5b52d36364f9b49fb33aed7b34428254789837c Mon Sep 17 00:00:00 2001 From: mesilov Date: Sun, 15 Mar 2026 02:57:39 +0600 Subject: [PATCH 8/8] Refactor tests and helpers for `Install` and `OnAppInstall` use cases: introduce in-memory repositories, collecting logger, and spy flusher; replace mocks with real implementations; update unit test logic, assertions, and coverage; standardize handling of pending installations and application tokens. Signed-off-by: mesilov --- ...licationInstallationInMemoryRepository.php | 168 ++++++++++++++++ ...rdingBitrix24AccountInMemoryRepository.php | 25 +++ tests/Helpers/CollectingLogger.php | 40 ++++ tests/Helpers/SpyFlusher.php | 30 +++ .../UseCase/Install/HandlerTest.php | 190 +++++++----------- .../UseCase/OnAppInstall/HandlerTest.php | 182 ++++++++--------- 6 files changed, 423 insertions(+), 212 deletions(-) create mode 100644 tests/Helpers/ApplicationInstallations/RecordingApplicationInstallationInMemoryRepository.php create mode 100644 tests/Helpers/Bitrix24Accounts/RecordingBitrix24AccountInMemoryRepository.php create mode 100644 tests/Helpers/CollectingLogger.php create mode 100644 tests/Helpers/SpyFlusher.php diff --git a/tests/Helpers/ApplicationInstallations/RecordingApplicationInstallationInMemoryRepository.php b/tests/Helpers/ApplicationInstallations/RecordingApplicationInstallationInMemoryRepository.php new file mode 100644 index 00000000..edc9b0be --- /dev/null +++ b/tests/Helpers/ApplicationInstallations/RecordingApplicationInstallationInMemoryRepository.php @@ -0,0 +1,168 @@ + */ + private array $items = []; + + private int $saveCalls = 0; + + public function __construct( + private readonly Bitrix24AccountRepositoryInterface $bitrix24AccountRepository + ) {} + + #[\Override] + public function save(ApplicationInstallationInterface $applicationInstallation): void + { + $this->saveCalls++; + $this->items[$applicationInstallation->getId()->toRfc4122()] = $applicationInstallation; + } + + #[\Override] + public function delete(Uuid $uuid): void + { + $applicationInstallation = $this->getById($uuid); + if (ApplicationInstallationStatus::deleted !== $applicationInstallation->getStatus()) { + throw new InvalidArgumentException( + sprintf( + 'you cannot delete application installation «%s», in status «%s», mark applicatoin installation as «deleted» before', + $applicationInstallation->getId()->toRfc4122(), + $applicationInstallation->getStatus()->name, + ) + ); + } + + unset($this->items[$uuid->toRfc4122()]); + } + + #[\Override] + public function getById(Uuid $uuid): ApplicationInstallationInterface + { + if (!array_key_exists($uuid->toRfc4122(), $this->items)) { + throw new ApplicationInstallationNotFoundException( + sprintf('application installation not found by id «%s» ', $uuid->toRfc4122()) + ); + } + + return $this->items[$uuid->toRfc4122()]; + } + + #[\Override] + public function findByBitrix24AccountId(Uuid $uuid): ?ApplicationInstallationInterface + { + foreach ($this->items as $item) { + if ($item->getBitrix24AccountId()->equals($uuid)) { + return $item; + } + } + + return null; + } + + #[\Override] + public function findByExternalId(string $externalId): array + { + if (trim($externalId) === '') { + throw new InvalidArgumentException('external id cannot be empty string'); + } + + $result = []; + foreach ($this->items as $item) { + if ($item->getExternalId() === $externalId) { + $result[] = $item; + } + } + + return $result; + } + + /** + * Find the current installation for a member id across non-deleted master accounts. + * This supports both pending (`new`) and finished (`active`) install flows. + */ + #[\Override] + public function findByBitrix24AccountMemberId(string $memberId): ?ApplicationInstallationInterface + { + if (trim($memberId) === '') { + throw new InvalidArgumentException('memberId id cannot be empty string'); + } + + $bitrix24Accounts = $this->bitrix24AccountRepository->findByMemberId($memberId); + + $masterAccounts = array_values(array_filter( + $bitrix24Accounts, + static fn (Bitrix24AccountInterface $bitrix24Account): bool => $bitrix24Account->isMasterAccount() + && Bitrix24AccountStatus::deleted !== $bitrix24Account->getStatus() + )); + + usort( + $masterAccounts, + static fn (Bitrix24AccountInterface $left, Bitrix24AccountInterface $right): int + => self::getStatusPriority($left->getStatus()) <=> self::getStatusPriority($right->getStatus()) + ); + + foreach ($masterAccounts as $masterAccount) { + foreach ($this->items as $item) { + if ($item->getBitrix24AccountId()->equals($masterAccount->getId()) + && ApplicationInstallationStatus::deleted !== $item->getStatus() + ) { + return $item; + } + } + } + + return null; + } + + #[\Override] + public function findByApplicationToken(string $applicationToken): ?ApplicationInstallationInterface + { + if (trim($applicationToken) === '') { + throw new InvalidArgumentException('applicationToken id cannot be empty string'); + } + + foreach ($this->items as $item) { + if ($item->isApplicationTokenValid($applicationToken)) { + return $item; + } + } + + return null; + } + + public function getSaveCalls(): int + { + return $this->saveCalls; + } + + private static function getStatusPriority(Bitrix24AccountStatus $bitrix24AccountStatus): int + { + return match ($bitrix24AccountStatus) { + Bitrix24AccountStatus::active => 0, + Bitrix24AccountStatus::new => 1, + default => 2, + }; + } +} diff --git a/tests/Helpers/Bitrix24Accounts/RecordingBitrix24AccountInMemoryRepository.php b/tests/Helpers/Bitrix24Accounts/RecordingBitrix24AccountInMemoryRepository.php new file mode 100644 index 00000000..c5ec0fa6 --- /dev/null +++ b/tests/Helpers/Bitrix24Accounts/RecordingBitrix24AccountInMemoryRepository.php @@ -0,0 +1,25 @@ +saveCalls++; + parent::save($bitrix24Account); + } + + public function getSaveCalls(): int + { + return $this->saveCalls; + } +} diff --git a/tests/Helpers/CollectingLogger.php b/tests/Helpers/CollectingLogger.php new file mode 100644 index 00000000..e987ab7b --- /dev/null +++ b/tests/Helpers/CollectingLogger.php @@ -0,0 +1,40 @@ +}> */ + private array $records = []; + + #[\Override] + public function log($level, Stringable|string $message, array $context = []): void + { + $this->records[] = [ + 'level' => (string) $level, + 'message' => (string) $message, + 'context' => $context, + ]; + } + + public function countByLevel(string $level): int + { + return count($this->recordsByLevel($level)); + } + + /** + * @return list}> + */ + public function recordsByLevel(string $level): array + { + return array_values(array_filter( + $this->records, + static fn (array $record): bool => $record['level'] === $level + )); + } +} diff --git a/tests/Helpers/SpyFlusher.php b/tests/Helpers/SpyFlusher.php new file mode 100644 index 00000000..a2aa187e --- /dev/null +++ b/tests/Helpers/SpyFlusher.php @@ -0,0 +1,30 @@ +> */ + private array $flushCalls = []; + + public function __construct() {} + + #[\Override] + public function flush(AggregateRootEventsEmitterInterface ...$aggregateRootEventsEmitter): void + { + $this->flushCalls[] = array_values($aggregateRootEventsEmitter); + } + + /** + * @return list> + */ + public function getFlushCalls(): array + { + return $this->flushCalls; + } +} diff --git a/tests/Unit/ApplicationInstallations/UseCase/Install/HandlerTest.php b/tests/Unit/ApplicationInstallations/UseCase/Install/HandlerTest.php index 5e05de00..623fc7be 100644 --- a/tests/Unit/ApplicationInstallations/UseCase/Install/HandlerTest.php +++ b/tests/Unit/ApplicationInstallations/UseCase/Install/HandlerTest.php @@ -9,16 +9,16 @@ use Bitrix24\Lib\ApplicationInstallations\UseCase\Install\Handler; use Bitrix24\Lib\Bitrix24Accounts\Entity\Bitrix24Account; use Bitrix24\Lib\Bitrix24Accounts\ValueObjects\Domain; -use Bitrix24\Lib\Services\Flusher; +use Bitrix24\Lib\Tests\Helpers\ApplicationInstallations\RecordingApplicationInstallationInMemoryRepository; +use Bitrix24\Lib\Tests\Helpers\Bitrix24Accounts\RecordingBitrix24AccountInMemoryRepository; +use Bitrix24\Lib\Tests\Helpers\SpyFlusher; use Bitrix24\SDK\Application\ApplicationStatus; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationStatus; -use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Repository\ApplicationInstallationRepositoryInterface; +use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Entity\Bitrix24AccountInterface; use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Entity\Bitrix24AccountStatus; -use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Repository\Bitrix24AccountRepositoryInterface; use Bitrix24\SDK\Application\PortalLicenseFamily; use Bitrix24\SDK\Core\Credentials\AuthToken; use Bitrix24\SDK\Core\Credentials\Scope; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; @@ -31,20 +31,22 @@ #[CoversClass(Handler::class)] class HandlerTest extends TestCase { - private Bitrix24AccountRepositoryInterface&MockObject $bitrix24AccountRepository; + private RecordingBitrix24AccountInMemoryRepository $bitrix24AccountRepository; - private ApplicationInstallationRepositoryInterface&MockObject $applicationInstallationRepository; + private RecordingApplicationInstallationInMemoryRepository $applicationInstallationRepository; - private Flusher&MockObject $flusher; + private SpyFlusher $flusher; private Handler $handler; #[\Override] protected function setUp(): void { - $this->bitrix24AccountRepository = $this->createMock(Bitrix24AccountRepositoryInterface::class); - $this->applicationInstallationRepository = $this->createMock(ApplicationInstallationRepositoryInterface::class); - $this->flusher = $this->createMock(Flusher::class); + $this->bitrix24AccountRepository = new RecordingBitrix24AccountInMemoryRepository(new NullLogger()); + $this->applicationInstallationRepository = new RecordingApplicationInstallationInMemoryRepository( + $this->bitrix24AccountRepository + ); + $this->flusher = new SpyFlusher(); $this->handler = new Handler( $this->bitrix24AccountRepository, @@ -58,42 +60,23 @@ protected function setUp(): void public function testHandleWithoutTokenCreatesPendingEntities(): void { $command = $this->createCommand(); - $savedEntities = []; - - $this->applicationInstallationRepository - ->expects($this->once()) - ->method('findByBitrix24AccountMemberId') - ->with($command->memberId) - ->willReturn(null); - - $this->bitrix24AccountRepository - ->expects($this->once()) - ->method('save') - ->willReturnCallback(static function (Bitrix24Account $bitrix24Account) use (&$savedEntities): void { - $savedEntities['account'] = $bitrix24Account; - }); - - $this->applicationInstallationRepository - ->expects($this->once()) - ->method('save') - ->willReturnCallback(static function (ApplicationInstallation $applicationInstallation) use (&$savedEntities): void { - $savedEntities['installation'] = $applicationInstallation; - }); - - $this->flusher - ->expects($this->once()) - ->method('flush') - ->with( - $this->callback(static fn (ApplicationInstallation $applicationInstallation): bool => $applicationInstallation->getStatus() === ApplicationInstallationStatus::new), - $this->callback(static fn (Bitrix24Account $bitrix24Account): bool => $bitrix24Account->getStatus() === Bitrix24AccountStatus::new) - ); $this->handler->handle($command); - self::assertArrayHasKey('account', $savedEntities); - self::assertArrayHasKey('installation', $savedEntities); - self::assertSame(Bitrix24AccountStatus::new, $savedEntities['account']->getStatus()); - self::assertSame(ApplicationInstallationStatus::new, $savedEntities['installation']->getStatus()); + $bitrix24Accounts = $this->bitrix24AccountRepository->findByMemberId($command->memberId); + self::assertCount(1, $bitrix24Accounts); + + $bitrix24Account = $bitrix24Accounts[0]; + $applicationInstallation = $this->applicationInstallationRepository->findByBitrix24AccountMemberId($command->memberId); + + self::assertNotNull($applicationInstallation); + self::assertSame(Bitrix24AccountStatus::new, $bitrix24Account->getStatus()); + self::assertSame(ApplicationInstallationStatus::new, $applicationInstallation->getStatus()); + self::assertTrue($applicationInstallation->getBitrix24AccountId()->equals($bitrix24Account->getId())); + self::assertSame(1, $this->bitrix24AccountRepository->getSaveCalls()); + self::assertSame(1, $this->applicationInstallationRepository->getSaveCalls()); + self::assertCount(1, $this->flusher->getFlushCalls()); + self::assertSame([$applicationInstallation, $bitrix24Account], $this->flusher->getFlushCalls()[0]); } #[Test] @@ -101,93 +84,68 @@ public function testHandleWithTokenFinishesInstallationInOneStep(): void { $applicationToken = Uuid::v7()->toRfc4122(); $command = $this->createCommand($applicationToken); - $savedEntities = []; - - $this->applicationInstallationRepository - ->expects($this->once()) - ->method('findByBitrix24AccountMemberId') - ->with($command->memberId) - ->willReturn(null); - - $this->bitrix24AccountRepository - ->expects($this->once()) - ->method('save') - ->willReturnCallback(static function (Bitrix24Account $bitrix24Account) use (&$savedEntities): void { - $savedEntities['account'] = $bitrix24Account; - }); - - $this->applicationInstallationRepository - ->expects($this->once()) - ->method('save') - ->willReturnCallback(static function (ApplicationInstallation $applicationInstallation) use (&$savedEntities): void { - $savedEntities['installation'] = $applicationInstallation; - }); - - $this->flusher - ->expects($this->once()) - ->method('flush') - ->with( - $this->callback(static fn(ApplicationInstallation $applicationInstallation): bool => $applicationInstallation->getStatus() === ApplicationInstallationStatus::active - && $applicationInstallation->isApplicationTokenValid($applicationToken)), - $this->callback(static fn(Bitrix24Account $bitrix24Account): bool => $bitrix24Account->getStatus() === Bitrix24AccountStatus::active - && $bitrix24Account->isApplicationTokenValid($applicationToken)) - ); $this->handler->handle($command); - self::assertSame(Bitrix24AccountStatus::active, $savedEntities['account']->getStatus()); - self::assertSame(ApplicationInstallationStatus::active, $savedEntities['installation']->getStatus()); - self::assertTrue($savedEntities['account']->isApplicationTokenValid($applicationToken)); - self::assertTrue($savedEntities['installation']->isApplicationTokenValid($applicationToken)); + $bitrix24Accounts = $this->bitrix24AccountRepository->findByMemberId($command->memberId); + self::assertCount(1, $bitrix24Accounts); + + $bitrix24Account = $bitrix24Accounts[0]; + $applicationInstallation = $this->applicationInstallationRepository->findByBitrix24AccountMemberId($command->memberId); + + self::assertNotNull($applicationInstallation); + self::assertSame(Bitrix24AccountStatus::active, $bitrix24Account->getStatus()); + self::assertSame(ApplicationInstallationStatus::active, $applicationInstallation->getStatus()); + self::assertTrue($bitrix24Account->isApplicationTokenValid($applicationToken)); + self::assertTrue($applicationInstallation->isApplicationTokenValid($applicationToken)); + self::assertSame(1, $this->bitrix24AccountRepository->getSaveCalls()); + self::assertSame(1, $this->applicationInstallationRepository->getSaveCalls()); + self::assertCount(1, $this->flusher->getFlushCalls()); + self::assertSame([$applicationInstallation, $bitrix24Account], $this->flusher->getFlushCalls()[0]); } #[Test] public function testHandleReinstallOverPendingInstallationDeletesOldPairBeforeCreatingNewOne(): void { $command = $this->createCommand(); - $bitrix24Account = $this->createAccount($command->memberId, true); - $applicationInstallation = $this->createInstallation($bitrix24Account->getId()); - $flushCalls = []; - - $this->applicationInstallationRepository - ->expects($this->exactly(2)) - ->method('save') - ->willReturnCallback(static function (ApplicationInstallation $applicationInstallation): void {}); - - $this->bitrix24AccountRepository - ->expects($this->exactly(2)) - ->method('save') - ->willReturnCallback(static function (Bitrix24Account $bitrix24Account): void {}); - - $this->applicationInstallationRepository - ->expects($this->once()) - ->method('findByBitrix24AccountMemberId') - ->with($command->memberId) - ->willReturn($applicationInstallation); - - $this->bitrix24AccountRepository - ->expects($this->once()) - ->method('findByMemberId') - ->with($command->memberId) - ->willReturn([$bitrix24Account]); - - $this->flusher - ->expects($this->exactly(2)) - ->method('flush') - ->willReturnCallback(static function (...$entities) use (&$flushCalls): void { - $flushCalls[] = $entities; - }); + $existingAccount = $this->createAccount($command->memberId, true); + $applicationInstallation = $this->createInstallation($existingAccount->getId()); + + $this->bitrix24AccountRepository->save($existingAccount); + $this->applicationInstallationRepository->save($applicationInstallation); + + $bitrix24AccountSaveCalls = $this->bitrix24AccountRepository->getSaveCalls(); + $applicationInstallationSaveCalls = $this->applicationInstallationRepository->getSaveCalls(); $this->handler->handle($command); - self::assertCount(2, $flushCalls); + $bitrix24Accounts = $this->bitrix24AccountRepository->findByMemberId($command->memberId); + self::assertCount(2, $bitrix24Accounts); + self::assertSame(Bitrix24AccountStatus::deleted, $existingAccount->getStatus()); self::assertSame(ApplicationInstallationStatus::deleted, $applicationInstallation->getStatus()); self::assertSame('reinstall before finish', $applicationInstallation->getComment()); - self::assertSame(Bitrix24AccountStatus::deleted, $bitrix24Account->getStatus()); - self::assertSame($applicationInstallation, $flushCalls[0][0]); - self::assertSame($bitrix24Account, $flushCalls[0][1]); - self::assertSame(ApplicationInstallationStatus::new, $flushCalls[1][0]->getStatus()); - self::assertSame(Bitrix24AccountStatus::new, $flushCalls[1][1]->getStatus()); + + $activeInstallation = $this->applicationInstallationRepository->findByBitrix24AccountMemberId($command->memberId); + self::assertNotNull($activeInstallation); + self::assertSame(ApplicationInstallationStatus::new, $activeInstallation->getStatus()); + + $newBitrix24Account = null; + foreach ($bitrix24Accounts as $bitrix24Account) { + if (Bitrix24AccountStatus::new === $bitrix24Account->getStatus()) { + $newBitrix24Account = $bitrix24Account; + + break; + } + } + + self::assertInstanceOf(Bitrix24AccountInterface::class, $newBitrix24Account); + self::assertSame(Bitrix24AccountStatus::new, $newBitrix24Account->getStatus()); + self::assertTrue($activeInstallation->getBitrix24AccountId()->equals($newBitrix24Account->getId())); + self::assertSame($bitrix24AccountSaveCalls + 2, $this->bitrix24AccountRepository->getSaveCalls()); + self::assertSame($applicationInstallationSaveCalls + 2, $this->applicationInstallationRepository->getSaveCalls()); + self::assertCount(2, $this->flusher->getFlushCalls()); + self::assertSame([$applicationInstallation, $existingAccount], $this->flusher->getFlushCalls()[0]); + self::assertSame([$activeInstallation, $newBitrix24Account], $this->flusher->getFlushCalls()[1]); } private function createCommand(?string $applicationToken = null): Command diff --git a/tests/Unit/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php b/tests/Unit/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php index ae541d83..74d9fbc6 100644 --- a/tests/Unit/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php +++ b/tests/Unit/ApplicationInstallations/UseCase/OnAppInstall/HandlerTest.php @@ -9,22 +9,23 @@ use Bitrix24\Lib\ApplicationInstallations\UseCase\OnAppInstall\Handler; use Bitrix24\Lib\Bitrix24Accounts\Entity\Bitrix24Account; use Bitrix24\Lib\Bitrix24Accounts\ValueObjects\Domain; -use Bitrix24\Lib\Services\Flusher; +use Bitrix24\Lib\Tests\Helpers\ApplicationInstallations\RecordingApplicationInstallationInMemoryRepository; +use Bitrix24\Lib\Tests\Helpers\Bitrix24Accounts\RecordingBitrix24AccountInMemoryRepository; +use Bitrix24\Lib\Tests\Helpers\CollectingLogger; +use Bitrix24\Lib\Tests\Helpers\SpyFlusher; use Bitrix24\SDK\Application\ApplicationStatus; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationStatus; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Exceptions\ApplicationInstallationNotFoundException; -use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Repository\ApplicationInstallationRepositoryInterface; use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Entity\Bitrix24AccountStatus; use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Exceptions\Bitrix24AccountNotFoundException; -use Bitrix24\SDK\Application\Contracts\Bitrix24Accounts\Repository\Bitrix24AccountRepositoryInterface; use Bitrix24\SDK\Application\PortalLicenseFamily; use Bitrix24\SDK\Core\Credentials\AuthToken; use Bitrix24\SDK\Core\Credentials\Scope; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -use Psr\Log\LoggerInterface; +use Psr\Log\LogLevel; +use Psr\Log\NullLogger; use Symfony\Component\Uid\Uuid; /** @@ -33,23 +34,25 @@ #[CoversClass(Handler::class)] class HandlerTest extends TestCase { - private Bitrix24AccountRepositoryInterface&MockObject $bitrix24AccountRepository; + private RecordingBitrix24AccountInMemoryRepository $bitrix24AccountRepository; - private ApplicationInstallationRepositoryInterface&MockObject $applicationInstallationRepository; + private RecordingApplicationInstallationInMemoryRepository $applicationInstallationRepository; - private Flusher&MockObject $flusher; + private SpyFlusher $flusher; - private LoggerInterface&MockObject $logger; + private CollectingLogger $logger; private Handler $handler; #[\Override] protected function setUp(): void { - $this->bitrix24AccountRepository = $this->createMock(Bitrix24AccountRepositoryInterface::class); - $this->applicationInstallationRepository = $this->createMock(ApplicationInstallationRepositoryInterface::class); - $this->flusher = $this->createMock(Flusher::class); - $this->logger = $this->createMock(LoggerInterface::class); + $this->bitrix24AccountRepository = new RecordingBitrix24AccountInMemoryRepository(new NullLogger()); + $this->applicationInstallationRepository = new RecordingApplicationInstallationInMemoryRepository( + $this->bitrix24AccountRepository + ); + $this->flusher = new SpyFlusher(); + $this->logger = new CollectingLogger(); $this->handler = new Handler( $this->bitrix24AccountRepository, @@ -66,32 +69,24 @@ public function testHandleFinishesPendingInstallation(): void $bitrix24Account = $this->createAccount($command->memberId, true); $applicationInstallation = $this->createInstallation($bitrix24Account->getId()); - $this->applicationInstallationRepository - ->expects($this->once()) - ->method('findByBitrix24AccountMemberId') - ->with($command->memberId) - ->willReturn($applicationInstallation); - - $this->bitrix24AccountRepository - ->expects($this->once()) - ->method('findByMemberId') - ->with($command->memberId, Bitrix24AccountStatus::new, null, null) - ->willReturn([$bitrix24Account]); - - $this->applicationInstallationRepository->expects($this->once())->method('save')->with($applicationInstallation); - $this->bitrix24AccountRepository->expects($this->once())->method('save')->with($bitrix24Account); + $this->bitrix24AccountRepository->save($bitrix24Account); + $this->applicationInstallationRepository->save($applicationInstallation); - $this->flusher - ->expects($this->once()) - ->method('flush') - ->with($applicationInstallation, $bitrix24Account); + $bitrix24AccountSaveCalls = $this->bitrix24AccountRepository->getSaveCalls(); + $applicationInstallationSaveCalls = $this->applicationInstallationRepository->getSaveCalls(); $this->handler->handle($command); - self::assertSame(ApplicationInstallationStatus::active, $applicationInstallation->getStatus()); + $storedInstallation = $this->applicationInstallationRepository->findByBitrix24AccountMemberId($command->memberId); + self::assertNotNull($storedInstallation); + self::assertSame(ApplicationInstallationStatus::active, $storedInstallation->getStatus()); self::assertSame(Bitrix24AccountStatus::active, $bitrix24Account->getStatus()); - self::assertTrue($applicationInstallation->isApplicationTokenValid($command->applicationToken)); + self::assertTrue($storedInstallation->isApplicationTokenValid($command->applicationToken)); self::assertTrue($bitrix24Account->isApplicationTokenValid($command->applicationToken)); + self::assertSame($bitrix24AccountSaveCalls + 1, $this->bitrix24AccountRepository->getSaveCalls()); + self::assertSame($applicationInstallationSaveCalls + 1, $this->applicationInstallationRepository->getSaveCalls()); + self::assertCount(1, $this->flusher->getFlushCalls()); + self::assertSame([$storedInstallation, $bitrix24Account], $this->flusher->getFlushCalls()[0]); } #[Test] @@ -100,31 +95,31 @@ public function testHandleDuplicateEventWithSameTokenIsNoop(): void $applicationToken = Uuid::v7()->toRfc4122(); $command = $this->createCommand($applicationToken, new ApplicationStatus('T')); $bitrix24Account = $this->createActiveAccount($command->memberId, $applicationToken, true); - $applicationInstallation = $this->createActiveInstallation($bitrix24Account->getId(), $applicationToken, new ApplicationStatus('F')); - - $this->applicationInstallationRepository - ->expects($this->once()) - ->method('findByBitrix24AccountMemberId') - ->with($command->memberId) - ->willReturn($applicationInstallation); + $applicationInstallation = $this->createActiveInstallation( + $bitrix24Account->getId(), + $applicationToken, + new ApplicationStatus('F') + ); - $this->bitrix24AccountRepository - ->expects($this->once()) - ->method('findByMemberId') - ->with($command->memberId, Bitrix24AccountStatus::active, null, null) - ->willReturn([$bitrix24Account]); + $this->bitrix24AccountRepository->save($bitrix24Account); + $this->applicationInstallationRepository->save($applicationInstallation); - $this->applicationInstallationRepository->expects($this->never())->method('save'); - $this->bitrix24AccountRepository->expects($this->never())->method('save'); - $this->flusher->expects($this->never())->method('flush'); - $this->logger->expects($this->once())->method('warning'); + $bitrix24AccountSaveCalls = $this->bitrix24AccountRepository->getSaveCalls(); + $applicationInstallationSaveCalls = $this->applicationInstallationRepository->getSaveCalls(); $this->handler->handle($command); - self::assertSame(ApplicationInstallationStatus::active, $applicationInstallation->getStatus()); - self::assertSame('free', $applicationInstallation->getApplicationStatus()->getStatusCode()); - self::assertTrue($applicationInstallation->isApplicationTokenValid($applicationToken)); + $storedInstallation = $this->applicationInstallationRepository->findByBitrix24AccountMemberId($command->memberId); + self::assertNotNull($storedInstallation); + self::assertSame(ApplicationInstallationStatus::active, $storedInstallation->getStatus()); + self::assertSame('free', $storedInstallation->getApplicationStatus()->getStatusCode()); + self::assertTrue($storedInstallation->isApplicationTokenValid($applicationToken)); self::assertTrue($bitrix24Account->isApplicationTokenValid($applicationToken)); + self::assertSame($bitrix24AccountSaveCalls, $this->bitrix24AccountRepository->getSaveCalls()); + self::assertSame($applicationInstallationSaveCalls, $this->applicationInstallationRepository->getSaveCalls()); + self::assertCount(0, $this->flusher->getFlushCalls()); + self::assertSame(1, $this->logger->countByLevel(LogLevel::WARNING)); + self::assertTrue($this->logger->recordsByLevel(LogLevel::WARNING)[0]['context']['tokenMatch']); } #[Test] @@ -133,30 +128,31 @@ public function testHandleRepeatedEventWithDifferentTokenIsNoop(): void $storedToken = Uuid::v7()->toRfc4122(); $command = $this->createCommand(Uuid::v7()->toRfc4122(), new ApplicationStatus('T')); $bitrix24Account = $this->createActiveAccount($command->memberId, $storedToken, true); - $applicationInstallation = $this->createActiveInstallation($bitrix24Account->getId(), $storedToken, new ApplicationStatus('F')); - - $this->applicationInstallationRepository - ->expects($this->once()) - ->method('findByBitrix24AccountMemberId') - ->with($command->memberId) - ->willReturn($applicationInstallation); + $applicationInstallation = $this->createActiveInstallation( + $bitrix24Account->getId(), + $storedToken, + new ApplicationStatus('F') + ); - $this->bitrix24AccountRepository - ->expects($this->once()) - ->method('findByMemberId') - ->with($command->memberId, Bitrix24AccountStatus::active, null, null) - ->willReturn([$bitrix24Account]); + $this->bitrix24AccountRepository->save($bitrix24Account); + $this->applicationInstallationRepository->save($applicationInstallation); - $this->applicationInstallationRepository->expects($this->never())->method('save'); - $this->bitrix24AccountRepository->expects($this->never())->method('save'); - $this->flusher->expects($this->never())->method('flush'); - $this->logger->expects($this->once())->method('warning'); + $bitrix24AccountSaveCalls = $this->bitrix24AccountRepository->getSaveCalls(); + $applicationInstallationSaveCalls = $this->applicationInstallationRepository->getSaveCalls(); $this->handler->handle($command); - self::assertTrue($applicationInstallation->isApplicationTokenValid($storedToken)); + $storedInstallation = $this->applicationInstallationRepository->findByBitrix24AccountMemberId($command->memberId); + self::assertNotNull($storedInstallation); + self::assertSame(ApplicationInstallationStatus::active, $storedInstallation->getStatus()); + self::assertSame('free', $storedInstallation->getApplicationStatus()->getStatusCode()); + self::assertTrue($storedInstallation->isApplicationTokenValid($storedToken)); self::assertTrue($bitrix24Account->isApplicationTokenValid($storedToken)); - self::assertSame('free', $applicationInstallation->getApplicationStatus()->getStatusCode()); + self::assertSame($bitrix24AccountSaveCalls, $this->bitrix24AccountRepository->getSaveCalls()); + self::assertSame($applicationInstallationSaveCalls, $this->applicationInstallationRepository->getSaveCalls()); + self::assertCount(0, $this->flusher->getFlushCalls()); + self::assertSame(1, $this->logger->countByLevel(LogLevel::WARNING)); + self::assertFalse($this->logger->recordsByLevel(LogLevel::WARNING)[0]['context']['tokenMatch']); } #[Test] @@ -164,45 +160,39 @@ public function testHandleThrowsWhenPendingInstallationNotFound(): void { $command = $this->createCommand(); - $this->applicationInstallationRepository - ->expects($this->once()) - ->method('findByBitrix24AccountMemberId') - ->with($command->memberId) - ->willReturn(null); - - $this->bitrix24AccountRepository->expects($this->never())->method('findByMemberId'); - $this->flusher->expects($this->never())->method('flush'); - $this->expectException(ApplicationInstallationNotFoundException::class); - $this->handler->handle($command); + try { + $this->handler->handle($command); + } finally { + self::assertCount(0, $this->flusher->getFlushCalls()); + self::assertSame(0, $this->bitrix24AccountRepository->getSaveCalls()); + self::assertSame(0, $this->applicationInstallationRepository->getSaveCalls()); + } } #[Test] public function testHandleThrowsWhenPendingInstallationExistsButNewMasterAccountMissing(): void { $command = $this->createCommand(); - $applicationInstallation = $this->createInstallation(Uuid::v7()); - - $this->applicationInstallationRepository - ->expects($this->once()) - ->method('findByBitrix24AccountMemberId') - ->with($command->memberId) - ->willReturn($applicationInstallation); + $activeBitrix24Account = $this->createActiveAccount($command->memberId, Uuid::v7()->toRfc4122(), true); + $applicationInstallation = $this->createInstallation($activeBitrix24Account->getId()); - $this->bitrix24AccountRepository - ->expects($this->once()) - ->method('findByMemberId') - ->with($command->memberId, Bitrix24AccountStatus::new, null, null) - ->willReturn([]); + $this->bitrix24AccountRepository->save($activeBitrix24Account); + $this->applicationInstallationRepository->save($applicationInstallation); - $this->applicationInstallationRepository->expects($this->never())->method('save'); - $this->bitrix24AccountRepository->expects($this->never())->method('save'); - $this->flusher->expects($this->never())->method('flush'); + $bitrix24AccountSaveCalls = $this->bitrix24AccountRepository->getSaveCalls(); + $applicationInstallationSaveCalls = $this->applicationInstallationRepository->getSaveCalls(); $this->expectException(Bitrix24AccountNotFoundException::class); - $this->handler->handle($command); + try { + $this->handler->handle($command); + } finally { + self::assertCount(0, $this->flusher->getFlushCalls()); + self::assertSame($bitrix24AccountSaveCalls, $this->bitrix24AccountRepository->getSaveCalls()); + self::assertSame($applicationInstallationSaveCalls, $this->applicationInstallationRepository->getSaveCalls()); + } } private function createCommand(?string $applicationToken = null, ?ApplicationStatus $applicationStatus = null): Command