Page MenuHomePhabricator

[web] processing ThreadStore ops on worker
ClosedPublic

Authored by kamil on Nov 22 2023, 8:20 AM.
Tags
None
Referenced Files
F3376043: D9954.id33706.diff
Tue, Nov 26, 10:07 PM
F3375444: D9954.diff
Tue, Nov 26, 8:15 PM
Unknown Object (File)
Tue, Nov 19, 7:00 AM
Unknown Object (File)
Tue, Nov 19, 6:08 AM
Unknown Object (File)
Thu, Nov 7, 10:21 PM
Unknown Object (File)
Thu, Nov 7, 10:09 PM
Unknown Object (File)
Thu, Nov 7, 9:46 PM
Unknown Object (File)
Wed, Nov 6, 7:54 AM
Subscribers

Details

Summary

Start processing ops on worker, the same API as on native.

Depends on D9952

Test Plan

Test if each operation works properly

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Nov 23 2023, 3:15 AM
kamil added inline comments.
web/database/utils/store.js
36 ↗(On Diff #33514)

We need this ( > 0) to avoid overriding thread store for non-staff users (we start with putting thread data to DB only for staff, so for others, it will be an empty array and could override data).

web/redux/redux-setup.js
240 ↗(On Diff #33514)

this line should be conditional depending on canUseDatabaseOnWeb introduced in D9948

tomek added inline comments.
web/database/utils/store.js
36 ↗(On Diff #33514)

It doesn't sound possible, but just to make sure: is it possible for a staff user to have threads.length === 0 in the DB? Do we have a way of determining if a user is a staff other than checking the length?

web/redux/redux-setup.js
240 ↗(On Diff #33514)

Is this something that will be added in this diff, or later in the stack?

This revision is now accepted and ready to land.Nov 24 2023, 1:17 AM
web/database/utils/store.js
36 ↗(On Diff #33514)

It might be worth considering a scenario where a staff user hasn't accepted updated policies when they load the web app. In that case, would threads.length === 0 for that staff user?

(Not sure if this is a concern or not, just figured it's relevant to @tomek's question)

web/database/utils/store.js
36 ↗(On Diff #33514)

is it possible for a staff user to have threads.length === 0 in the DB?

Yes, this is the case pointed out by @ashoat.

Do we have a way of determining if a user is a staff other than checking the length?

It's not for determining if a user is a staff, it's for determining whether threads are in a table or not

  1. If threads are in DB, we read it and populate the store
  2. If threads are not in DB (length 0) we could have 4 scenarios:

a. this is a non-staff user, we don't want to treat an empty array as a source of truth and override the state with empty threads, so we return a null
b. this is a staff user, so it means threads are not migrated yet, we return null to fetch it from the keyserver (D9959)
c. user has not acknowledged policies, so returning null is also okay, but doesn't change anything
d. user doesn't have any threads, and if it's possible or not, in the worst case it will ask the keyserver for threads because logic from D9959 will think it's case b, and keyserver will return an empty object again, but this is a really corner case, as right now each user has at least one thread.

so I think it's safe to keep this condition with length > 0

36 ↗(On Diff #33514)

It might be worth considering a scenario where a staff user hasn't accepted updated policies when they load the web app. In that case, would threads.length === 0 for that staff user?

(Not sure if this is a concern or not, just figured it's relevant to @tomek's question)

That case applies to only changed policies when the user is logged out, when the user logs in thread length is 0 anyway, so this change has no influence on policies logic.

web/redux/redux-setup.js
240 ↗(On Diff #33514)

In this diff, I just discovered I missed this change while creating a diff and made a note to myself and the reviewers. Adding this right now

This revision was automatically updated to reflect the committed changes.