Page MenuHomePhabricator

[keyserver/lib] Set up responders, endpoints, and related functions to fetch all pinned messages
ClosedPublic

Authored by rohan on Mar 20 2023, 7:02 PM.
Tags
None
Referenced Files
F1789138: D7112.id.diff
Sun, May 19, 2:11 AM
F1788759: D7112.id24495.diff
Sun, May 19, 1:11 AM
F1788100: D7112.id24998.diff
Sun, May 19, 12:40 AM
F1787869: D7112.id24807.diff
Sat, May 18, 11:46 PM
F1786665: D7112.id24039.diff
Sat, May 18, 5:45 PM
Unknown Object (File)
Mon, May 6, 1:57 PM
Unknown Object (File)
Apr 17 2024, 1:41 AM
Unknown Object (File)
Mar 28 2024, 1:04 AM
Subscribers

Details

Summary

Following D7110, this diff handles introducing the necessary endpoint and responders to call the query.

Depends on D7110

Linear: https://linear.app/comm/issue/ENG-3189/set-up-responders-endpoints-and-related-functions-to-fetch-all-pinned

Test Plan

Ran the following code to test, and confirmed output in my console (only including the relevant testing code).

import { fetchPinnedMessages } from 'lib/actions/message-actions.js';

const callFetchPinnedMessages = useServerCall(fetchPinnedMessages);

React.useEffect(() => {
  const getPins = async () => {
    const pins = await callFetchPinnedMessages({ threadID });
    console.log(pins);
  };
  getPins();
}, [...]);

Screenshot 2023-03-20 at 9.37.57 PM.png (1×1 px, 186 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rohan requested review of this revision.Mar 20 2023, 7:18 PM
tomek requested changes to this revision.Mar 23 2023, 2:58 AM
tomek added inline comments.
lib/actions/message-actions.js
253 ↗(On Diff #23908)

Why it is optional? We're always returning a new object from this function.
On the other hand, server declares a response as optional, but here we're accessing a property pinnedMessages of the response without checking if it was returned. Have you tested if this function works correctly when there are no pins?

A better approach would be to make the response mandatory and return e.g. an empty array if there are no pins.

This revision now requires changes to proceed.Mar 23 2023, 2:58 AM

Make response FetchPinnedMessagesResult mandatory

This revision is now accepted and ready to land.Mar 24 2023, 8:20 AM