Skip to content
This repository has been archived by the owner on Aug 11, 2024. It is now read-only.

Log key activities in SQLite #1067

Merged
merged 10 commits into from
Jan 16, 2024
Merged

Log key activities in SQLite #1067

merged 10 commits into from
Jan 16, 2024

Conversation

bfollington
Copy link
Collaborator

@bfollington bfollington commented Jan 10, 2024

Trying not to over engineer anything here.

  • Create a new activity SQLite table
  • Log events we may be interested in responding to / surfacing:

As mentioned above, this is a step towards offering more sophisticated behaviour in the Deck view over multiple sessions but it could also let us offer an "activity" view in the app where you can see updates like "@gordon published 4 notes".

Screen.Recording.2024-01-10.at.12.21.16.pm.mov

@bfollington bfollington marked this pull request as ready for review January 10, 2024 02:28
Copy link
Collaborator

@gordonbrander gordonbrander left a comment

Choose a reason for hiding this comment

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

One test request. Otherwise LGTM.

@@ -1371,9 +1372,27 @@ final class DatabaseService {
})
.first
}

func writeActivity<Data: Codable>(event: ActivityEvent<Data>) throws {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add a test for this method using in-memory SQL

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep 👍

metadata TEXT DEFAULT '{}' NOT NULL,
created TEXT DEFAULT CURRENT_TIMESTAMP NOT NULL
);
"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to also add a test for these migrations using a fresh in-memory SQL DB? This is a question. I don't recall if I put together the code to make that work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

Copy link
Collaborator

@gordonbrander gordonbrander left a comment

Choose a reason for hiding this comment

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

LGTM!

@bfollington bfollington merged commit ad2681c into main Jan 16, 2024
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants