diff --git a/AGENTS.md b/AGENTS.md index e01e9bd..80c4cb5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,7 +28,7 @@ Then inspect the code paths relevant to the task: - [`backend/migrations/`](backend/migrations/): migration files managed by `mpt-tool` - [`backend/tests/`](backend/tests/): backend test suite - [`frontend/src/modules/`](frontend/src/modules/): React plug entry points (one bundle per module). Frontend authoring rules live in the shared [frontend standard](https://github.com/softwareone-platform/mpt-extension-skills/blob/main/standards/extensions-ui-best-practices.md) -- [`frontend/src/shared/`](frontend/src/shared/): shared frontend components, hooks, and model helpers +- [`frontend/src/shared/`](frontend/src/shared/): shared frontend components, hooks, constants, model helpers, and test fixtures - [`frontend/esbuild.config.js`](frontend/esbuild.config.js): frontend build that emits plug bundles into `static/` - [`peripherals/devmock/`](peripherals/devmock): WireMock image and mappings backing `make run-local` - [`make/`](make): canonical commands used by the repository diff --git a/backend/mpt_extension_python_template/routers/api/agreement.py b/backend/mpt_extension_python_template/routers/api/agreement.py index 4a5732d..6f770d6 100644 --- a/backend/mpt_extension_python_template/routers/api/agreement.py +++ b/backend/mpt_extension_python_template/routers/api/agreement.py @@ -1,13 +1,43 @@ +import asyncio from http import HTTPStatus from mpt_api_client.exceptions import MPTHttpError -from mpt_extension_sdk.api import APIResponse, PaginatedResult +from mpt_extension_sdk import TaskTimeouts +from mpt_extension_sdk.api import APIResponse, PaginatedResult, TaskResult from mpt_extension_sdk.api.context import APIContext from mpt_extension_sdk.api.errors import NotFoundError +from mpt_extension_sdk.models import Agreement from mpt_extension_sdk.routing import APIRouter +from mpt_extension_sdk.schemas import BaseSchema +from mpt_extension_sdk.tasks import TaskHandle +from pydantic import Field agreements_router = APIRouter(prefix="/api/v2/agreements") +# The deferred sync reports progress one step per percent while it waits, and 100 % once +# it has synced. The wait is long on purpose: it outlasts the 300-second Delay the SDK +# answers the first delivery with, so the Extension Service delivers the task again while +# the handler is still processing. +PROGRESS_INTERVAL_SECONDS = 4 +FULL_PROGRESS = 100 +# The whole handler must fit in this budget, declared on the route. The platform finalizes +# a task that runs longer, and the SDK keeps 60 of these seconds as its own safety margin +# before that deadline, so the sync itself gets the rest. +SYNC_HEADROOM_SECONDS = 120 +DEFERRED_SYNC_MAX_PROCESSING_SECONDS = ( + PROGRESS_INTERVAL_SECONDS * (FULL_PROGRESS - 1) + SYNC_HEADROOM_SECONDS +) + + +class SyncAgreementPayload(BaseSchema): + """Body of the sync call: a deferrable route cannot take the id in the path. + + The id is a path segment of every Marketplace call the handler makes, and the + API client does not escape it, so the pattern keeps separators out of it. + """ + + agreement_id: str = Field(alias="agreementId", min_length=1, pattern=r"^[A-Za-z0-9-]+$") + @agreements_router.get("/", name="agreements-list") async def get_agreements(ctx: APIContext) -> APIResponse: @@ -32,16 +62,53 @@ async def get_agreement(agreement_id: str, ctx: APIContext) -> APIResponse: return APIResponse.ok(payload=agreement.to_dict()) -@agreements_router.post(path="/{agreement_id}/sync", name="agreements-sync") -async def sync_agreement(agreement_id: str, ctx: APIContext) -> APIResponse: +@agreements_router.post( + path="/sync", + name="agreements-sync", + body_validator=SyncAgreementPayload, + deferrable=True, + timeouts=TaskTimeouts(max_task_processing=DEFERRED_SYNC_MAX_PROCESSING_SECONDS), +) +async def sync_agreement( + payload: SyncAgreementPayload, ctx: APIContext +) -> APIResponse | TaskResult: """Synchronize an agreement view with the current Marketplace data. - Writes `externalIds.vendor` as the vendor, the only agreement key a client - or operations account cannot touch. The value is the one already there, so - the example changes no data. + Writes `externalIds.vendor` as the vendor, the only agreement key a client or + operations account cannot touch. It writes back the value already there, so the + example changes no data. + + The route is deferrable. A plain call syncs and answers with the agreement. + A call with `MPT-Async: true` runs as a platform Task: it sleeps, reporting + progress, syncs at the end and returns a result pointing at the agreement. + The sleep is deliberate — it lasts long enough for the Extension Service to + deliver the same task again while this execution is still running, which is + the redelivery an extension has to tolerate. """ + agreement_id = payload.agreement_id + if ctx.task is None: + agreement = await _sync_vendor_external_id(ctx, agreement_id) + return APIResponse.ok(payload=agreement.to_dict()) + + await ctx.task.log( + "Agreement sync for {{AgreementId}} waits for the redelivery before it runs", + parameters={"AgreementId": agreement_id}, + ) + await _wait_reporting_progress(ctx.task) + await _sync_vendor_external_id(ctx, agreement_id) + await ctx.task.progress(FULL_PROGRESS) + return TaskResult.reference(f"/public/v1/commerce/agreements/{agreement_id}") + + +async def _sync_vendor_external_id(ctx: APIContext, agreement_id: str) -> Agreement: agreement = await ctx.mpt_api_service.agreements.get_by_id(agreement_id) await ctx.vendor_mpt_api_service.agreements.update( agreement_id, {"externalIds": {"vendor": agreement.external_ids.vendor}} ) - return APIResponse.ok(payload=agreement.to_dict()) + return agreement + + +async def _wait_reporting_progress(task: TaskHandle) -> None: + for progress in range(1, FULL_PROGRESS): + await asyncio.sleep(PROGRESS_INTERVAL_SECONDS) # noqa: WPS476 + await task.progress(progress) # noqa: WPS476 diff --git a/backend/tests/routers/conftest.py b/backend/tests/routers/conftest.py index 4a10543..a4ca03e 100644 --- a/backend/tests/routers/conftest.py +++ b/backend/tests/routers/conftest.py @@ -1,4 +1,6 @@ import pytest +from mpt_api_client.models.meta import Pagination +from mpt_extension_sdk.api.context import APIContext, AuthenticatedRequestContext from mpt_extension_sdk.services.mpt_api_service import MPTAPIService from mpt_extension_sdk.services.mpt_api_service.agreement import AgreementService @@ -21,3 +23,27 @@ def mpt_api_service(mpt_api_service_factory): @pytest.fixture def vendor_mpt_api_service(mpt_api_service_factory): return mpt_api_service_factory() + + +@pytest.fixture +def api_context_factory(mocker, mpt_api_service, vendor_mpt_api_service): + def factory(*, task=None, offset=0, limit=10): + ctx = mocker.create_autospec(APIContext, instance=True) + ctx.mpt_api_service = mpt_api_service + ctx.vendor_mpt_api_service = vendor_mpt_api_service + ctx.task = task + ctx.request = mocker.create_autospec(AuthenticatedRequestContext, instance=True) + ctx.request.pagination = Pagination(offset=offset, limit=limit) + return ctx + + return factory + + +@pytest.fixture +def agreement_id(agreement_payload): + return agreement_payload["id"] + + +@pytest.fixture +def vendor_external_id(): + return "ABC-2023-C07-dbeee0b302c0" diff --git a/backend/tests/routers/test_agreements.py b/backend/tests/routers/test_agreements.py index c74159b..dc1e264 100644 --- a/backend/tests/routers/test_agreements.py +++ b/backend/tests/routers/test_agreements.py @@ -2,36 +2,54 @@ import pytest from mpt_api_client.exceptions import MPTAPIError -from mpt_api_client.models.meta import Pagination -from mpt_extension_sdk.api.context import APIContext, AuthenticatedRequestContext +from mpt_extension_sdk.api import TaskResult from mpt_extension_sdk.api.errors import NotFoundError from mpt_extension_sdk.models import Agreement from mpt_extension_sdk.models.external_id import ExternalIds from mpt_extension_sdk.services.mpt_api_service.base import PaginatedCollection +from mpt_extension_sdk.tasks import TaskHandle +from pydantic import ValidationError from mpt_extension_python_template.routers.api.agreement import ( + FULL_PROGRESS, + SyncAgreementPayload, get_agreement, get_agreements, sync_agreement, ) -async def test_get_reads_marketplace(mocker, agreement_payload, mpt_api_service): - agreement = mocker.Mock(spec=Agreement) +@pytest.fixture +def agreement_in_mpt(mocker, agreement_payload, vendor_external_id, mpt_api_service): + agreement = mocker.create_autospec(Agreement, instance=True) agreement.to_dict.return_value = agreement_payload - ctx = mocker.create_autospec(APIContext, instance=True) - ctx.mpt_api_service = mpt_api_service + agreement.external_ids = ExternalIds(vendor=vendor_external_id) mpt_api_service.agreements.get_by_id.return_value = agreement + return agreement - result = await get_agreement("AGR-1234-5678", ctx) - mpt_api_service.agreements.get_by_id.assert_awaited_once_with("AGR-1234-5678") +@pytest.fixture +def deferred_context(mocker, api_context_factory): + return api_context_factory(task=mocker.create_autospec(TaskHandle, instance=True)) + + +@pytest.fixture +def mock_sleep(mocker): + return mocker.patch( + "mpt_extension_python_template.routers.api.agreement.asyncio.sleep", autospec=True + ) + + +async def test_get_reads_marketplace( + api_context_factory, agreement_id, agreement_in_mpt, agreement_payload +): + result = await get_agreement(agreement_id, api_context_factory()) + assert result.payload == agreement_payload -async def test_get_maps_marketplace_not_found(mocker, mpt_api_service): - ctx = mocker.create_autospec(APIContext, instance=True) - ctx.mpt_api_service = mpt_api_service +async def test_get_maps_marketplace_not_found(api_context_factory, mpt_api_service): + ctx = api_context_factory() mpt_api_service.agreements.get_by_id.side_effect = MPTAPIError( HTTPStatus.NOT_FOUND, "Not Found", {"detail": "Entity for given id not-found not found"} ) @@ -40,39 +58,77 @@ async def test_get_maps_marketplace_not_found(mocker, mpt_api_service): await get_agreement("not-found", ctx) +async def test_get_agreements_paginated(api_context_factory, agreement_payload): + page = PaginatedCollection(limit=10, offset=0, resources=[agreement_payload], total=2) + ctx = api_context_factory(offset=0, limit=10) + ctx.mpt_api_service.agreements.get_all.return_value = page + + result = await get_agreements(ctx) + + ctx.mpt_api_service.agreements.get_all.assert_awaited_once_with(offset=0, limit=10) + assert result.payload == [agreement_payload] + assert result.paginated_result.total == 2 + assert result.paginated_result.offset == 0 + assert result.paginated_result.limit == 10 + + +async def test_sync_answers_with_the_agreement( + api_context_factory, agreement_id, agreement_in_mpt, agreement_payload +): + result = await sync_agreement( + SyncAgreementPayload(agreement_id=agreement_id), api_context_factory() + ) + + assert result.payload == agreement_payload + + async def test_sync_writes_back_as_the_vendor( - mocker, agreement_payload, mpt_api_service, vendor_mpt_api_service + api_context_factory, agreement_id, agreement_in_mpt, vendor_external_id ): - agreement = mocker.Mock(spec=Agreement) - agreement.to_dict.return_value = agreement_payload - agreement.external_ids = ExternalIds(vendor="ABC-2023-C07-dbeee0b302c0") - ctx = mocker.create_autospec(APIContext, instance=True) - ctx.mpt_api_service = mpt_api_service - ctx.vendor_mpt_api_service = vendor_mpt_api_service - mpt_api_service.agreements.get_by_id.return_value = agreement + ctx = api_context_factory() - result = await sync_agreement("AGR-1234-5678", ctx) + await sync_agreement(SyncAgreementPayload(agreement_id=agreement_id), ctx) # act - mpt_api_service.agreements.get_by_id.assert_awaited_once_with("AGR-1234-5678") - vendor_mpt_api_service.agreements.update.assert_awaited_once_with( - "AGR-1234-5678", {"externalIds": {"vendor": "ABC-2023-C07-dbeee0b302c0"}} + ctx.vendor_mpt_api_service.agreements.update.assert_awaited_once_with( + agreement_id, {"externalIds": {"vendor": vendor_external_id}} ) - mpt_api_service.agreements.update.assert_not_awaited() - assert result.payload == agreement_payload + ctx.mpt_api_service.agreements.update.assert_not_awaited() -async def test_get_agreements_paginated(mocker, agreement_payload, mpt_api_service): - page = mocker.Mock(spec=PaginatedCollection, resources=[agreement_payload], total=1) - ctx = mocker.create_autospec(APIContext, instance=True) - ctx.request = mocker.create_autospec(AuthenticatedRequestContext, instance=True) - ctx.request.pagination = mocker.Mock(spec=Pagination, offset=0, limit=10) - ctx.mpt_api_service = mpt_api_service - mpt_api_service.agreements.get_all.return_value = page +async def test_sync_deferred_returns_task_result( + deferred_context, agreement_id, agreement_in_mpt, mock_sleep +): + result = await sync_agreement(SyncAgreementPayload(agreement_id=agreement_id), deferred_context) - result = await get_agreements(ctx) + assert result == TaskResult.reference(f"/public/v1/commerce/agreements/{agreement_id}") - mpt_api_service.agreements.get_all.assert_awaited_once_with(offset=0, limit=10) - assert result.payload == [agreement_payload] - assert result.paginated_result.total == 1 - assert result.paginated_result.offset == 0 - assert result.paginated_result.limit == 10 + +async def test_sync_deferred_logs_the_wait( + deferred_context, agreement_id, agreement_in_mpt, mock_sleep +): + await sync_agreement( # act + SyncAgreementPayload(agreement_id=agreement_id), deferred_context + ) + + deferred_context.task.log.assert_awaited_once_with( + "Agreement sync for {{AgreementId}} waits for the redelivery before it runs", + parameters={"AgreementId": agreement_id}, + ) + + +async def test_sync_deferred_reports_progress( + deferred_context, agreement_id, agreement_in_mpt, mock_sleep +): + await sync_agreement( # act + SyncAgreementPayload(agreement_id=agreement_id), deferred_context + ) + + reported = [call.args[0] for call in deferred_context.task.progress.await_args_list] + assert reported == list(range(1, FULL_PROGRESS + 1)) + assert mock_sleep.await_count == FULL_PROGRESS - 1 + + +@pytest.mark.parametrize("unsafe_id", ["", "AGR-1/../other", "AGR 1", "AGR-1/sync"]) +def test_sync_payload_rejects_an_unsafe_id(unsafe_id): + with pytest.raises(ValidationError): + SyncAgreementPayload(agreement_id=unsafe_id) diff --git a/backend/tests/test_app.py b/backend/tests/test_app.py index 989b655..5d7f24a 100644 --- a/backend/tests/test_app.py +++ b/backend/tests/test_app.py @@ -24,7 +24,9 @@ def test_app_registers_agreement_event_route(): def test_app_registers_sync_route(): result = {route.path: route for route in ext_app.routes} - assert isinstance(result["/api/v2/agreements/{agreement_id}/sync"], APIRouteDefinition) + sync_route = result["/api/v2/agreements/sync"] + assert isinstance(sync_route, APIRouteDefinition) + assert sync_route.deferrable is True def test_app_registers_get_route(): diff --git a/docs/architecture.md b/docs/architecture.md index 2dd1613..a52db82 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -26,7 +26,7 @@ repository; for the SDK APIs themselves, see the SDK references in The extension currently registers: -- agreement API routes under `/api/v2/agreements` +- agreement API routes under `/api/v2/agreements`, with `POST /api/v2/agreements/sync` published in the `deferrables` metadata block - order event route `/events/v2/orders/purchase` - agreement event route `/events/v2/agreements/complete` - agreement schedule route `/schedules/v1/agreements/sync` @@ -61,7 +61,9 @@ This section only covers what is specific to *this* repository: `components/` (presentational UI, including `AddPlugShowcase` reused by the `add-*` modules), `hooks/` (for example `useAgreement`, which fetches `/api/v2/agreements/{id}` through the SDK `http` client and exposes - load/error/ready states), and `model.ts` (response types and formatters). + load/error/ready states), `model.ts` (response types and formatters), and + `constants.ts` (paths, intervals and other configuration literals, kept out of + the modules that use them). - **Build output**: the build emits bundles plus sourcemaps, and TypeScript declarations under `static/types/`. diff --git a/docs/examples.md b/docs/examples.md index 79cc1ea..e5e2b8d 100644 --- a/docs/examples.md +++ b/docs/examples.md @@ -43,11 +43,50 @@ exposes an `APIRouter` under `/api/v2/agreements`: | --- | --- | --- | | `GET /` | `agreements-list` | Paginated reads with `ctx.request.pagination` and `PaginatedResult` / `APIResponse.paginated` | | `GET /{agreement_id}` | `agreements-get` | Reading a single agreement via `ctx.mpt_api_service`, returning `APIResponse.ok`, and turning a Marketplace 404 into an SDK `NotFoundError` so the route answers 404 | -| `POST /{agreement_id}/sync` | `agreements-sync` | A read as the calling account followed by a write back as the extension vendor (see [Two Account Identities](#two-account-identities)) | +| `POST /sync` | `agreements-sync` | A deferrable route with a `body_validator` (`{"agreementId": ...}`): a read as the calling account followed by a write back as the extension vendor (see [Two Account Identities](#two-account-identities)), served as a direct call or as a platform Task (see [Deferrable Sync](#deferrable-sync)) | These handlers read Marketplace data through `ctx.mpt_api_service.agreements`. -The `sync` route is the backend half of the frontend "Sync now" example (see -[Plugs](#plugs)). +The `sync` route is the backend half of the frontend "Sync now" and "Sync in +background" example (see [Plugs](#plugs)). + +## Deferrable Sync + +`agreements-sync` is declared with `deferrable=True`, so the SDK publishes it in +the `deferrables` block of `meta.yaml` and one handler serves two call modes: + +- **Direct call** — a plain `POST`. `ctx.task` is `None`, and the handler answers + with the agreement through `APIResponse.ok`. +- **Deferred call** — the same `POST` with the `MPT-Async: true` header. The + Extension Service answers `202 Accepted` with `MPT-Task-Id` itself, then delivers + the request to the extension as a Task. `ctx.task` is set, and the handler + returns `TaskResult.reference(...)` pointing at the agreement, which the SDK + stores as the Task result before completing it. + +The deferred branch sleeps before it syncs, reporting progress 1 % … 99 % every +`PROGRESS_INTERVAL_SECONDS` (4 s) and 100 % once the sync is done. The sleep is +deliberate: it outlasts the `Delay` of at least 300 s the SDK answers the first +delivery with, so the Extension Service delivers the task again while the handler is +still running — the redelivery an extension has to tolerate. The SDK answers that +second delivery with another `Delay` instead of running the handler twice. The route +declares a `max_task_processing` budget covering the sleep plus two minutes, one of +which the SDK keeps as its own safety margin. + +The plug follows the Task on the Marketplace API: after "Sync in background" it polls +`GET /public/v1/system/tasks/{id}` every 5 seconds with the caller's token — the account +that triggered the call is in the Task's `access` — and keeps both buttons disabled until +the Task is final. After `MAX_POLL_FAILURES` failed reads in a row it stops following and +says so, while the Task keeps running. + +The plug follows a Task only while it is on screen. It does not survive a page reload, and +another browser or device does not see the running sync either: the Tasks API cannot be +asked for "the open task of this agreement", so there is nothing to resume from. An +extension that needs that has to keep the task id itself. + +A deferrable route cannot declare path parameters, because the platform does not +resolve them for a deferred call, so the agreement id travels in the request +body. See the SDK +[deferrable endpoints guide](https://github.com/softwareone-platform/mpt-extension-sdk/blob/main/docs/sdk_usage/api.md#deferrable-endpoints) +for the full contract. ## Two Account Identities @@ -57,8 +96,8 @@ authenticated as the account in the incoming request token, and extension. Use the first by default; reach for the second only when a flow has to act as the extension owner regardless of who triggered it. -The `agreements-sync` route shows both in one handler — it reads as the caller, -then writes back as the vendor: +The `agreements-sync` route shows both in one step, shared by its two call modes — +it reads as the caller, then writes back as the vendor: ```python agreement = await ctx.mpt_api_service.agreements.get_by_id(agreement_id) @@ -187,7 +226,7 @@ module name matches the bundle path one-to-one: | Plug id / socket | Frontend module | Demonstrates | | --- | --- | --- | -| `agreements-agreement-` — `portal.commerce.agreements.agreement` | [`modules/agreements-agreement/`](../frontend/src/modules/agreements-agreement/) | A full agreement tab: sync status, `useAgreementSync`, status chips, and a "Sync now" action calling the `sync` API | +| `agreements-agreement-` — `portal.commerce.agreements.agreement` | [`modules/agreements-agreement/`](../frontend/src/modules/agreements-agreement/) | A full agreement tab: sync status, `useAgreementSync`, status chips, a "Sync now" action calling the `sync` API directly, and a "Sync in background" action sending the same call with `MPT-Async: true` and following the returned task, with its progress, until it is final | ## Modal Plugs (Opened By Id) diff --git a/frontend/jest.config.cjs b/frontend/jest.config.cjs index 190e420..6896a3a 100644 --- a/frontend/jest.config.cjs +++ b/frontend/jest.config.cjs @@ -7,6 +7,12 @@ module.exports = { '\\.scss$': '/__mocks__/styleMock.js', }, transform: { - '^.+\\.(ts|tsx)$': ['ts-jest', { tsconfig: { esModuleInterop: true } }], + // ts-jest compiles each file with its own program and drops the automatic `@types/*` + // inclusion, so whichever suite jest compiles first fails with "Cannot find name + // 'jest'". Declaring the test globals here makes every suite compile in any order. + '^.+\\.(ts|tsx)$': [ + 'ts-jest', + { tsconfig: { esModuleInterop: true, types: ['jest', '@testing-library/jest-dom'] } }, + ], }, }; diff --git a/frontend/src/modules/agreements-agreement/App.test.tsx b/frontend/src/modules/agreements-agreement/App.test.tsx index 37c76bf..eb72554 100644 --- a/frontend/src/modules/agreements-agreement/App.test.tsx +++ b/frontend/src/modules/agreements-agreement/App.test.tsx @@ -1,13 +1,20 @@ -import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; import '@testing-library/jest-dom'; import { http } from '@mpt-extension/sdk'; import { useMPTContext } from '@mpt-extension/sdk-react'; +import { + MOCK_AGREEMENT_ID, + MOCK_TASK_ID, + mockDeferredAccepted, + mockTaskResponse, +} from '../../shared/test-utils/agreement-mocks'; import App from './App'; jest.mock('@mpt-extension/sdk', () => ({ http: { + get: jest.fn(), post: jest.fn(), }, }), { virtual: true }); @@ -84,6 +91,7 @@ jest.mock('@softwareone-platform/sdk-react-ui-v0/utils', () => { }; }); +const mockGet = jest.mocked(http.get); const mockPost = jest.mocked(http.post); const mockUseMPTContext = jest.mocked(useMPTContext); @@ -93,29 +101,58 @@ describe('agreement plug app', () => { }); it('marks the sync as successful after a successful request', async () => { - mockUseMPTContext.mockReturnValue({ data: { agreement: { id: 'AGR-1234-5678-9012' } } }); + mockUseMPTContext.mockReturnValue({ data: { agreement: { id: MOCK_AGREEMENT_ID } } }); mockPost.mockResolvedValue({ data: { data: {} } }); render(); fireEvent.click(screen.getByRole('button', { name: 'Sync now' })); - await waitFor(() => { - expect(mockPost).toHaveBeenCalledWith('/api/v2/agreements/AGR-1234-5678-9012/sync'); - }); expect(await screen.findAllByText('Success')).not.toHaveLength(0); }); + it('requests a background sync with MPT-Async and shows the task id', async () => { + mockUseMPTContext.mockReturnValue({ data: { agreement: { id: MOCK_AGREEMENT_ID } } }); + mockPost.mockResolvedValue(mockDeferredAccepted()); + mockGet.mockResolvedValue(mockTaskResponse('Processing', 40)); + + render(); + fireEvent.click(screen.getByRole('button', { name: 'Sync in background' })); + + expect( + await screen.findByText('Sync requested. Marketplace runs it in the background.'), + ).toBeInTheDocument(); + expect(await screen.findByText(`${MOCK_TASK_ID} (processing)`)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Sync in background' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Sync now' })).toBeDisabled(); + expect(await screen.findByText('Task progress')).toBeInTheDocument(); + expect(screen.getByText('40%')).toBeInTheDocument(); + }); + + it('offers the background sync again once the task is completed', async () => { + mockUseMPTContext.mockReturnValue({ data: { agreement: { id: MOCK_AGREEMENT_ID } } }); + mockPost.mockResolvedValue(mockDeferredAccepted()); + mockGet.mockResolvedValue(mockTaskResponse('Completed')); + + render(); + fireEvent.click(screen.getByRole('button', { name: 'Sync in background' })); + + expect(await screen.findByText(`${MOCK_TASK_ID} (completed)`)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Sync in background' })).toBeEnabled(); + expect(screen.queryByText('Task progress')).not.toBeInTheDocument(); + }); + it('disables synchronization when the agreement context is missing', () => { mockUseMPTContext.mockReturnValue({}); render(); expect(screen.getByRole('button', { name: 'Sync now' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Sync in background' })).toBeDisabled(); expect(screen.getByText('Agreement context was not provided by Marketplace.')).toBeInTheDocument(); }); it('renders synchronization errors', async () => { - mockUseMPTContext.mockReturnValue({ data: { agreement: { id: 'AGR-1234-5678-9012' } } }); + mockUseMPTContext.mockReturnValue({ data: { agreement: { id: MOCK_AGREEMENT_ID } } }); mockPost.mockRejectedValue(new Error('Marketplace unavailable')); render(); diff --git a/frontend/src/modules/agreements-agreement/App.tsx b/frontend/src/modules/agreements-agreement/App.tsx index a14cc30..04bc5e2 100644 --- a/frontend/src/modules/agreements-agreement/App.tsx +++ b/frontend/src/modules/agreements-agreement/App.tsx @@ -13,6 +13,7 @@ import { ExtensionNavigation } from '../../shared/components/ExtensionNavigation const STATUS_LABEL: Record = { idle: 'Idle', loading: 'Synchronising', + requested: 'Requested', success: 'Success', error: 'Error', }; @@ -20,13 +21,28 @@ const STATUS_LABEL: Record = { const STATUS_COLOR: Record = { idle: 'gray', loading: 'primary', + requested: 'primary', success: 'success', error: 'danger', }; export default function App() { const agreementId = useAgreementId(); - const { error, lastCompleted, lastStatus, status, syncAgreement } = useAgreementSync(agreementId); + const { + error, + isTaskRunning, + lastCompleted, + lastStatus, + mode, + requestAgreementSync, + status, + syncAgreement, + taskId, + taskProgress, + taskStatus, + } = useAgreementSync(agreementId); + const isLoading = status === 'loading'; + const isSyncDisabled = !agreementId || isLoading || isTaskRunning; return ( Now + {taskId && ( + + {taskStatus ? `${taskId} (${taskStatus})` : taskId} + + )} + {taskProgress !== null && ( + + {`${Math.round(taskProgress)}%`} + + )} @@ -95,17 +121,30 @@ export default function App() { Synchronise now - To request a sync, click the “Sync now” button below. + “Sync now” waits for the sync to finish. “Sync in background” + sends the same call with MPT-Async: true, so Marketplace runs it as a + task and answers right away; it stays disabled until that task is complete. - +
+ + +
{status === 'error' && ( @@ -113,6 +152,11 @@ export default function App() { {error} )} + {status === 'requested' && ( + + Sync requested. Marketplace runs it in the background. + + )} {status === 'success' && ( This is an example. The agreement was not modified. diff --git a/frontend/src/shared/constants.ts b/frontend/src/shared/constants.ts new file mode 100644 index 0000000..6aaae9c --- /dev/null +++ b/frontend/src/shared/constants.ts @@ -0,0 +1,10 @@ +// Configuration literals shared by the agreement sync hook and its tests. + +export const SYNC_PATH = '/api/v2/agreements/sync'; +// The plug reads the Task on the Marketplace API: `http` sends a platform path straight +// there, and the account that triggered the deferred call can read the Task it owns. +export const TASKS_PATH = '/public/v1/system/tasks'; +export const TASK_POLL_INTERVAL_MS = 5000; +// A read can fail without the Task being in trouble, so it is retried; after this many +// failures in a row the plug stops following it. +export const MAX_POLL_FAILURES = 3; diff --git a/frontend/src/shared/hooks/useAgreementSync.test.ts b/frontend/src/shared/hooks/useAgreementSync.test.ts index 681ef02..c6e9197 100644 --- a/frontend/src/shared/hooks/useAgreementSync.test.ts +++ b/frontend/src/shared/hooks/useAgreementSync.test.ts @@ -2,37 +2,187 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import { http } from '@mpt-extension/sdk'; +import { MAX_POLL_FAILURES, TASKS_PATH, TASK_POLL_INTERVAL_MS } from '../constants'; +import { + MOCK_AGREEMENT_ID, + MOCK_TASK_ID, + mockDeferredAccepted, + mockTaskResponse, +} from '../test-utils/agreement-mocks'; import { useAgreementSync } from './useAgreementSync'; jest.mock('@mpt-extension/sdk', () => ({ http: { + get: jest.fn(), post: jest.fn(), }, }), { virtual: true }); +const mockGet = jest.mocked(http.get); const mockPost = jest.mocked(http.post); +const TASK_PATH = `${TASKS_PATH}/${MOCK_TASK_ID}`; + describe('useAgreementSync', () => { + const requestBackgroundSync = async () => { + mockPost.mockResolvedValue(mockDeferredAccepted()); + const hook = renderHook(() => useAgreementSync(MOCK_AGREEMENT_ID)); + await act(async () => { + await hook.result.current.requestAgreementSync(); + }); + return hook; + }; + beforeEach(() => { jest.clearAllMocks(); }); - it('posts the encoded agreement id and records successful completion', async () => { + afterEach(() => { + jest.restoreAllMocks(); + jest.useRealTimers(); + }); + + it('posts the agreement id and records successful completion', async () => { mockPost.mockResolvedValue({ data: { data: {} } }); - const { result } = renderHook(() => useAgreementSync('AGR-1234-5678-9012')); + const { result } = renderHook(() => useAgreementSync(MOCK_AGREEMENT_ID)); await act(async () => { await result.current.syncAgreement(); }); - expect(mockPost).toHaveBeenCalledWith('/api/v2/agreements/AGR-1234-5678-9012/sync'); + expect(mockPost).toHaveBeenCalledWith('/api/v2/agreements/sync', { agreementId: MOCK_AGREEMENT_ID }); await waitFor(() => expect(result.current.status).toBe('success')); expect(result.current.lastStatus).toBe('success'); expect(result.current.lastCompleted).toEqual(expect.any(String)); expect(result.current.error).toBe(''); }); + it('requests a deferred sync with MPT-Async and follows the running task', async () => { + mockGet.mockResolvedValue(mockTaskResponse('Processing', 40)); + + const { result } = await requestBackgroundSync(); + + expect(mockPost).toHaveBeenCalledWith( + '/api/v2/agreements/sync', + { agreementId: MOCK_AGREEMENT_ID }, + { headers: { 'MPT-Async': 'true' } }, + ); + await waitFor(() => expect(result.current.taskStatus).toBe('processing')); + expect(mockGet).toHaveBeenCalledWith(TASK_PATH); + expect(result.current.status).toBe('requested'); + expect(result.current.taskId).toBe(MOCK_TASK_ID); + expect(result.current.taskProgress).toBe(40); + expect(result.current.isTaskRunning).toBe(true); + expect(result.current.lastCompleted).toBeNull(); + }); + + it('polls the task until it is completed', async () => { + jest.useFakeTimers(); + mockGet + .mockResolvedValueOnce(mockTaskResponse('Processing')) + .mockResolvedValueOnce(mockTaskResponse('Completed')); + + const { result } = await requestBackgroundSync(); + await waitFor(() => expect(result.current.taskStatus).toBe('processing')); + + await act(async () => { + jest.advanceTimersByTime(TASK_POLL_INTERVAL_MS); + }); + + await waitFor(() => expect(result.current.status).toBe('success')); + expect(mockGet.mock.calls.filter(([url]) => url === TASK_PATH)).toHaveLength(2); + expect(result.current.isTaskRunning).toBe(false); + expect(result.current.lastStatus).toBe('success'); + expect(result.current.lastCompleted).toEqual(expect.any(String)); + }); + + it('records a failed background task', async () => { + mockGet.mockResolvedValue(mockTaskResponse('Failed')); + + const { result } = await requestBackgroundSync(); + + await waitFor(() => expect(result.current.status).toBe('error')); + expect(result.current.error).toBe(`Background sync task ${MOCK_TASK_ID} failed.`); + }); + + it('stops following the task after MAX_POLL_FAILURES unreadable reads', async () => { + jest.useFakeTimers(); + mockGet.mockRejectedValue(new Error('Gateway timeout')); + + const { result } = await requestBackgroundSync(); + + for (let attempt = 1; attempt < MAX_POLL_FAILURES; attempt += 1) { + await act(async () => { + jest.advanceTimersByTime(TASK_POLL_INTERVAL_MS); + }); + } + + await waitFor(() => expect(result.current.status).toBe('error')); + expect(mockGet).toHaveBeenCalledTimes(MAX_POLL_FAILURES); + expect(result.current.error).toBe( + `Background sync task ${MOCK_TASK_ID} could not be read.`, + ); + }); + + it('drops the tracked task when the portal switches agreement', async () => { + mockPost.mockResolvedValue(mockDeferredAccepted()); + mockGet.mockResolvedValue(mockTaskResponse('Processing')); + const { result, rerender } = renderHook(({ id }) => useAgreementSync(id), { + initialProps: { id: MOCK_AGREEMENT_ID }, + }); + await act(async () => { + await result.current.requestAgreementSync(); + }); + await waitFor(() => expect(result.current.taskStatus).toBe('processing')); + + rerender({ id: 'AGR-9999-9999-9999' }); + + await waitFor(() => expect(result.current.status).toBe('idle')); + expect(result.current.taskId).toBeNull(); + }); + + it('ignores a sync that answers after the agreement changed', async () => { + let resolvePost: ((value: unknown) => void) | undefined; + mockPost.mockImplementation( + () => + new Promise((resolve) => { + resolvePost = resolve; + }), + ); + mockGet.mockResolvedValue(mockTaskResponse('Processing')); + const { result, rerender } = renderHook(({ id }) => useAgreementSync(id), { + initialProps: { id: MOCK_AGREEMENT_ID }, + }); + act(() => { + void result.current.requestAgreementSync(); + }); + rerender({ id: 'AGR-9999-9999-9999' }); + + await act(async () => { + resolvePost?.({ headers: { 'mpt-task-id': MOCK_TASK_ID } }); + }); + + expect(result.current.status).toBe('idle'); + expect(result.current.taskId).toBeNull(); + expect(mockGet).not.toHaveBeenCalled(); + }); + + it('fails a background sync accepted without a task id', async () => { + mockPost.mockResolvedValue({ headers: {} }); + + const { result } = renderHook(() => useAgreementSync(MOCK_AGREEMENT_ID)); + await act(async () => { + await result.current.requestAgreementSync(); + }); + + expect(result.current.status).toBe('error'); + expect(result.current.error).toBe( + 'Marketplace accepted the sync without returning a task id.', + ); + expect(mockGet).not.toHaveBeenCalled(); + }); + it('does not post when agreement id is missing', async () => { const { result } = renderHook(() => useAgreementSync('')); @@ -47,7 +197,7 @@ describe('useAgreementSync', () => { it('records sync failures', async () => { mockPost.mockRejectedValue(new Error('Marketplace unavailable')); - const { result } = renderHook(() => useAgreementSync('AGR-1234-5678-9012')); + const { result } = renderHook(() => useAgreementSync(MOCK_AGREEMENT_ID)); await act(async () => { await result.current.syncAgreement(); diff --git a/frontend/src/shared/hooks/useAgreementSync.ts b/frontend/src/shared/hooks/useAgreementSync.ts index e4dd0e8..c49a149 100644 --- a/frontend/src/shared/hooks/useAgreementSync.ts +++ b/frontend/src/shared/hooks/useAgreementSync.ts @@ -1,52 +1,203 @@ -import { useCallback, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { http } from '@mpt-extension/sdk'; -export type Status = 'idle' | 'loading' | 'success' | 'error'; +import { MAX_POLL_FAILURES, SYNC_PATH, TASKS_PATH, TASK_POLL_INTERVAL_MS } from '../constants'; + +export type Status = 'idle' | 'loading' | 'requested' | 'success' | 'error'; + +export type SyncMode = 'direct' | 'deferred'; + +interface TaskPayload { + progress?: number | null; + status?: string; +} interface SyncState { error: string; lastCompleted: string | null; lastStatus: Status | null; + mode: SyncMode | null; status: Status; + taskId: string | null; + taskProgress: number | null; + taskStatus: string | null; } const INITIAL_SYNC_STATE: SyncState = { error: '', lastCompleted: null, lastStatus: null, + mode: null, status: 'idle', + taskId: null, + taskProgress: null, + taskStatus: null, }; +function isFinal(taskStatus: string): boolean { + return taskStatus === 'completed' || taskStatus === 'failed'; +} + +function applyTaskUpdate( + current: SyncState, + taskStatus: string, + taskProgress: number | null, +): SyncState { + const followed = { ...current, taskProgress, taskStatus }; + if (!isFinal(taskStatus)) { + return followed; + } + + const isFailed = taskStatus === 'failed'; + return { + ...followed, + error: isFailed ? `Background sync task ${current.taskId} failed.` : '', + lastCompleted: new Date().toLocaleString(), + lastStatus: isFailed ? 'error' : 'success', + status: isFailed ? 'error' : 'success', + }; +} + export function useAgreementSync(agreementId: string) { const [state, setState] = useState(INITIAL_SYNC_STATE); + const { status, taskId } = state; + // The agreement the hook follows right now, for calls that started before a switch. + const followedAgreement = useRef(agreementId); + + // The state belongs to one agreement: `useMPTContext` can hand the plug a different + // one without remounting, and a task followed for the previous agreement must not + // carry over. The plug follows a task only for as long as it is on screen. + useEffect(() => { + followedAgreement.current = agreementId; + setState(INITIAL_SYNC_STATE); + }, [agreementId]); - const syncAgreement = useCallback(async () => { + // Follow a background sync until its Task is final. + useEffect(() => { + if (status !== 'requested' || !taskId) { + return undefined; + } + + let active = true; + let failures = 0; + let timer: ReturnType | undefined; + + const poll = async () => { + try { + const response = await http.get( + `${TASKS_PATH}/${encodeURIComponent(taskId)}`, + ); + if (!active) { + return; + } + failures = 0; + const taskStatus = response.data.status?.toLowerCase() ?? null; + const taskProgress = response.data.progress ?? null; + if (taskStatus) { + setState((current) => applyTaskUpdate(current, taskStatus, taskProgress)); + if (isFinal(taskStatus)) { + return; + } + } + } catch { + // A read that lands after the effect was cleaned up belongs to an agreement the + // plug no longer follows, so it must not touch the state. + if (!active) { + return; + } + failures += 1; + if (failures >= MAX_POLL_FAILURES) { + // The Task keeps running; this browser just stopped being able to read it. + setState((current) => ({ + ...current, + error: `Background sync task ${taskId} could not be read.`, + lastStatus: 'error', + status: 'error', + })); + return; + } + } + if (active) { + timer = setTimeout(() => void poll(), TASK_POLL_INTERVAL_MS); + } + }; + + void poll(); + + return () => { + active = false; + clearTimeout(timer); + }; + }, [agreementId, status, taskId]); + + const runSync = useCallback(async (mode: SyncMode) => { if (!agreementId) { return; } - setState((current) => ({ ...current, error: '', status: 'loading' })); + setState((current) => ({ + ...current, + error: '', + mode, + status: 'loading', + taskId: null, + taskProgress: null, + taskStatus: null, + })); + + // The context can hand the plug another agreement while the call is in flight; its + // answer then belongs to an agreement the hook no longer follows. + const stillFollowed = () => followedAgreement.current === agreementId; try { - const encodedAgreementId = encodeURIComponent(agreementId); - await http.post(`/api/v2/agreements/${encodedAgreementId}/sync`); - setState({ - error: '', - lastCompleted: new Date().toLocaleString(), - lastStatus: 'success', - status: 'success', - }); + const body = { agreementId }; + if (mode === 'direct') { + await http.post(SYNC_PATH, body); + if (!stillFollowed()) { + return; + } + setState((current) => ({ + ...current, + lastCompleted: new Date().toLocaleString(), + lastStatus: 'success', + status: 'success', + })); + return; + } + + // The Extension Service answers 202 with the id of the Task that runs the sync. + const response = await http.post(SYNC_PATH, body, { headers: { 'MPT-Async': 'true' } }); + const requestedTaskId: string | undefined = response.headers?.['mpt-task-id']; + if (!requestedTaskId) { + throw new Error('Marketplace accepted the sync without returning a task id.'); + } + if (!stillFollowed()) { + return; + } + setState((current) => ({ ...current, status: 'requested', taskId: requestedTaskId })); } catch (syncError) { + if (!stillFollowed()) { + return; + } const error = syncError instanceof Error ? syncError.message : 'Agreement sync failed.'; - setState({ + setState((current) => ({ + ...current, error, lastCompleted: new Date().toLocaleString(), lastStatus: 'error', status: 'error', - }); + })); } }, [agreementId]); - return { ...state, syncAgreement }; + const requestAgreementSync = useCallback(() => runSync('deferred'), [runSync]); + const syncAgreement = useCallback(() => runSync('direct'), [runSync]); + + return { + ...state, + isTaskRunning: status === 'requested', + requestAgreementSync, + syncAgreement, + }; } diff --git a/frontend/src/shared/test-utils/agreement-mocks.ts b/frontend/src/shared/test-utils/agreement-mocks.ts new file mode 100644 index 0000000..3bd0760 --- /dev/null +++ b/frontend/src/shared/test-utils/agreement-mocks.ts @@ -0,0 +1,13 @@ +// Shared ids and payloads for the agreement sync tests (`useAgreementSync` and the +// `agreements-agreement` plug), so both drive the hook with the same Marketplace shapes. + +export const MOCK_AGREEMENT_ID = 'AGR-1234-5678-9012'; +export const MOCK_TASK_ID = 'TSK-1234-5678'; + +// `GET /public/v1/system/tasks/{id}` answers with the task itself, no `data` envelope. +export const mockTaskResponse = (status: string, progress?: number) => ({ + data: progress === undefined ? { status } : { progress, status }, +}); + +// The Extension Service answers a deferred call with 202 and the task id in a header. +export const mockDeferredAccepted = () => ({ headers: { 'mpt-task-id': MOCK_TASK_ID } }); diff --git a/frontend/src/style.scss b/frontend/src/style.scss index 6c2124e..82bf736 100644 --- a/frontend/src/style.scss +++ b/frontend/src/style.scss @@ -87,6 +87,12 @@ pre, .__pre { margin: 0; } +.template__actions { + display: flex; + flex-wrap: wrap; + gap: var(--spacing-2); +} + .template__section { align-items: flex-start; display: flex;