Page MenuHomePhabricator
Differential D4626

[services] consolidate ddb client code, small refactor
ClosedPublic
Actions

Authored by varun on Jul 25 2022, 12:49 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 14, 8:22 AM
Unknown Object (File)
Wed, Apr 9, 10:44 AM
Unknown Object (File)
Tue, Apr 8, 8:31 PM
Unknown Object (File)
Tue, Apr 8, 8:31 PM
Unknown Object (File)
Tue, Apr 8, 8:31 PM
Unknown Object (File)
Tue, Apr 8, 8:30 PM
Unknown Object (File)
Tue, Apr 8, 8:30 PM
Unknown Object (File)
Tue, Apr 8, 8:30 PM
View All Files
Subscribers
• abosh
• adrian
ashoat
atul

Details

Reviewers
tomek
• karol
Commits
rCOMM7c7831094dec: [services] consolidate ddb client code, small refactor
Summary

consolidated some of the client code to minimize the number of calls we make to DynamoDB. also changed the PAKE_USER_ prefix to USER in constants.rs since the table holds more than just PAKE data now.

Test Plan

cargo build, tested that the update expression worked by creating, updating an item in the DDB table

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun created this revision.Jul 25 2022, 12:49 PM
Herald added subscribers: • abosh, atul, • adrian, ashoat. · View Herald TranscriptJul 25 2022, 12:49 PM
Harbormaster completed remote builds in B10827: Diff 14890.Jul 25 2022, 1:02 PM
varun requested review of this revision.Jul 25 2022, 1:02 PM
varun updated this revision to Diff 14893.Jul 25 2022, 1:27 PM

fix update expression syntax

