Skip to content

Fix activity log properties for Spatie v5 attribute_changes#186

Merged
ht3aa merged 2 commits intomainfrom
cursor/2026-04-03-activity-log-properties
Apr 3, 2026
Merged

Fix activity log properties for Spatie v5 attribute_changes#186
ht3aa merged 2 commits intomainfrom
cursor/2026-04-03-activity-log-properties

Conversation

@ht3aa
Copy link
Copy Markdown
Owner

@ht3aa ht3aa commented Apr 3, 2026

Summary

The activity log Properties modal and detail page stopped showing changed attributes after upgrading to spatie/laravel-activitylog v5, which stores model diffs in the attribute_changes column. The dashboard only read properties, which is now reserved for custom withProperties() data.

Changes

  • Merge attribute_changes with properties in ActivityLogController for the JSON properties endpoint and the Inertia show payload (legacy rows that still use properties for attributes/old remain supported).
  • Add ActivityLogPropertiesTest covering the merge behavior.

Testing

  • php artisan test --compact tests/Feature/ActivityLogPropertiesTest.php

Made with Cursor

Merge attribute_changes with properties in ActivityLogController so the
Properties modal and detail page receive attributes/old again. Spatie
laravel-activitylog v5 stores model diffs in attribute_changes, not
properties. Add feature tests for the properties JSON endpoint.

Made-with: Cursor
@ht3aa ht3aa added the cursor Pull request opened by a Cursor agent label Apr 3, 2026
@ht3aa ht3aa merged commit ab0e4e3 into main Apr 3, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the ActivityLogController to correctly merge Spatie v5 model attribute changes with custom properties, ensuring that legacy and new data formats are handled consistently. I have reviewed the changes and the accompanying feature test; the test case for merging overlapping keys should be updated to explicitly include a collision to verify the merge logic as described in the test name.

Comment on lines +55 to +57
'properties' => [
'note' => 'extra',
],
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The test name merges custom properties with attribute_changes with changes winning overlapping keys suggests it tests for key collisions, but the current implementation of the test does not have any overlapping keys between properties and attribute_changes. To make the test more robust and match its description, you should add an overlapping key to the properties array. This will ensure the merge precedence is correctly tested.

        'properties' => [
            'attributes' => ['title' => 'Should be overwritten'],
            'note' => 'extra',
        ],

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

Labels

cursor Pull request opened by a Cursor agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant