Skip to content

feat(FOUR-30668): add API_FAST_TASK fast path for V1.1 task show - #9097

Open
gproly wants to merge 6 commits into
developfrom
feature/FOUR-30668
Open

gproly wants to merge 6 commits into
developfrom
feature/FOUR-30668

Conversation

@gproly

@gproly gproly commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Introduce showFast on GET api/1.1/tasks/{id} when API_FAST_TASK is enabled, keeping the same endpoint and response shape while reducing duplicate work on large request payloads.

  • Strip data from the include list so requestData is not built twice
  • Load task via Eloquent and process request metadata via raw SQL without the data column; hydrate relations with selective columns
  • Add TaskResource::fromTokenWithIncludes() to avoid preprocessInclude re-query
  • Resolve request data from DB when processRequest has no data attribute (showFast path and Nayra empty datastore)
  • Guard includeBpmnTagName when BPMN definition is missing
  • Add TaskShowFastResponseTimeTest (parity + median timing vs legacy includes)

Config: API_FAST_TASK=false by default in config/app.php
Timing: TASK_SHOW_FAST_MAX_RATIO (default 0.53) for the benchmark test

https://processmaker.atlassian.net/browse/FOUR-30668

Run:
./vendor/bin/phpunit tests/Feature/Api/V1_1/TaskShowFastResponseTimeTest.php

Tests:

  • Same request_data as legacy (fast has no data key)
  • Median of 3 runs: legacy (API_FAST_TASK=false) vs fast (true)

Threshold env:
TASK_SHOW_FAST_MAX_RATIO=0.53 default (~47% faster)
TASK_SHOW_FAST_MAX_RATIO=0.40 ~60% faster
TASK_SHOW_FAST_MAX_RATIO=0.30 ~70% faster

Rule: fast_median <= legacy_median * ratio

Improvement % ≈ (1 - fast/legacy) * 100 (same machine/DB each time)

Captura de pantalla 2026-10-09 a la(s) 5 10 39 p  m

ci:deploy

Introduce showFast on GET api/1.1/tasks/{id} when API_FAST_TASK is enabled,
keeping the same endpoint and response shape while reducing duplicate work on
large request payloads.

- Strip `data` from the include list so `requestData` is not built twice
- Load task via Eloquent and process request metadata via raw SQL without
  the `data` column; hydrate relations with selective columns
- Add TaskResource::fromTokenWithIncludes() to avoid preprocessInclude re-query
- Resolve request data from DB when processRequest has no `data` attribute
  (showFast path and Nayra empty datastore)
- Guard includeBpmnTagName when BPMN definition is missing
- Add TaskShowFastResponseTimeTest (parity + median timing vs legacy includes)

Config: API_FAST_TASK=false by default in config/app.php
Timing: TASK_SHOW_FAST_MAX_RATIO (default 0.53) for the benchmark test

https://processmaker.atlassian.net/browse/FOUR-30668
@cursor

cursor Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes a heavily used task API read path and how large process_requests.data is loaded, though behavior is gated by API_FAST_TASK (default false) and tests cover parity.

Overview
Adds an opt-in API_FAST_TASK fast path for GET api/1.1/tasks/{id} (default off). When enabled, show delegates to showFast, which strips data from the include query so requestData is not built twice, loads the task through TaskV11ShowFastRepository (lean process_requests row without the heavy data column, selective relation columns), and only hydrates request data when requestData is requested.

TaskResource::fromTokenWithIncludes() builds the response from an already-loaded token and skips re-fetching relations that are already eager-loaded. includeBpmnTagName now returns null when BPMN definition is missing instead of erroring.

Feature tests assert request_data parity with the legacy path, that include=data does not add a top-level data key on the fast path, and median response time improvement vs legacy under TASK_SHOW_FAST_MAX_RATIO (default 0.58).

Reviewed by Cursor Bugbot for commit d1b4ecb. Bugbot is set up for automated code reviews on this repo. Configure here.

@gproly
gproly requested review from caleeli and pmPaulis October 9, 2026 21:06

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread ProcessMaker/Http/Resources/V1_1/TaskResource.php
Comment thread ProcessMaker/Managers/DataManager.php Outdated
@vladyrichter

Copy link
Copy Markdown

QA server K8S was successfully deployed https://tenant-1.ci-023a748934.engk8s.processmaker.net

@vladyrichter

Copy link
Copy Markdown

QA server K8S was successfully deployed https://tenant-1.ci-023a748934.engk8s.processmaker.net

- Load process_requests.data in TaskV11ShowFastRepository when showFast
  includes requestData, instead of extending DataManager.
- Restore DataManager to develop behavior; drop resolveRequestDataFromToken.
- Fix empty include after stripping data (remove query param, trim parts).
- Guard includeBpmnTagName when BPMN definition is missing on fast path.
- Add test for include=data-only on API_FAST_TASK; remove DataManager
  hydration unit test superseded by fast-path parity.

https://processmaker.atlassian.net/browse/FOUR-30668
Re-add tests/Feature/Api/V1_1/ProcessVariableControllerTest.php from develop after
it was removed by mistake. Keeps FOUR-30668 aligned with develop for unrelated
V1.1 process-variables coverage.

https://processmaker.atlassian.net/browse/FOUR-30668
Restore tests/Feature/DataManagerTest.php from develop to drop an unrelated
import-only diff. Fast-path request data coverage remains in
TaskShowFastResponseTimeTest.

https://processmaker.atlassian.net/browse/FOUR-30668
@decisions-sonarqube

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d1b4ecb. Configure here.

if ($processRequest->user_id) {
$requestor = User::query()->select(self::REQUESTOR_COLUMNS)->find($processRequest->user_id);
$processRequest->setRelation('user', $requestor);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requestor include loses user fields

Medium Severity

The fast path loads the requestor with only id, firstname, lastname, and email, then fromTokenWithIncludes skips further loading because processRequest.user is already set. Legacy show never applies that slim select: ProcessRequestToken has no requestor relation, so includeRequestor lazy-loads the full processRequest.user. With API_FAST_TASK enabled, requestor drops fields clients already rely on, such as username, avatar, status, and title.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d1b4ecb. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants