Page MenuHomePhabricator

[lib] Create function for extracing keyserver id from object id
ClosedPublic

Authored by inka on Sep 19 2023, 2:07 AM.
Tags
None
Referenced Files
F3181603: D9225.diff
Fri, Nov 8, 6:37 AM
Unknown Object (File)
Sat, Nov 2, 6:34 AM
Unknown Object (File)
Tue, Oct 29, 11:10 AM
Unknown Object (File)
Sun, Oct 27, 10:19 PM
Unknown Object (File)
Sun, Oct 27, 10:19 PM
Unknown Object (File)
Sun, Oct 27, 10:19 PM
Unknown Object (File)
Sun, Oct 27, 10:19 PM
Unknown Object (File)
Sun, Oct 27, 10:02 PM
Subscribers

Details

Summary

issue: https://linear.app/comm/issue/ENG-4682/create-a-function-to-extract-keyserver-id-from-ids
We need a function that will allow us to extract the keyserver id from message/thread/etc id

Test Plan

Ran the provided test suite

Diff Detail

Repository
rCOMM Comm
Branch
inka/actions_new_branch
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka requested review of this revision.Sep 19 2023, 2:24 AM
lib/utils/action-utils.js
42–46 ↗(On Diff #31249)

Wondering if we really have to match against two regexes. Can we simplify it and use only one?

47 ↗(On Diff #31249)

Can't we simply call return id.split('|')[0];?

This does not handle pending threads (more precisely sidebars created from a message). It's might be fine depending on the use case but I it feel like it should have a code comment in that case. Also agree with @tomek comments.

This does not handle pending threads (more precisely sidebars created from a message)

Pending threads exist in two contexts: pending sidebars, and pending chats created via the "Create new chat" interface.

Curiously, it does appear that BaseNewThreadRequest takes an optional id field. However, threadRequestValidationShape excludes this, meaning that it's only possible to set the id of a new thread from the keyserver. As such, I don't think pending thread IDs ever get passed up in an action... so this should (hopefully) be safe.

This revision is now accepted and ready to land.Oct 4 2023, 6:54 AM