Skip to content

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Aug 19, 2025

The main changes here are all in tests. The last few commits make a small refactor to our model code which we'll want for #814, and which starts requiring that the self-user is present in the list of users the server returns in the initial snapshot.

That should always be true according to the API, but previously it was false for the test data in tons of our tests. So the first N commits of the branch fix our test data to make it true there.

Over the next few commits, we'll ensure that all tests' data put the
self-user in the users list of the initial snapshot.  This will help
by making that happen automatically for the majority of tests.
This completes the sweep through the tests to ensure the self-user
is always found in the list of users in the initial snapshot,
like it would be in a real-life initial snapshot from a Zulip server.

Upcoming commits will start checking that expectation in the
model code.
We'll need to pass RealmStoreImpl some details of the self-user, in
order to correctly interpret (for zulip#814) the permissions that live
on RealmStore.

But because UserStore already depends on RealmStore, we don't want
to introduce a dependency in RealmStoreImpl on UserStore as a whole.
So separate out the step that processes the user lists, and do that
step in advance before constructing RealmStoreImpl.
We'll want to pass this to RealmStoreImpl soon, in order to interpret
permissions correctly.

In the series of commits leading up to here, we've already adapted all
the tests' data to satisfy this invariant.
This is already guaranteed by PerAccountStore.fromInitialSnapshot,
the only caller of this constructor.
@gnprice gnprice requested a review from chrisbobbe August 19, 2025 01:45
@gnprice gnprice added the maintainer review PR ready for review by Zulip maintainers label Aug 19, 2025
@gnprice gnprice mentioned this pull request Aug 20, 2025
@chrisbobbe chrisbobbe merged commit edeb05d into zulip:main Aug 20, 2025
1 check passed
@chrisbobbe
Copy link
Collaborator

Thanks, LGTM! Merged.

@gnprice gnprice deleted the pr-ensure-selfuser branch August 20, 2025 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer review PR ready for review by Zulip maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants