refactor: separate activity occurrence dates from creation dates#791
Conversation
|
@RussH I highly suggest to review this PR before creating the v0.9.8.0 release otherwise the database migration might result in unexpected results. |
8130552 to
9fd9697
Compare
RussH
left a comment
There was a problem hiding this comment.
Thanks — as always this is a good PR. Separating date_occurred from date_created makes sense, and backfilling it from date_created during migration will work.
I think there is one functional issue to fix before merge: in ActivityEntries::add(), $dateOccurredSQL is calculated and passed into sprintf(), but the new date_occurred value appears to be hardcoded as NOW() in the INSERT values list. That means a user-selected activity date may be ignored and stored as the current timestamp instead?
Suggest you update the INSERT so date_occurred uses $dateOccurredSQL, while date_created and date_modified remain NOW().
One other small migration question: is ALTER IGNORE TABLE needed here? Since this is adding a new column rather than resolving duplicate rows, plain ALTER TABLE would be preferable and would be more future-safe for any newer MySQL/MariaDB versions.
=> Otherwise, can you rebase against master so it'll merge without conflicts?
I double-checked the current branch and So the manually selected activity date should be stored in That said, a manual test would probably be helpful here to make sure the behavior is clear end-to-end: create an activity with a manually selected past/future date and verify that the timeline displays/sorts by that selected date, while
Good point on the migration. I think the |
9fd9697 to
dad3d84
Compare
RussH
left a comment
There was a problem hiding this comment.
Thanks — this looks good to me now.
Looks like you’ve made the requested changes. Assuming CI passes, I’m happy to merge this.
This PR separates the functional occurrence timestamp of activity records from their technical creation timestamp.
PR #758 made it possible to log activities with a manual date and time, but the manually selected activity timestamp was stored in
activity.date_created. That made the activity timeline behave as intended, but it also changed the meaning ofdate_created: instead of always representing when the database row was created, it could represent when the activity occurred.This PR adds a dedicated
activity.date_occurredcolumn and backfills it from the existingdate_createdvalues so that existing activity timelines keep their current ordering and displayed timestamps after migration.Manual activity dates are now stored in
date_occurred, whiledate_createdis always set toNOW()when a new activity row is inserted. Editing an activity date also updatesdate_occurredinstead of overwritingdate_created.Activity timelines, activity grids, date filters, activity statistics, and latest-activity lookups now use
date_occurredas the functional activity timestamp. Existing output aliases such asdateCreated,dateCreatedSortandlastActivityare intentionally kept unchanged to avoid unnecessary template and JavaScript changes.This restores
date_createdas the technical record creation timestamp while preserving the user-facing behavior introduced by manual activity dates.