Skip to content

docs: describe and diagram the Leantime sync - #333

Draft
tuj wants to merge 4 commits into
developfrom
feature/docs-leantime-sync-diagram
Draft

docs: describe and diagram the Leantime sync#333
tuj wants to merge 4 commits into
developfrom
feature/docs-leantime-sync-diagram

Conversation

@tuj

@tuj tuj commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Description

Adds docs/leantime-sync.md with a flowchart, a sequence diagram of one paged run, the scheduled jobs and the command options, plus a hand-drawn SVG of the same pipeline.

Rewrites the README Synchronization section, which pointed at a QueueSyncCommand and an app:queue-sync command that no longer exist, claimed Symfony Scheduler queues the jobs, and named the wrong data provider interface.

Adds docs/leantime-sync.md with a flowchart, a sequence diagram of one paged
run, the scheduled jobs and the command options, plus a hand-drawn SVG of the
same pipeline.

Rewrites the README Synchronization section, which pointed at a QueueSyncCommand
and an app:queue-sync command that no longer exist, claimed Symfony Scheduler
queues the jobs, and named the wrong data provider interface.

@turegjorup turegjorup left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed locally. The prose is the strong part of this PR, and the README rewrite fixes real rot — QueueSyncCommand, app:queue-sync, the Symfony Scheduler claim and the wrong interface name were all wrong before it. The notes below are about the diagrams.

1. #334 has made three statements here wrong

The delete endpoint changed from types + deleted (all four types, unpaginated) to one type per request with type/start/limit/deletedAfter, and the response now carries deletionId. That lands in:

  • docs/images/leantime-sync.svg — the LeantimeDeleteHandler box says "sends the four types". It now sends one type plus start, limit and deletedAfter, and delete() queues a message per type.
  • docs/leantime-sync.md:57DAJ -- "POST /APIData/API/deleted<br>x-api-key, types, deleted" --> API. Both parameter names are gone; the plugin answers 400 on deleted now.
  • docs/leantime-sync.md:137 — "derive modifiedAfter / deleted from it" → deletedAfter.

Two prose bullets became incomplete rather than wrong:

  • :147-148 Paging — "start is an id cursor … the next page starts at the last returned id plus one" holds for the entity endpoints, but the delete endpoint pages on deletionId, the deletion's own row id, not the deleted entity's id. That distinction is the point of #334 — entity ids on a page are in no order, so paging on them skips deletions — and the bullet as written says the opposite.
  • :152-154 Deletions — no longer mentions that this endpoint is paged at all.

2. Worth adding, given what #334 changed

The type order in DELETED_TYPES — timesheets, tickets, milestones, projects — is now load-bearing and transport-dependent. Children have to be removed before parents (projectRemovedFromDataProvider() refuses while issues exist, issueRemovedFromDataProvider() while worklogs exist), and that only holds because sync-deleted runs inline through sync://. On the async transport, pagination interleaves the four types, and a project whose worklog deletions sit on page 2 gets soft-marked instead of removed.

Line 71 already notes that sync-deleted is the inline one. A sentence on why that matters would stop someone "fixing" it by adding -j.

3. The SVG and the flowchart draw the same spine twice

Both run cron → commands → service → queue → handlers → endpoints → DB, with dashed returns and a re-queue.

Unique to the flowchart: the plugin internals (Controllers/API.php with 400 on bad parameters, Services/APIData, Repositories/ApiDataRepository) and EntityRemovedFromDataProviderHandler as its own node. Unique to the SVG and found nowhere else in the file: 1 worker, time-limit 900. Everything else the SVG carries is already in the prose — schedules in the table at :112-121, the sync:// fallback at :69-71, the failed transport at :157-158, the id cursor at :147-148, itk_data_api_modified at :149-151, entity mapping at :141-142, x-api-key at :159-161.

The cost is concrete: item 1 needs the same fact edited in two diagrams plus the prose, and one of those diagrams is hand-placed coordinates.

I tried the obvious alternative — redrawing the SVG in Mermaid 1:1, box for box and arrow for arrow — to see whether one format could carry both. It renders (checked with mermaid-cli v11), but the layout comes out 2584×649 against the SVG's 1480×930, a 4:1 strip that scales the 11px annotations into illegibility in a fixed-width page. The cron jobs will not stack (direction TB on a subgraph is a hint the renderer drops once edges cross the cluster) and the legend floats to a corner. The SVG's readability comes from folding the write path under the read path, which is a coordinate decision Mermaid does not expose — so the SVG is the better of the two for this particular view, and the duplication is better resolved by dropping the flowchart's spine than by converting the SVG.

4. The two diagrams contradict each other in four places

  1. Arrow grammar is inverted — the worst of the four. The SVG's legend defines dashed as "data coming back, and what it queues next", so Upsert*Message and the next-page arrow are dashed. At :61-63 those same three arrows are solid, and dashed means only "data returning". A reader who learns the legend at the top of the page is misled halfway down.
  2. Handler placement — the SVG draws LeantimeUpdateHandler and LeantimeDeleteHandler outside the ECONOMICS box entirely (x 830–1060, between the Economics group at 296–546 and Leantime's at 1104–1462). The flowchart puts them inside economics, which is correct.
  3. Where the response comes from:59-60 draws it from LTDB, skipping the API → APIData → Repository layers the flowchart itself introduced on the request side. The SVG draws it from the endpoint.
  4. Handler granularity — the SVG merges Upsert*Handler and EntityRemoved…Handler into one box; the flowchart splits them into UPS and RMV.

5. The SVG has no heading or caption

It sits at :12 between the intro and the first ##, while both Mermaid diagrams are under headings. Nothing tells a reader which of the two structural diagrams is authoritative, which is plausibly how the four disagreements above went unnoticed. Whichever one stays, a heading and a one-line "what this view is for" would earn its space.

Minor

  • The CHANGELOG entry at :10-13 mentions only the SVG, not the two Mermaid diagrams, and it is the one ## [Unreleased] entry without a PR link — every other entry starts * [PR-NNN](…).
  • The sequence diagram at :75-106 is the only diagram here with no overlap at all: it is the single place the paging loop and the two alt branches are shown as a sequence. Worth keeping whatever happens to the other two.

@turegjorup

Copy link
Copy Markdown
Contributor

Also consider adding ADR's to supplement the docs. And given the amount of Claude driven commits to this project, a CLAUDE.md and project level settings are long overdue.

@tuj
tuj marked this pull request as draft August 21, 2026 05:09
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