Page MenuHomePhabricator

[keyserver][lib] Allow skipping createRobotextInThinThread
ClosedPublic

Authored by ashoat on Tue, Sep 24, 11:51 AM.
Tags
None
Referenced Files
F2845393: D13451.id44494.diff
Sun, Sep 29, 9:23 PM
F2845388: D13451.id44499.diff
Sun, Sep 29, 9:22 PM
F2845382: D13451.id44533.diff
Sun, Sep 29, 9:21 PM
F2844526: D13451.diff
Sun, Sep 29, 7:36 PM
F2843351: D13451.diff
Sun, Sep 29, 5:04 PM
Unknown Object (File)
Wed, Sep 25, 1:29 AM
Unknown Object (File)
Wed, Sep 25, 1:29 AM
Unknown Object (File)
Wed, Sep 25, 1:29 AM
Subscribers
None

Details

Summary

This parameter only needs to be specified for FARCASTER_MUTUAL and FRIEND. By making it optional, we can address this feedback on D13443.

Submitting it as a separate diff as I'm not sure the complexity is worth it, or whether there are other alternatives that reviewers might prefer.

Depends on D13453

Test Plan

Flow, and tested the relationship adding flow end-to-end again

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

I don't have a strong opinion on this solution. We can keep it, or modify the type to be an array when the robotext creation flag is irrelevant, or we can even consider introducing specs for relationship actions where we define whether robotext messages could be created. But overall, I don't think it is beneficial to spend too much time on it, so even abandoning it makes sense because it is unlikely to cause any trouble in the future.

keyserver/src/responders/relationship-responders.js
84–89 ↗(On Diff #44499)

I'm wondering whether we should replace this dict with an array... but that could complicate the solution even more.

This revision is now accepted and ready to land.Wed, Sep 25, 3:24 AM

I'll just land this one then

keyserver/src/responders/relationship-responders.js
84–89 ↗(On Diff #44499)

Yeah, I think that would complicate the solution