Fix timeline dedup over-collapse, empty-state icon, and tenant-scoped reads#30
Merged
Conversation
… reads - ActivityLogSource dedup key now includes the activity id so distinct activities in the same second (multi-field save) no longer collapse to one. - Replace leftover Remix ri-history-line icon in the empty-state with heroicon-o-clock; the missing icon set rendered a blank card. - Resolve the read-path Activity model from the plugin key, then Spatie's canonical activitylog.activity_model, then the base model, so a tenant-scoped Activity subclass applies to reads as well as writes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three fixes to the activity-log timeline, each with regression tests.
BR233-1 — multi-change save collapses to one entry (high)
ActivityLogSourcekeyed dedup asclass:subjectId:secondwith no activity id, so several distinct activities written for the same subject in the same second (e.g. one save touching a core field + a custom field) collapsed into a single rendered entry while the DB held N.Dedup key for the own-activity source now includes the activity id (
dedupKeyForActivity()), so distinct rows stay separate.RelatedActivityLogSourcekeeps the second-precision key on purpose — that is what merges it against the syntheticRelatedModelSourceevents (covered byBuilderDedupTest).BR233-3 — empty Activity log tab renders a blank card (low)
The empty-state branch in
timeline.blade.phpstill referenced the Remix iconri-history-line, missed by the Heroicons migration (#24) because it only renders when the timeline is empty. The missing icon set produced a blank card with no message. Swapped toheroicon-o-clock. The empty-state string was already passed non-empty from both call sites.F3 — timeline read path not tenant-scoped (medium)
The read path resolved its model from
config('activity-log.activity_model'), while hosts configure their (tenant-scoped) Activity subclass via Spatie's canonicalactivitylog.activity_model, which governs writes. The plugin config also defaulted to the base SpatieActivity, so reads ignored any subclass and ran without the team scope.Reads now resolve
activity-log.activity_model→activitylog.activity_model→ baseActivityvia a sharedactivityModelClass(). The plugin config default isnullso it no longer shadows the Spatie key. A host can configure the scoped subclass once and have both writes and timeline reads honor it, even without publishingconfig/activity-log.php.Tests
BuilderDedupTest— multi-change save yields N entries; existing cross-source dedup tests still pass.ActivityModelResolutionTest(+ScopedActivityfixture) — plugin key wins, falls back to Spatie key, then base model.Out of scope
BR233-2 and BR233-4 target the host app's
app/Observers/CustomFieldValueObserver.php, which is not part of this package.