Page MenuHomePhabricator

[keyserver][lib] Allow skipping createRobotextInThinThread
ClosedPublic

Authored by ashoat on Sep 24 2024, 11:51 AM.
Tags
None
Referenced Files
F3514390: D13451.id44499.diff
Sun, Dec 22, 4:36 AM
F3514343: D13451.id44494.diff
Sun, Dec 22, 4:20 AM
F3513272: D13451.diff
Sat, Dec 21, 11:32 PM
Unknown Object (File)
Thu, Dec 5, 7:51 PM
Unknown Object (File)
Thu, Dec 5, 7:50 PM
Unknown Object (File)
Thu, Dec 5, 6:20 PM
Unknown Object (File)
Nov 17 2024, 3:02 PM
Unknown Object (File)
Nov 10 2024, 7:25 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.Sep 25 2024, 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