From f5d3980857320f6c7726cfdf814e5bb3358e7f21 Mon Sep 17 00:00:00 2001 From: AsmitNepali Date: Sun, 14 Jun 2026 18:55:55 +0545 Subject: [PATCH 1/2] Fix timeline dedup over-collapse, empty-state icon, and tenant-scoped 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. --- config/activity-log.php | 9 ++++- resources/views/timeline.blade.php | 2 +- .../Sources/AbstractTimelineSource.php | 29 ++++++++++++++ src/Timeline/Sources/ActivityLogSource.php | 12 +----- .../Sources/RelatedActivityLogSource.php | 12 +----- tests/Feature/ActivityModelResolutionTest.php | 40 +++++++++++++++++++ tests/Feature/BuilderDedupTest.php | 25 ++++++++++++ tests/Fixtures/Models/ScopedActivity.php | 22 ++++++++++ 8 files changed, 128 insertions(+), 23 deletions(-) create mode 100644 tests/Feature/ActivityModelResolutionTest.php create mode 100644 tests/Fixtures/Models/ScopedActivity.php diff --git a/config/activity-log.php b/config/activity-log.php index 5e6c337..f9591ef 100644 --- a/config/activity-log.php +++ b/config/activity-log.php @@ -3,8 +3,13 @@ declare(strict_types=1); return [ - 'activity_model' => \Spatie\Activitylog\Models\Activity::class, - + // Activity model for the read (timeline) path. Leave null to inherit + // Spatie's `activitylog.activity_model` — the same model used for writes — + // so a tenant-scoped Activity subclass applies to both. Set explicitly only + // to override that. + 'activity_model' => null, + + 'default_per_page' => 20, 'pagination_buffer' => 2, 'deduplicate_by_default' => true, diff --git a/resources/views/timeline.blade.php b/resources/views/timeline.blade.php index 7801d71..71091f8 100644 --- a/resources/views/timeline.blade.php +++ b/resources/views/timeline.blade.php @@ -2,7 +2,7 @@ @if ($entries->isEmpty())
- +

{{ $emptyState }}

diff --git a/src/Timeline/Sources/AbstractTimelineSource.php b/src/Timeline/Sources/AbstractTimelineSource.php index 4df8a4d..d027c26 100644 --- a/src/Timeline/Sources/AbstractTimelineSource.php +++ b/src/Timeline/Sources/AbstractTimelineSource.php @@ -19,6 +19,25 @@ public function priority(): int return $this->priority; } + /** + * Resolve the Activity model used for the read path. + * + * Prefers the plugin's own key, then falls back to Spatie's canonical + * `activitylog.activity_model` — the same key that governs writes — so the + * rendered timeline reads through whatever (possibly tenant-scoped) Activity + * subclass the host configured, even when the hyphenated plugin config was + * never published. + * + * @return class-string<\Spatie\Activitylog\Contracts\Activity&\Illuminate\Database\Eloquent\Model> + */ + protected function activityModelClass(): string + { + /** @var class-string<\Spatie\Activitylog\Contracts\Activity&\Illuminate\Database\Eloquent\Model> */ + return config('activity-log.activity_model') + ?? config('activitylog.activity_model') + ?? \Spatie\Activitylog\Models\Activity::class; + } + protected function dedupKeyFor(string $class, int|string $id, CarbonImmutable $occurredAt): string { return sprintf( @@ -29,6 +48,16 @@ protected function dedupKeyFor(string $class, int|string $id, CarbonImmutable $o ); } + /** + * Dedup key for an activity-log row. Unlike the second-precision key above, + * this includes the activity id so several distinct activities written for + * the same subject in the same second (e.g. a multi-field save) stay separate. + */ + protected function dedupKeyForActivity(string $class, int|string $id, CarbonImmutable $occurredAt, int|string $activityId): string + { + return $this->dedupKeyFor($class, $id, $occurredAt).':'.$activityId; + } + /** * @return Relation */ diff --git a/src/Timeline/Sources/ActivityLogSource.php b/src/Timeline/Sources/ActivityLogSource.php index 4c7e0e5..a321eaf 100644 --- a/src/Timeline/Sources/ActivityLogSource.php +++ b/src/Timeline/Sources/ActivityLogSource.php @@ -13,19 +13,11 @@ final class ActivityLogSource extends AbstractTimelineSource { - /** - * Get the activity model class from config or use default - */ - private function getActivityModelClass(): string - { - return config('activity-log.activity_model', ActivityModel::class); - } - public function resolve(Model $subject, Window $window): iterable { throw_if($subject->getKey() === null, DomainException::class, 'ActivityLogSource cannot resolve entries for an unsaved subject.'); - $modelClass = $this->getActivityModelClass(); + $modelClass = $this->activityModelClass(); $query = $modelClass::query() ->with(['causer', 'subject']) @@ -56,7 +48,7 @@ private function makeEntry(Model $subject, ActivityModel $activity): TimelineEnt type: 'activity_log', event: $event, occurredAt: $occurredAt, - dedupKey: $this->dedupKeyFor($subject->getMorphClass(), (string) $subject->getKey(), $occurredAt), + dedupKey: $this->dedupKeyForActivity($subject->getMorphClass(), (string) $subject->getKey(), $occurredAt, (string) $activity->getKey()), sourcePriority: $this->priority, subject: $subject, causer: $activity->causer, diff --git a/src/Timeline/Sources/RelatedActivityLogSource.php b/src/Timeline/Sources/RelatedActivityLogSource.php index 7a01a38..97ee0ef 100644 --- a/src/Timeline/Sources/RelatedActivityLogSource.php +++ b/src/Timeline/Sources/RelatedActivityLogSource.php @@ -14,14 +14,6 @@ final class RelatedActivityLogSource extends AbstractTimelineSource { - /** - * Get the activity model class from config or use default - */ - private function getActivityModelClass(): string - { - return config('activity-log.activity_model', ActivityModel::class); - } - /** * @param array $relations */ @@ -53,8 +45,8 @@ public function resolve(Model $subject, Window $window): iterable return; } - $modelClass = $this->getActivityModelClass(); - + $modelClass = $this->activityModelClass(); + $query = $modelClass::query() ->with(['causer', 'subject']) ->where(function (Builder $q) use ($subjectPairs): void { diff --git a/tests/Feature/ActivityModelResolutionTest.php b/tests/Feature/ActivityModelResolutionTest.php new file mode 100644 index 0000000..3277867 --- /dev/null +++ b/tests/Feature/ActivityModelResolutionTest.php @@ -0,0 +1,40 @@ +set('activity-log.activity_model', ScopedActivity::class); + + $person = Person::factory()->create(); + + $entries = collect((new ActivityLogSource(priority: 10))->resolve($person, new Window(cap: 10))); + + expect($entries)->toBeEmpty(); +}); + +it('falls back to Spatie activitylog.activity_model when the plugin key is null', function (): void { + config()->set('activity-log.activity_model', null); + config()->set('activitylog.activity_model', ScopedActivity::class); + + $person = Person::factory()->create(); + + $entries = collect((new ActivityLogSource(priority: 10))->resolve($person, new Window(cap: 10))); + + expect($entries)->toBeEmpty(); +}); + +it('uses the base Spatie Activity when nothing is configured', function (): void { + config()->set('activity-log.activity_model', null); + config()->set('activitylog.activity_model', null); + + $person = Person::factory()->create(); + + $entries = collect((new ActivityLogSource(priority: 10))->resolve($person, new Window(cap: 10))); + + expect($entries)->toHaveCount(1); +}); diff --git a/tests/Feature/BuilderDedupTest.php b/tests/Feature/BuilderDedupTest.php index 45eb5e8..2647a4c 100644 --- a/tests/Feature/BuilderDedupTest.php +++ b/tests/Feature/BuilderDedupTest.php @@ -65,6 +65,31 @@ expect($entries)->toHaveCount(1); }); +it('keeps distinct own-activity entries that land in the same second (multi-change save)', function (): void { + $person = Person::factory()->create(); + $person->update(['name' => 'First change']); + $person->update(['name' => 'Second change']); + + // Simulate one save producing several activity rows in the same second. + $stamp = CarbonImmutable::parse('2026-04-17T10:00:00Z'); + Activity::query() + ->where('subject_type', $person->getMorphClass()) + ->where('subject_id', $person->id) + ->update(['created_at' => $stamp]); + + $count = Activity::query() + ->where('subject_type', $person->getMorphClass()) + ->where('subject_id', $person->id) + ->count(); + + $entries = TimelineBuilder::make($person) + ->fromActivityLog() + ->deduplicate() + ->get(); + + expect($entries)->toHaveCount($count); +}); + it('deduplicate(false) skips dedup entirely', function (): void { $person = Person::factory()->create(); $email = Email::factory()->for($person)->create(); diff --git a/tests/Fixtures/Models/ScopedActivity.php b/tests/Fixtures/Models/ScopedActivity.php new file mode 100644 index 0000000..a228b6c --- /dev/null +++ b/tests/Fixtures/Models/ScopedActivity.php @@ -0,0 +1,22 @@ +whereRaw('1 = 0'); + }); + } +} From cdac57a836ebd01ce67eb5eff3d1d7fa80c41b6f Mon Sep 17 00:00:00 2001 From: AsmitNepali Date: Sun, 14 Jun 2026 19:34:51 +0545 Subject: [PATCH 2/2] Keep getActivityModelClass() name, move resolver to base class --- src/Timeline/Sources/AbstractTimelineSource.php | 2 +- src/Timeline/Sources/ActivityLogSource.php | 2 +- src/Timeline/Sources/RelatedActivityLogSource.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Timeline/Sources/AbstractTimelineSource.php b/src/Timeline/Sources/AbstractTimelineSource.php index d027c26..cf43e55 100644 --- a/src/Timeline/Sources/AbstractTimelineSource.php +++ b/src/Timeline/Sources/AbstractTimelineSource.php @@ -30,7 +30,7 @@ public function priority(): int * * @return class-string<\Spatie\Activitylog\Contracts\Activity&\Illuminate\Database\Eloquent\Model> */ - protected function activityModelClass(): string + protected function getActivityModelClass(): string { /** @var class-string<\Spatie\Activitylog\Contracts\Activity&\Illuminate\Database\Eloquent\Model> */ return config('activity-log.activity_model') diff --git a/src/Timeline/Sources/ActivityLogSource.php b/src/Timeline/Sources/ActivityLogSource.php index a321eaf..a885a56 100644 --- a/src/Timeline/Sources/ActivityLogSource.php +++ b/src/Timeline/Sources/ActivityLogSource.php @@ -17,7 +17,7 @@ public function resolve(Model $subject, Window $window): iterable { throw_if($subject->getKey() === null, DomainException::class, 'ActivityLogSource cannot resolve entries for an unsaved subject.'); - $modelClass = $this->activityModelClass(); + $modelClass = $this->getActivityModelClass(); $query = $modelClass::query() ->with(['causer', 'subject']) diff --git a/src/Timeline/Sources/RelatedActivityLogSource.php b/src/Timeline/Sources/RelatedActivityLogSource.php index 97ee0ef..061ca71 100644 --- a/src/Timeline/Sources/RelatedActivityLogSource.php +++ b/src/Timeline/Sources/RelatedActivityLogSource.php @@ -45,7 +45,7 @@ public function resolve(Model $subject, Window $window): iterable return; } - $modelClass = $this->activityModelClass(); + $modelClass = $this->getActivityModelClass(); $query = $modelClass::query() ->with(['causer', 'subject'])