Details

Reviewers

ashoat
varun

Commits

rCOMM65ca88236580: [keyserver] create message and sidebar based on cast

Summary

This creates a message based on a cast and converts it to a sidebar. If there is a parent cast included in the webhook event, we use the parent cast, if not, we use the tagger's cast.

Depends D13815

Example sidebar creation with commbot

Test Plan

Triggered the creation of a new community with a webhook POST request and confirmed sidebar was correctly created in the community

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

will created this revision.Oct 23 2024, 10:38 PM

Herald added a subscriber: tomek. · View Herald TranscriptOct 23 2024, 10:38 PM

will added a child revision: D13783: [keyserver] create invite link based on newly created sidebar.Oct 23 2024, 10:42 PM

will added a parent revision: D13775: [keyserver] add community creation logic to farcaster webhook responder.

Harbormaster completed remote builds in B32359: Diff 45364.Oct 23 2024, 10:55 PM

will requested review of this revision.Oct 23 2024, 10:55 PM

will added inline comments.Oct 24 2024, 6:18 AM

keyserver/src/responders/farcaster-webhook-responders.js
246 ↗	(On Diff #45364)	Placeholder for now

Can you a share a screenshot of an example message/sidebar that is created this way?

keyserver/src/responders/farcaster-webhook-responders.js
44–47 ↗	(On Diff #45364)	Can we move this logic to the caller, and avoid passing both params in here? Can you add a code comment explaining the logic here? Seems like it would cleaner like this (see code change). Note that this is slightly different in the case where `parentHash` is an empty string (I think `??` only considers `null` / `undefined`)... not sure if this matters here, but if it does you could do `parentHash ? parentHash : castHash` instead
57 ↗	(On Diff #45364)	Not sure the newline makes sense here
58 ↗	(On Diff #45364)	Can we put quotes around the cast text? Might want to consider "escaping" quotes inside of the text, although not sure how important that is
76 ↗	(On Diff #45364)	Is the condition necessary here? Why'd you choose to diverge from the approach we use for sidebar titles in the rest of the codebase?

This revision now requires changes to proceed.Oct 24 2024, 6:20 AM

will removed a parent revision: D13775: [keyserver] add community creation logic to farcaster webhook responder.Oct 29 2024, 4:35 PM

will added a parent revision: D13812: [keyserver] Add dontCheckJoinPermissions override option to createThread for sidebar.

will edited the summary of this revision. (Show Details)Oct 29 2024, 11:27 PM

will edited parent revisions, added: D13815: [keyserver] check if viewer role is positive for updateThread; removed: D13812: [keyserver] Add dontCheckJoinPermissions override option to createThread for sidebar.

will edited the summary of this revision. (Show Details)Oct 30 2024, 12:15 AM

will edited parent revisions, added: D13816: [keyserver] add dontAddViewerAsMember to createThread; removed: D13815: [keyserver] check if viewer role is positive for updateThread.

will removed a parent revision: D13816: [keyserver] add dontAddViewerAsMember to createThread.Oct 30 2024, 10:11 AM

will edited the summary of this revision. (Show Details)Oct 30 2024, 11:12 AM

will added a parent revision: D13815: [keyserver] check if viewer role is positive for updateThread.

feedback and rebase

keyserver/src/responders/farcaster-webhook-responders.js
44–47 ↗	(On Diff #45364)	Moved logic to the caller by passing a single `sidebarCastHash` as an arg Added this code comment to the caller like so: `// we use the parent cast to create the sidebar source message if it exists` Used `const sidebarCastHash = parentHash ?? castHash;`. parentHash should only be a full non-empty hash or null
57 ↗	(On Diff #45364)	Included in latest rebase
58 ↗	(On Diff #45364)	Included in latest rebase
76 ↗	(On Diff #45364)	condition was not necessary didn't find this approach. Added usage of trimText in latest rebase

accidentally left a console.log

keyserver/src/responders/farcaster-webhook-responders.js
82	Following previous feedback, we're now going to create the message with the viewer

will edited the summary of this revision. (Show Details)Oct 30 2024, 1:22 PM

Harbormaster completed remote builds in B32453: Diff 45486.Oct 30 2024, 2:03 PM

Harbormaster completed remote builds in B32454: Diff 45487.Oct 30 2024, 2:10 PM

ashoat accepted this revision.Oct 30 2024, 2:39 PM

ashoat added inline comments.

keyserver/src/responders/farcaster-webhook-responders.js
92 ↗	(On Diff #45487)	Instead of copy-pasting this from the place I linked, please factor it out. Copy-paste is almost never appropriate... please consider DRY principles going forward

This revision is now accepted and ready to land.Oct 30 2024, 2:39 PM

will mentioned this in D13775: [keyserver] add community creation logic to farcaster webhook responder.Oct 30 2024, 6:11 PM

rebase

Harbormaster completed remote builds in B32462: Diff 45495.Oct 30 2024, 6:54 PM

remove trimText code duplication with sidebar thread name function

will added inline comments.Oct 31 2024, 6:10 AM

keyserver/src/responders/farcaster-webhook-responders.js
92 ↗	(On Diff #45487)	Created a function for this called `createSidebarThreadName` which performs the trimText in the latest rebase. I thought about where to put it but decided that `lib/shared/sidebar-utils.js` was an appropriate place to keep it.

Harbormaster completed remote builds in B32472: Diff 45505.Oct 31 2024, 6:25 AM

use eventChannel from prior diff instead of event.data.channel

Harbormaster completed remote builds in B32495: Diff 45529.Oct 31 2024, 10:51 AM

Closed by commit rCOMM65ca88236580: [keyserver] create message and sidebar based on cast (authored by will). · Explain WhyOct 31 2024, 12:19 PM

This revision was automatically updated to reflect the committed changes.

will added a commit: rCOMM65ca88236580: [keyserver] create message and sidebar based on cast.

Diff 45486

View Options

keyserver/src/responders/farcaster-webhook-responders.js

	// @flow			// @flow

	import invariant from 'invariant';			import invariant from 'invariant';
	import * as React from 'react';			import * as React from 'react';

	import { logOutActionTypes, useLogOut } from '../actions/user-actions.js';			import {
				keyserverAuthActionTypes,
				logOutActionTypes,
				useKeyserverAuth,
				useLogOut,
				} from '../actions/user-actions.js';
				import { extractKeyserverIDFromID } from '../keyserver-conn/keyserver-call-utils.js';
				import { filterThreadIDsInFilterList } from '../reducers/calendar-filters-reducer.js';
	import {			import {
	connectionSelector,			connectionSelector,
	cookieSelector,			cookieSelector,
				deviceTokenSelector,
	} from '../selectors/keyserver-selectors.js';			} from '../selectors/keyserver-selectors.js';
	import { IdentityClientContext } from '../shared/identity-client-context.js';			import { IdentityClientContext } from '../shared/identity-client-context.js';
	import { OlmSessionCreatorContext } from '../shared/olm-session-creator-context.js';			import { OlmSessionCreatorContext } from '../shared/olm-session-creator-context.js';
	import type { BaseSocketProps } from '../socket/socket.react.js';			import type { BaseSocketProps } from '../socket/socket.react.js';
				import { logInActionSources } from '../types/account-types.js';
	import { useDispatchActionPromise } from '../utils/redux-promise-utils.js';			import { useDispatchActionPromise } from '../utils/redux-promise-utils.js';
	import { useSelector } from '../utils/redux-utils.js';			import { useSelector } from '../utils/redux-utils.js';
	import { usingCommServicesAccessToken } from '../utils/services-utils.js';			import { usingCommServicesAccessToken } from '../utils/services-utils.js';
	import { ashoatKeyserverID } from '../utils/validation-utils.js';			import { ashoatKeyserverID } from '../utils/validation-utils.js';

	type Props = {			type Props = {
	...BaseSocketProps,			...BaseSocketProps,
	+keyserverID: string,			+keyserverID: string,
	+socketComponent: React.ComponentType<BaseSocketProps>,			+socketComponent: React.ComponentType<BaseSocketProps>,
	};			};
	function KeyserverConnectionHandler(props: Props) {			function KeyserverConnectionHandler(props: Props) {
	const { socketComponent: Socket, keyserverID, ...rest } = props;			const { socketComponent: Socket, keyserverID, ...rest } = props;

	const dispatchActionPromise = useDispatchActionPromise();			const dispatchActionPromise = useDispatchActionPromise();
	const callLogOut = useLogOut();			const callLogOut = useLogOut();
				const keyserverAuth = useKeyserverAuth();

	const hasConnectionIssue = useSelector(			const hasConnectionIssue = useSelector(
	state => !!connectionSelector(keyserverID)(state)?.connectionIssue,			state => !!connectionSelector(keyserverID)(state)?.connectionIssue,
	);			);
	const cookie = useSelector(cookieSelector(keyserverID));			const cookie = useSelector(cookieSelector(keyserverID));

				const keyserverDeviceToken = useSelector(deviceTokenSelector(keyserverID));
				// We have an assumption that we should be always connected to Ashoat's
				// keyserver. It is possible that a token which it has is correct, so we can
				// try to use it. In worst case it is invalid and our push-handler will try
				// to fix it.
				ashoatUnsubmitted Not Done Inline Actions This logic is a bit confusing. I guess the reason we decided to store a separate `deviceToken` per keyserver is because we want to know if we've shared our `deviceToken` to that keyserver yet, not because we expect to have a different `deviceToken` per keyserver. It would probably make more sense if we have a `deviceTokenHasBeenUploaded: boolean` in `KeyserverInfo`, and a `deviceToken` field at the top level of Redux. But we'll likely end up removing `deviceToken`s from keyservers soon anyways... ashoat: This logic is a bit confusing. I guess the reason we decided to store a separate `deviceToken`…
				tomekAuthorUnsubmitted Done Inline Actions It would probably make more sense if we have a deviceTokenHasBeenUploaded: boolean in KeyserverInfo, and a deviceToken field at the top level of Redux. But we'll likely end up removing deviceTokens from keyservers soon anyways... Agree. But moving the token to Tunnelbroker will simplify it even more. For now, I don't think it is worth fixing now. tomek: > It would probably make more sense if we have a deviceTokenHasBeenUploaded: boolean in…
				const ashoatKeyserverDeviceToken = useSelector(
				deviceTokenSelector(ashoatKeyserverID),
				);
				const deviceToken = keyserverDeviceToken ?? ashoatKeyserverDeviceToken;

				const navInfo = useSelector(state => state.navInfo);
				const calendarFilters = useSelector(state => state.calendarFilters);
				const calendarQuery = React.useMemo(() => {
				const filters = filterThreadIDsInFilterList(
				calendarFilters,
				(threadID: string) => extractKeyserverIDFromID(threadID) === keyserverID,
				);
				return {
				startDate: navInfo.startDate,
				endDate: navInfo.endDate,
				filters,
				};
				}, [calendarFilters, keyserverID, navInfo.endDate, navInfo.startDate]);

	React.useEffect(() => {			React.useEffect(() => {
	if (hasConnectionIssue) {			if (hasConnectionIssue) {
	void dispatchActionPromise(logOutActionTypes, callLogOut());			void dispatchActionPromise(logOutActionTypes, callLogOut());
	}			}
	}, [callLogOut, hasConnectionIssue, dispatchActionPromise]);			}, [callLogOut, hasConnectionIssue, dispatchActionPromise]);

	const identityContext = React.useContext(IdentityClientContext);			const identityContext = React.useContext(IdentityClientContext);
	invariant(identityContext, 'Identity context should be set');			invariant(identityContext, 'Identity context should be set');
	const { identityClient } = identityContext;			const { identityClient, getAuthMetadata } = identityContext;

	const olmSessionCreator = React.useContext(OlmSessionCreatorContext);			const olmSessionCreator = React.useContext(OlmSessionCreatorContext);
	invariant(olmSessionCreator, 'Olm session creator should be set');			invariant(olmSessionCreator, 'Olm session creator should be set');

	React.useEffect(() => {			React.useEffect(() => {
	if (!usingCommServicesAccessToken) {			if (!usingCommServicesAccessToken) {
	return;			return;
	}			}

	void (async () => {			void (async () => {
	try {			try {
	const keyserverKeys =			const keyserverKeys =
	await identityClient.getKeyserverKeys(keyserverID);			await identityClient.getKeyserverKeys(keyserverID);

	// eslint-disable-next-line no-unused-vars
	const [notifsSession, contentSession] = await Promise.all([			const [notifsSession, contentSession] = await Promise.all([
	olmSessionCreator.notificationsSessionCreator(			olmSessionCreator.notificationsSessionCreator(
	cookie,			cookie,
	keyserverKeys.identityKeysBlob.notificationIdentityPublicKeys,			keyserverKeys.identityKeysBlob.notificationIdentityPublicKeys,
	keyserverKeys.notifInitializationInfo,			keyserverKeys.notifInitializationInfo,
	keyserverID,			keyserverID,
	),			),
	olmSessionCreator.contentSessionCreator(			olmSessionCreator.contentSessionCreator(
	keyserverKeys.identityKeysBlob.primaryIdentityPublicKeys,			keyserverKeys.identityKeysBlob.primaryIdentityPublicKeys,
	keyserverKeys.contentInitializationInfo,			keyserverKeys.contentInitializationInfo,
	),			),
	]);			]);

				const { userID, deviceID } = await getAuthMetadata();
				invariant(userID, 'userID should be set');
				invariant(deviceID, 'deviceID should be set');

				const deviceTokenUpdateInput = deviceToken
				? { [keyserverID]: { deviceToken } }
				: {};

				await dispatchActionPromise(
				keyserverAuthActionTypes,
				keyserverAuth({
				userID,
				deviceID,
				doNotRegister: false,
				calendarQuery,
				deviceTokenUpdateInput,
				logInActionSource: process.env.BROWSER
				? logInActionSources.keyserverAuthFromWeb
				: logInActionSources.keyserverAuthFromNative,
				keyserverData: {
				[keyserverID]: {
				initialContentEncryptedMessage: contentSession,
				initialNotificationsEncryptedMessage: notifsSession,
				},
				},
				}),
				);
	} catch (e) {			} catch (e) {
	console.log(			console.log(
	`Error getting keys for keyserver with id ${keyserverID}`,			`Error while authenticating to keyserver with id ${keyserverID}`,
	e,			e,
	);			);
	}			}
	})();			})();
	}, [keyserverID, identityClient, olmSessionCreator, cookie]);			}, [
				keyserverID,
				identityClient,
				olmSessionCreator,
				cookie,
				getAuthMetadata,
				dispatchActionPromise,
				keyserverAuth,
				deviceToken,
				calendarQuery,
				]);

	if (keyserverID !== ashoatKeyserverID) {			if (keyserverID !== ashoatKeyserverID) {
	return null;			return null;
	}			}
	return <Socket {...rest} />;			return <Socket {...rest} />;
	}			}

	const Handler: React.ComponentType<Props> = React.memo<Props>(			const Handler: React.ComponentType<Props> = React.memo<Props>(
	KeyserverConnectionHandler,			KeyserverConnectionHandler,
	);			);

	export default Handler;			export default Handler;

[keyserver] create message and sidebar based on cast
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 45486

keyserver/src/responders/farcaster-webhook-responders.js

[keyserver] create message and sidebar based on castClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 45486

keyserver/src/responders/farcaster-webhook-responders.js

[keyserver] create message and sidebar based on cast
ClosedPublic
Actions

Revision Contents
Changeset List