Repository navigation
feat!: one activity per work item, without an event type - #103
claude-for-timatic[bot] wants to merge 4 commits into
Conversation
| && $a->ticket_number === $event->ticket_number | ||
| && $a->event_type_id === $event->event_type_id | ||
| && ($a->event_type_id === $event->event_type_id | ||
| || ($a->eventType?->chain_group !== null && $a->eventType->chain_group === $event->eventType?->chain_group)) |
There was a problem hiding this comment.
we could also argue that an activity could consist out of multiple events for the same customer and budget. But that depends on how we exactly define an activity. Is it just some work in a single app? Or would it also be logical if testing in the browser and coding at the same time is considered one activity "Development"
There was a problem hiding this comment.
Agreed and reworked: an activity is now continuous work on one work item (same customer + ticket within 15 minutes, any app or type). Following your follow-up, activities no longer have an event type at all; the types are read from the activity's events. The PR body has the details and the coordinated frontend/SDK PRs.
A review session spreads over commits, pull request comments, approvals, change requests and merges. Each event type became its own activity, so one piece of work on a ticket showed up as several suggestions. Events on the same customer and ticket within the chain gap now chain whatever their event type. Because events are projected by weight before time, an activity that widens can come to lie next to an earlier projected activity of the same ticket; the two merge unless a booked entry or another activity lies in the gap between them. Refs #41
An activity is continuous work on one work item and may now hold events of several types, so a single event type on the activity no longer describes it. Clients read the type of each event instead. BREAKING CHANGE: activities no longer have an `eventTypeId` attribute or an `eventType` relationship, and `activities.eventType` is no longer an allowed include on /entry-suggestions. Events now expose an `eventType` relationship; include it with `activities.events.eventType`. Refs #41
A newly created activity could end up next to an activity of the same ticket without either being widened, so the neighbour merge never ran and one stretch of work stayed split. Every part of a new activity now looks for neighbours too; customerless activities still merge with nothing. Merging lists each event once, as the parts of one split event share it, and two activities no longer merge into a span that would cover a booked entry. Refs #41
a480d7f to
d458bbc
Compare
claude-for-timatic
Bot
changed the title
| return ! $this->entries->contains(fn (Entry $entry) => $entry->started_at->lt($unionEnd) && $entry->ended_at->gt($unionStart)) | ||
| && ($gapEnd->lte($gapStart) || ! $this->activities->contains(fn (Activity $claim) => $claim !== $activity && $claim !== $other | ||
| && $claim->started_at->lt($gapEnd) && $claim->ended_at->gt($gapStart))); | ||
| } |
There was a problem hiding this comment.
It seems that the projector is becoming large, is there a better pattern, a separate class perhaps for each step/rule.
There was a problem hiding this comment.
Agreed, it's growing: 288 lines on main, 334 here, 380 with #33. It holds three concerns rather than many rules:
- Attribution: customer, budget and internal flag, including learning from earlier bookings.
- Overlap: what an activity may claim against entries and other activities.
- Chaining and merging.
Plan: a refactor PR stacked on #33 that extracts EventAttribution and ActivityOverlap (plus ActivityChainer if it still feels big). ActivityProjector then only orchestrates the loop.
Not a class per rule: the steps share the activities collection, and their order matters (weight, then the fill pass, then merging). A rule pipeline would hide that order without making it simpler. It comes after #33 because #33 changes exactly these methods.
An activity is now continuous work on one work item, whatever the app or event type. Events on the same customer and ticket within 15 minutes chain into one activity, so coding, testing and reviewing PROJ-12 become one suggestion. Ticketless events keep chaining on customer only. Because an activity now mixes types, activities no longer have an event type.
Closes #41
Breaking API change: ship together with Timatic/timatic-frontend#30 and Timatic/timatic-php-sdk#10.
API contract
activitieslose theeventTypeIdattribute and theeventTyperelationship.include=activities.eventTypeon/entry-suggestionsis no longer allowed (400).eventsgain aneventTyperelationship of typeeventTypes, with attributename.include=activities.events.eventTypeis allowed and eager-loaded.showloadsactivities.events.sourceandactivities.events.eventType.activities.event_type_idtogether with its FK and index. RunRebundleSuggestionsCommandafter deploy so existing days are re-projected.Decisions (answers to the open questions)
No
code_reviewevent type and no chain group. After review, the event type is no longer a boundary: the activity is the work item (decision in the review of this PR).pr_mergedcounts as work on the ticket and joins the review.Ticketless chaining stays on customer only. Requiring an equal budget would split work where one source knows the budget and another doesn't (unknown is unknown).
Neighbour merge. Events are processed by weight, then start time, so two halves of one stretch can be projected separately. After an activity is created or widened, it merges with an activity of the same customer and ticket within 15 minutes. These keep them apart:
Each event is listed once. The activity projected first is kept.
Out of scope (Allow overlapping activities when multitasking #33):
appendEventToActivitycan still widen an activity across a booked entry. This was already the case on main and is pinned in a test.Review
The first version (a
chain_groupfor review types) was replaced after review on this PR. A code-review agent then reviewed the rework and found:All three are fixed with tests. Mutation-checked: removing any fix fails its test.
Docs to update after #96 (
suggestions/activities.md)activities.events.eventType).a commit, a pull request comment and an approval on one ticket become one activity in any input order,a merged pull request joins the review on its ticket,adjacent events on different tickets stay separate activities.ActivityProjectorTest.Verification
Full Pest suite green (627), PHPStan 0 errors, Pint clean.