Harbormaster completed remote builds in B10830: Diff 14893.Jul 25 2022, 1:37 PM
varun added a child revision: D4628: [services] use username and public key from registration request.Jul 25 2022, 1:39 PM
• karol accepted this revision.Jul 26 2022, 6:09 AM
• karol added inline comments.
services/identity/src/database.rs
84–86 ↗(On Diff #14893)

Clever solution, nice.

125 ↗(On Diff #14893)

How does it work? I mean, don't we need some sort of delimiter for multiple ops?

tomek accepted this revision.Jul 26 2022, 6:38 AM
tomek added inline comments.
services/identity/src/database.rs
90–98 ↗(On Diff #14893)

When only one arm of match expression does useful job, we should use if let instead

This revision is now accepted and ready to land.Jul 26 2022, 6:38 AM
varun added inline comments.Jul 26 2022, 8:38 AM
services/identity/src/database.rs
90–98 ↗(On Diff #14893)

will fix before landing

125 ↗(On Diff #14893)

I originally thought we needed a comma delimiter but turns out the keyword SET is enough for the parser to split up the expression

varun updated this revision to Diff 14993.Jul 26 2022, 10:40 AM

address feedback

This revision was landed with ongoing or failed builds.Jul 26 2022, 10:41 AM
Closed by commit rCOMM7c7831094dec: [services] consolidate ddb client code, small refactor (authored by varun). · Explain Why
This revision was automatically updated to reflect the committed changes.
varun added a commit: rCOMM7c7831094dec: [services] consolidate ddb client code, small refactor.
Harbormaster completed remote builds in B10897: Diff 14993.Jul 26 2022, 10:53 AM

Revision Contents
Changeset List

PathSize
services/
identity/
src/
10 lines
93 lines
6 lines

Diff 14993

View Options

services/identity/src/constants.rs

// @flow // @flow
import invariant from 'invariant'; import invariant from 'invariant';
import * as React from 'react'; import * as React from 'react';
import { View } from 'react-native'; import { View } from 'react-native';
import { ScrollView } from 'react-native-gesture-handler'; import { ScrollView } from 'react-native-gesture-handler';
import { fetchPinnedMessages } from 'lib/actions/message-actions.js'; import { fetchPinnedMessages } from 'lib/actions/message-actions.js';
import { messageListData } from 'lib/selectors/chat-selectors.js'; import { messageListData } from 'lib/selectors/chat-selectors.js';
import { createMessageInfo } from 'lib/shared/message-utils.js'; import { createMessageInfo } from 'lib/shared/message-utils.js';
import { isComposableMessageType } from 'lib/types/message-types.js';
import type { ThreadInfo } from 'lib/types/thread-types.js'; import type { ThreadInfo } from 'lib/types/thread-types.js';
import { useServerCall } from 'lib/utils/action-utils.js'; import { useServerCall } from 'lib/utils/action-utils.js';
import { useHeightMeasurer } from './chat-context.js'; import { useHeightMeasurer } from './chat-context.js';
import type { ChatNavigationProp } from './chat.react'; import type { ChatNavigationProp } from './chat.react';
import MessageResult from './message-result.react.js'; import MessageResult from './message-result.react.js';
import type { NavigationRoute } from '../navigation/route-names'; import type { NavigationRoute } from '../navigation/route-names';
import { useSelector } from '../redux/redux-utils.js'; import { useSelector } from '../redux/redux-utils.js';
▲ Show 20 Lines • Show All 46 Lines • ▼ Show 20 Lines const chatMessageInfos = useSelector(
messageListData(threadInfo.id, translatedMessageResults), messageListData(threadInfo.id, translatedMessageResults),
); );
const sortedUniqueChatMessageInfoItems = React.useMemo(() => { const sortedUniqueChatMessageInfoItems = React.useMemo(() => {
if (!chatMessageInfos) { if (!chatMessageInfos) {
return []; return [];
} }
const pinnedMessageIDs = new Set(); const pinnedMessageIDs = new Set();
translatedMessageResults.forEach(item => pinnedMessageIDs.add(item.id)); translatedMessageResults.forEach(item => pinnedMessageIDs.add(item.id));
inkaUnsubmitted
Not Done
Inline Actions

We could probably go back to using isPinned property here. The thing we undid in D8097. This would make the code more readable, but it's up to you

inka: We could probably go back to using `isPinned` property here. The thing we undid in D8097. This…
const chatMessageInfoItems = chatMessageInfos.filter( const chatMessageInfoItems = chatMessageInfos.filter(
item => item =>
item.itemType === 'message' && item.itemType === 'message' &&
pinnedMessageIDs.has(item.messageInfo.id), pinnedMessageIDs.has(item.messageInfo.id) &&
isComposableMessageType(item.messageInfo.type),
rohanAuthorUnsubmitted
Done
Inline Actions

As mentioned in the description, this is because toggle_pin messages are visible robotext, so if we don't filter those out then they will become visible in the message results modal. This is done here, after the ChatMessageInfos have been created, so we can accurately use the updated fetched related messages to determine the isPinned flag. If we filtered this earlier, then we'd be in the same situation as before

This also is fine because we only allow the user to pin composable message types so we won't exclude anything from the view that we need to show.

rohan: As mentioned in the description, this is because toggle_pin messages are visible robotext, so…
); );
// By the nature of using messageListData and passing in // By the nature of using messageListData and passing in
// the desired translatedMessageResults as additional // the desired translatedMessageResults as additional
// messages, we will have duplicate ChatMessageInfoItems. // messages, we will have duplicate ChatMessageInfoItems.
const uniqueChatMessageInfoItemsMap = new Map(); const uniqueChatMessageInfoItemsMap = new Map();
chatMessageInfoItems.forEach( chatMessageInfoItems.forEach(
item => item =>
▲ Show 20 Lines • Show All 93 Lines • Show Last 20 Lines
View Options

services/identity/src/database.rs

// @flow // @flow
import invariant from 'invariant'; import invariant from 'invariant';
import * as React from 'react'; import * as React from 'react';
import { View } from 'react-native'; import { View } from 'react-native';
import { ScrollView } from 'react-native-gesture-handler'; import { ScrollView } from 'react-native-gesture-handler';
import { fetchPinnedMessages } from 'lib/actions/message-actions.js'; import { fetchPinnedMessages } from 'lib/actions/message-actions.js';
import { messageListData } from 'lib/selectors/chat-selectors.js'; import { messageListData } from 'lib/selectors/chat-selectors.js';
import { createMessageInfo } from 'lib/shared/message-utils.js'; import { createMessageInfo } from 'lib/shared/message-utils.js';
import { isComposableMessageType } from 'lib/types/message-types.js';
import type { ThreadInfo } from 'lib/types/thread-types.js'; import type { ThreadInfo } from 'lib/types/thread-types.js';
import { useServerCall } from 'lib/utils/action-utils.js'; import { useServerCall } from 'lib/utils/action-utils.js';
import { useHeightMeasurer } from './chat-context.js'; import { useHeightMeasurer } from './chat-context.js';
import type { ChatNavigationProp } from './chat.react'; import type { ChatNavigationProp } from './chat.react';
import MessageResult from './message-result.react.js'; import MessageResult from './message-result.react.js';
import type { NavigationRoute } from '../navigation/route-names'; import type { NavigationRoute } from '../navigation/route-names';
import { useSelector } from '../redux/redux-utils.js'; import { useSelector } from '../redux/redux-utils.js';
▲ Show 20 Lines • Show All 46 Lines • ▼ Show 20 Lines const chatMessageInfos = useSelector(
messageListData(threadInfo.id, translatedMessageResults), messageListData(threadInfo.id, translatedMessageResults),
); );
const sortedUniqueChatMessageInfoItems = React.useMemo(() => { const sortedUniqueChatMessageInfoItems = React.useMemo(() => {
if (!chatMessageInfos) { if (!chatMessageInfos) {
return []; return [];
} }
const pinnedMessageIDs = new Set(); const pinnedMessageIDs = new Set();
translatedMessageResults.forEach(item => pinnedMessageIDs.add(item.id)); translatedMessageResults.forEach(item => pinnedMessageIDs.add(item.id));
inkaUnsubmitted
Not Done
Inline Actions

We could probably go back to using isPinned property here. The thing we undid in D8097. This would make the code more readable, but it's up to you

inka: We could probably go back to using `isPinned` property here. The thing we undid in D8097. This…
const chatMessageInfoItems = chatMessageInfos.filter( const chatMessageInfoItems = chatMessageInfos.filter(
item => item =>
item.itemType === 'message' && item.itemType === 'message' &&
pinnedMessageIDs.has(item.messageInfo.id), pinnedMessageIDs.has(item.messageInfo.id) &&
isComposableMessageType(item.messageInfo.type),
rohanAuthorUnsubmitted
Done
Inline Actions

As mentioned in the description, this is because toggle_pin messages are visible robotext, so if we don't filter those out then they will become visible in the message results modal. This is done here, after the ChatMessageInfos have been created, so we can accurately use the updated fetched related messages to determine the isPinned flag. If we filtered this earlier, then we'd be in the same situation as before

This also is fine because we only allow the user to pin composable message types so we won't exclude anything from the view that we need to show.

rohan: As mentioned in the description, this is because toggle_pin messages are visible robotext, so…
); );
// By the nature of using messageListData and passing in // By the nature of using messageListData and passing in
// the desired translatedMessageResults as additional // the desired translatedMessageResults as additional
// messages, we will have duplicate ChatMessageInfoItems. // messages, we will have duplicate ChatMessageInfoItems.
const uniqueChatMessageInfoItemsMap = new Map(); const uniqueChatMessageInfoItemsMap = new Map();
chatMessageInfoItems.forEach( chatMessageInfoItems.forEach(
item => item =>
▲ Show 20 Lines • Show All 93 Lines • Show Last 20 Lines
View Options

services/identity/src/service.rs

// @flow // @flow
import invariant from 'invariant'; import invariant from 'invariant';
import * as React from 'react'; import * as React from 'react';
import { View } from 'react-native'; import { View } from 'react-native';
import { ScrollView } from 'react-native-gesture-handler'; import { ScrollView } from 'react-native-gesture-handler';
import { fetchPinnedMessages } from 'lib/actions/message-actions.js'; import { fetchPinnedMessages } from 'lib/actions/message-actions.js';
import { messageListData } from 'lib/selectors/chat-selectors.js'; import { messageListData } from 'lib/selectors/chat-selectors.js';
import { createMessageInfo } from 'lib/shared/message-utils.js'; import { createMessageInfo } from 'lib/shared/message-utils.js';
import { isComposableMessageType } from 'lib/types/message-types.js';
import type { ThreadInfo } from 'lib/types/thread-types.js'; import type { ThreadInfo } from 'lib/types/thread-types.js';
import { useServerCall } from 'lib/utils/action-utils.js'; import { useServerCall } from 'lib/utils/action-utils.js';
import { useHeightMeasurer } from './chat-context.js'; import { useHeightMeasurer } from './chat-context.js';
import type { ChatNavigationProp } from './chat.react'; import type { ChatNavigationProp } from './chat.react';
import MessageResult from './message-result.react.js'; import MessageResult from './message-result.react.js';
import type { NavigationRoute } from '../navigation/route-names'; import type { NavigationRoute } from '../navigation/route-names';
import { useSelector } from '../redux/redux-utils.js'; import { useSelector } from '../redux/redux-utils.js';
▲ Show 20 Lines • Show All 46 Lines • ▼ Show 20 Lines const chatMessageInfos = useSelector(
messageListData(threadInfo.id, translatedMessageResults), messageListData(threadInfo.id, translatedMessageResults),
); );
const sortedUniqueChatMessageInfoItems = React.useMemo(() => { const sortedUniqueChatMessageInfoItems = React.useMemo(() => {
if (!chatMessageInfos) { if (!chatMessageInfos) {
return []; return [];
} }
const pinnedMessageIDs = new Set(); const pinnedMessageIDs = new Set();
translatedMessageResults.forEach(item => pinnedMessageIDs.add(item.id)); translatedMessageResults.forEach(item => pinnedMessageIDs.add(item.id));
inkaUnsubmitted
Not Done
Inline Actions

We could probably go back to using isPinned property here. The thing we undid in D8097. This would make the code more readable, but it's up to you

inka: We could probably go back to using `isPinned` property here. The thing we undid in D8097. This…
const chatMessageInfoItems = chatMessageInfos.filter( const chatMessageInfoItems = chatMessageInfos.filter(
item => item =>
item.itemType === 'message' && item.itemType === 'message' &&
pinnedMessageIDs.has(item.messageInfo.id), pinnedMessageIDs.has(item.messageInfo.id) &&
isComposableMessageType(item.messageInfo.type),
rohanAuthorUnsubmitted
Done
Inline Actions

As mentioned in the description, this is because toggle_pin messages are visible robotext, so if we don't filter those out then they will become visible in the message results modal. This is done here, after the ChatMessageInfos have been created, so we can accurately use the updated fetched related messages to determine the isPinned flag. If we filtered this earlier, then we'd be in the same situation as before

This also is fine because we only allow the user to pin composable message types so we won't exclude anything from the view that we need to show.

rohan: As mentioned in the description, this is because toggle_pin messages are visible robotext, so…
); );
// By the nature of using messageListData and passing in // By the nature of using messageListData and passing in
// the desired translatedMessageResults as additional // the desired translatedMessageResults as additional
// messages, we will have duplicate ChatMessageInfoItems. // messages, we will have duplicate ChatMessageInfoItems.
const uniqueChatMessageInfoItemsMap = new Map(); const uniqueChatMessageInfoItemsMap = new Map();
chatMessageInfoItems.forEach( chatMessageInfoItems.forEach(
item => item =>
▲ Show 20 Lines • Show All 93 Lines • Show Last 20 Lines
View Options

services/identity/src/users.rs

// @flow // @flow
import invariant from 'invariant'; import invariant from 'invariant';
import * as React from 'react'; import * as React from 'react';
import { View } from 'react-native'; import { View } from 'react-native';
import { ScrollView } from 'react-native-gesture-handler'; import { ScrollView } from 'react-native-gesture-handler';
import { fetchPinnedMessages } from 'lib/actions/message-actions.js'; import { fetchPinnedMessages } from 'lib/actions/message-actions.js';
import { messageListData } from 'lib/selectors/chat-selectors.js'; import { messageListData } from 'lib/selectors/chat-selectors.js';
import { createMessageInfo } from 'lib/shared/message-utils.js'; import { createMessageInfo } from 'lib/shared/message-utils.js';
import { isComposableMessageType } from 'lib/types/message-types.js';
import type { ThreadInfo } from 'lib/types/thread-types.js'; import type { ThreadInfo } from 'lib/types/thread-types.js';
import { useServerCall } from 'lib/utils/action-utils.js'; import { useServerCall } from 'lib/utils/action-utils.js';
import { useHeightMeasurer } from './chat-context.js'; import { useHeightMeasurer } from './chat-context.js';
import type { ChatNavigationProp } from './chat.react'; import type { ChatNavigationProp } from './chat.react';
import MessageResult from './message-result.react.js'; import MessageResult from './message-result.react.js';
import type { NavigationRoute } from '../navigation/route-names'; import type { NavigationRoute } from '../navigation/route-names';
import { useSelector } from '../redux/redux-utils.js'; import { useSelector } from '../redux/redux-utils.js';
▲ Show 20 Lines • Show All 46 Lines • ▼ Show 20 Lines const chatMessageInfos = useSelector(
messageListData(threadInfo.id, translatedMessageResults), messageListData(threadInfo.id, translatedMessageResults),
); );
const sortedUniqueChatMessageInfoItems = React.useMemo(() => { const sortedUniqueChatMessageInfoItems = React.useMemo(() => {
if (!chatMessageInfos) { if (!chatMessageInfos) {
return []; return [];
} }
const pinnedMessageIDs = new Set(); const pinnedMessageIDs = new Set();
translatedMessageResults.forEach(item => pinnedMessageIDs.add(item.id)); translatedMessageResults.forEach(item => pinnedMessageIDs.add(item.id));
inkaUnsubmitted
Not Done
Inline Actions

We could probably go back to using isPinned property here. The thing we undid in D8097. This would make the code more readable, but it's up to you

inka: We could probably go back to using `isPinned` property here. The thing we undid in D8097. This…
const chatMessageInfoItems = chatMessageInfos.filter( const chatMessageInfoItems = chatMessageInfos.filter(
item => item =>
item.itemType === 'message' && item.itemType === 'message' &&
pinnedMessageIDs.has(item.messageInfo.id), pinnedMessageIDs.has(item.messageInfo.id) &&
isComposableMessageType(item.messageInfo.type),
rohanAuthorUnsubmitted
Done
Inline Actions

As mentioned in the description, this is because toggle_pin messages are visible robotext, so if we don't filter those out then they will become visible in the message results modal. This is done here, after the ChatMessageInfos have been created, so we can accurately use the updated fetched related messages to determine the isPinned flag. If we filtered this earlier, then we'd be in the same situation as before

This also is fine because we only allow the user to pin composable message types so we won't exclude anything from the view that we need to show.

rohan: As mentioned in the description, this is because toggle_pin messages are visible robotext, so…
); );
// By the nature of using messageListData and passing in // By the nature of using messageListData and passing in
// the desired translatedMessageResults as additional // the desired translatedMessageResults as additional
// messages, we will have duplicate ChatMessageInfoItems. // messages, we will have duplicate ChatMessageInfoItems.
const uniqueChatMessageInfoItemsMap = new Map(); const uniqueChatMessageInfoItemsMap = new Map();
chatMessageInfoItems.forEach( chatMessageInfoItems.forEach(
item => item =>
▲ Show 20 Lines • Show All 93 Lines • Show Last 20 Lines