Page MenuHomePhabricator

[lib] implement `KeyserverStore` spec
ClosedPublic

Authored by kamil on Jan 22 2024, 2:41 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Sep 19, 10:09 AM
Unknown Object (File)
Tue, Sep 17, 9:58 AM
Unknown Object (File)
Tue, Sep 17, 7:55 AM
Unknown Object (File)
Tue, Sep 17, 7:32 AM
Unknown Object (File)
Tue, Sep 17, 7:24 AM
Unknown Object (File)
Wed, Sep 4, 8:13 AM
Unknown Object (File)
Aug 23 2024, 6:26 PM
Unknown Object (File)
Aug 5 2024, 1:38 PM

Details

Summary

Add code with required ops.

Depends on D10754

Test Plan

Flow, functionality tested later in the stack.

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.Jan 22 2024, 6:32 AM
lib/ops/keyserver-store-ops.js
20–21 ↗(On Diff #35910)

Typically we have a remove action that removes a subset, with a payload that looks like this: +payload: { +ids: $ReadOnlyArray<string> },. Can we do the same here?
This will be useful for example for delete account action, that returns an array of keyserver ids to be removed.

lib/ops/keyserver-store-ops.js
79 ↗(On Diff #35910)

Can we take the whole store here? It seems odd that we call this "process store operations", but only process a part of the store.

Couple nits

lib/ops/keyserver-store-ops.js
16 ↗(On Diff #35910)

This payload and the next are not read-only

53–54 ↗(On Diff #35910)

This input obj should be read-only

  • make fields read-only
  • use array in remove operation
kamil added inline comments.
lib/ops/keyserver-store-ops.js
20–21 ↗(On Diff #35910)

I was introducing +payload: { +ids: $ReadOnlyArray<string> }, in other stores because it was a use case for this. In this reducer, we will either remove only one keyserver or remove all (we have a separate operation for it).

But since it was also requested by @marcin in D10775 I can refactor this.

79 ↗(On Diff #35910)

We are doing the same for ReportStore and UserStore, so I think it's fine. If you insist I can update this but I would prefer to do it in a separate diff or D10773 (I know it's a bad convention but if I change this here I will have to update all diffs in the stack, so making this change in the separate diff or the end of the stack will save lot of my time)

This revision is now accepted and ready to land.Jan 29 2024, 1:43 AM
This revision was automatically updated to reflect the committed changes.