Page MenuHomePhabricator
Differential D13411

[lib] Process blob holders in DM ops
ClosedPublic
Actions

Authored by bartek on Sep 20 2024, 6:30 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 1, 5:38 AM
Unknown Object (File)
Wed, Oct 30, 11:25 PM
Unknown Object (File)
Wed, Oct 30, 11:25 PM
Unknown Object (File)
Wed, Oct 30, 11:25 PM
Unknown Object (File)
Wed, Oct 30, 11:24 PM
Unknown Object (File)
Wed, Oct 30, 11:24 PM
Unknown Object (File)
Wed, Oct 30, 11:14 PM
Unknown Object (File)
Tue, Oct 22, 10:40 AM
View All 62 Files
Subscribers
ashoat

Details

Reviewers
kamil
tomek
Commits
rCOMM1dea35865e35: [lib] Process blob holders in DM ops
Summary

Added a hook that dispatches holder processing action as a result of DM ops.

Depends on D13407, D13409, D13410

Test Plan

Created dummy blob op in dm-ops/change-thread-settings for creating and removing a holder.
Watched Redux Devtools and Blob HTTP traffic to confirm that holders are established and removed correctly.
Also tested with D13413 test plan

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek created this revision.Sep 20 2024, 6:30 AM
bartek held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptSep 20 2024, 6:30 AM
bartek added a child revision: D13413: [lib] Create holder ops for thick thread avatars.Sep 20 2024, 6:42 AM
bartek edited the test plan for this revision. (Show Details)Sep 20 2024, 6:42 AM
Harbormaster failed remote builds in B31740: Diff 44374!Sep 20 2024, 6:47 AM
bartek updated this revision to Diff 44380.Sep 20 2024, 6:47 AM

Rebase

bartek updated this revision to Diff 44383.Sep 20 2024, 6:57 AM

Rebase

Harbormaster failed remote builds in B31744: Diff 44380!Sep 20 2024, 7:05 AM
bartek published this revision for review.Sep 20 2024, 7:09 AM
Harbormaster completed remote builds in B31747: Diff 44383.Sep 20 2024, 7:17 AM
bartek added a child revision: D13416: [lib] Clear all holders on logout.Sep 20 2024, 7:41 AM
ashoat added inline comments.Sep 20 2024, 8:02 AM
lib/actions/holder-actions.js
141–142 ↗(On Diff #44383)

Why do we want until getAuthMetadata() resolves before starting createBlobHolderGenerator()?

kamil accepted this revision.Sep 23 2024, 1:16 AM
kamil added inline comments.
lib/actions/holder-actions.js
7–8 ↗(On Diff #44383)

can be merged

129 ↗(On Diff #44383)

nit: I prefer this but since this is not a convention, I've seen both it codebase it's fine 😆

163 ↗(On Diff #44383)

are you sure this will always be defined?

This revision is now accepted and ready to land.Sep 23 2024, 1:16 AM
bartek added inline comments.Sep 23 2024, 5:12 AM
lib/actions/holder-actions.js
129 ↗(On Diff #44383)

We cannot destructure undefined. And I prefer to invariant as late as possible

141–142 ↗(On Diff #44383)

After refactor it's no longer this way

163 ↗(On Diff #44383)

Good catch

bartek updated this revision to Diff 44417.Sep 23 2024, 5:12 AM

Address feedback

Harbormaster completed remote builds in B31770: Diff 44417.Sep 23 2024, 5:29 AM
Closed by commit rCOMM1dea35865e35: [lib] Process blob holders in DM ops (authored by bartek). · Explain WhySep 23 2024, 7:25 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM1dea35865e35: [lib] Process blob holders in DM ops.

Revision Contents
Changeset List

PathSize
lib/
actions/
88 lines
shared/
dm-ops/
32 lines

Diff 44417

View Options

lib/actions/holder-actions.js

Loading...
View Options

lib/shared/dm-ops/process-dm-ops.js

Loading...