[CommCoreModule] Remove `last[NavigatedTo/Pruned]` from `MessageStoreThread` and `message_store_threads` table
ClosedPublic
Actions

Authored by atul on Oct 17 2023, 1:56 PM.

Details

Reviewers

ashoat
kamil
tomek
ginsu
rohan

Commits

rCOMM48e72dc9548c: [CommCoreModule] Remove `last[NavigatedTo/Pruned]` from `MessageStoreThread`…

Summary

IMPORTANT: While I tested this migration a few times and read through things as carefully as I could, making changes to C++ codebase and SQLite store can be risky. Adding @ashoat and @kamil as blocking to take a look... would appreciate careful review to ensure everything is correct.

In this diff we:

Remove lastPruned/lastNavigatedTo from MessageStoreThread struct and remove usages on C++ side.
Introduce SQLite migration that drops lastPruned and lastNavigatedTo columns.

As of D9475, we've started ignoring what's persisted here in SQLite entirely... so this is effectively just dead code that we're cleaning up.

Depends on

D9475

Test Plan

C++ codebase continues to build/run without issue.
recreate_message_store_threads_table migration succeeded and new message_store_threads table is as expected:

Before:

After:

Diff Detail

Repository

rCOMM Comm

Branch

master

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Oct 17 2023, 1:56 PM

Herald added a subscriber: will. · View Herald TranscriptOct 17 2023, 1:56 PM

atul edited the test plan for this revision. (Show Details)Oct 17 2023, 1:56 PM

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 17 2023, 1:59 PM

Harbormaster failed remote builds in B23311: Diff 32106!

atul requested review of this revision.Oct 17 2023, 1:59 PM

atul added inline comments.

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
399 ↗	(On Diff #32106)	IMO The migrations at the top of `SQLiteQueryExecutor` are super long and take up a lot of the file. Would be good to move these to a separate `migrations` file.. or even a `migrations` folder where each migration gets its own file.