Page MenuHomePhabricator

[web/keyserver] Handle different thread ids in url
ClosedPublic

Authored by michal on Sep 12 2023, 5:36 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 5:00 PM
Unknown Object (File)
Fri, Nov 15, 7:03 PM
Unknown Object (File)
Fri, Nov 8, 12:28 AM
Unknown Object (File)
Fri, Nov 8, 12:27 AM
Unknown Object (File)
Fri, Nov 8, 12:27 AM
Unknown Object (File)
Fri, Nov 8, 12:08 AM
Unknown Object (File)
Thu, Nov 7, 8:05 PM
Unknown Object (File)
Sep 28 2024, 7:35 PM
Subscribers

Details

Summary

ENG-4752
Depends on D9147
Depends on D9146

We want to handle older links too. Previously the server just ignored errors if the link couldn't be converted (so e.g. it contained older ids). Now we convert the ids in the urlinfo to the new schema on the client and server always tries to convert them.

Test Plan

Navigated to:

  • http://localhost:3000/comm/chat/thread/256%7C91890/ and http://localhost:3000/comm/chat/thread/91890/ -> web app opened correct thread
  • http://localhost:3000/comm/chat/thread/pending/sidebar/256%7C91894/ and http://localhost:3000/comm/chat/thread/pending/sidebar/91894/ -> the keyserver doesn't support navigating to pending threads right now, so the web app opened on the most recent thread, but there were no error
  • http://localhost:3000/comm/chat/thread/new/83884+86227/ -> web app opened on the chat creation menu with the selected users
  • http://localhost:3000/comm/settings/account/ -> web app opened on settings

Also run yarn jest

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul added inline comments.
lib/utils/conversion-utils.js
35

Nit: Would prefer using template literal instead of + operator to concatenate strings

42–54

Could we go with something like

if (!pendingIDContents) {
  throw new Error('invalid_client_id_prefix');
}

if (!pendingIDContents.sourceMessageID) {
  return id;
}

return getPendingThreadID(
  pendingIDContents.threadType,
  pendingIDContents.memberIDs,
  pendingIDContents.sourceMessageID.substring(prefix.length),
);

to reduce indentation and improve readability?

This revision is now accepted and ready to land.Sep 14 2023, 9:46 AM