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

Authored by inka on Sep 19 2023, 2:07 AM.

Details

Reviewers

michal
kamil
ginsu

Commits

rCOMM0c123b4904c0: [lib] Create function for extracing keyserver id from object id

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

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

inka created this revision.Sep 19 2023, 2:07 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptSep 19 2023, 2:08 AM

inka added a parent revision: D9217: [lib] Add logic for handling faonut actions.Sep 19 2023, 2:08 AM

Harbormaster completed remote builds in B22713: Diff 31249.Sep 19 2023, 2:24 AM

inka requested review of this revision.Sep 19 2023, 2:24 AM

tomek added inline comments.Sep 19 2023, 2:51 AM

lib/utils/action-utils.js
42–46	Wondering if we really have to match against two regexes. Can we simplify it and use only one?
47	Can't we simply call `return id.split('\|')[0];`?

inka added a child revision: D9226: [lib] Update KeyserverCall type.Sep 19 2023, 8:09 AM

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.