Details

Reviewers

atul
ginsu
tomek

Commits

rCOMM3926cc412e51: [keyserver/lib] Set up responders, endpoints, and related functions to fetch…

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 created this revision.Mar 21 2023, 2:02 AM

Herald added a subscriber: ashoat. · View Herald TranscriptMar 21 2023, 2:02 AM

rohan edited the test plan for this revision. (Show Details)Mar 21 2023, 2:03 AM

rohan attached a referenced file: F436921: Screenshot 2023-03-20 at 9.37.57 PM.png. (Show Details)

Harbormaster completed remote builds in B17450: Diff 23908.Mar 21 2023, 2:18 AM

rohan requested review of this revision.Mar 21 2023, 2:18 AM

rohan mentioned this in D6924: [keyserver] Introduce columns in messages table to support pinned messages.Mar 22 2023, 1:38 PM

rohan added a child revision: D7148: [keyserver/lib] Introduce a new message type and spec for toggling pins.Mar 22 2023, 4:38 PM

tomek requested changes to this revision.Mar 23 2023, 9: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.