From 65d97469bbe60be0cae129091a4632d59689810d Mon Sep 17 00:00:00 2001 From: qialex Date: Thu, 17 Oct 2024 13:06:03 +0200 Subject: [PATCH] initial proposal for threads implementation --- feature-threads.md | 95 +++++++++++++++++++ .../components/Conversation/Conversation.tsx | 9 +- .../Message/ContentMessage/ContentMessage.tsx | 10 ++ .../MessageActions/MessageActions.tsx | 5 + src/script/conversation/EventBuilder.ts | 7 +- src/script/entity/Conversation.ts | 15 +++ src/script/event/Client.ts | 4 + .../ConversationTabs/ConversationTabs.tsx | 4 + .../panels/Conversations/Conversations.tsx | 6 +- .../util/LocalizerUtil/LocalizerUtil.ts | 5 +- 10 files changed, 156 insertions(+), 4 deletions(-) create mode 100644 feature-threads.md diff --git a/feature-threads.md b/feature-threads.md new file mode 100644 index 00000000000..36100a7e642 --- /dev/null +++ b/feature-threads.md @@ -0,0 +1,95 @@ +## Reflecting challenge conditions +> Giving yourself a timebox of 4-6 hours (at most), investigate how the webapp is currently handling replies and come up with a solution for how to implement threading given the current architecture. Some code examples or other visualization may be helpful to properly explain your thought process. + +At first I explored +1. https://github.com/wireapp/wire-webapp/issues +2. https://github.com/wireapp/wire-webapp/pulls + +I checked `EventBuilder` https://github.com/wireapp/wire-webapp/blob/dev/src/script/conversation/EventBuilder.ts#L128 - It's quite clear, some common structure and some event type. + +I tried to find some features requests related to **threads** or may be channels. I saw that repository exist from at least 2016 and was surprised that there were no requests about **threads** + +I quickly figured out that `Conversation` is one of the backbone. It's a `Conversation` when it's 1:1 or in group, it's the same entity, just different `CONVERSATION_TYPE`. **Threads** also will be a new type of `Conversation`. slightly different but the same in common. + +To create a **Thread** we minimally need 3 things: +1. **Thread** technically will be a new `Conversation` with a new type. +2. We need a `quoted message` that contains a link to this **Thread** +3. **Thread** will contain a link to the quoted message + +At the same moment when we are able to create quoted message we need to have an option to create a linked `Conversation` type **Thread** + +## Code point of view + +From technical point of view there is a need for at least +1. New menu item **Threads** in a sidebar menu +2. New `CONVERSATION_TYPE` +3. Represent those new type of conversations in `Conversations` list +4. create new action for creating this new type of `Conversation` +5. We need some alteration of a single message so it could be linked to a **Thread** +6. This also will trigger some changes in backend and other files to provide an opportunity for this +7. Update some backend part to be ready to handle those new type of conversations +8. Update single `Conversation` view to be capable of representing new type of conversation +9. Update behaviour of `Message actions` +10. Update localisation + +I put `TODO: ` comments in corresponding parts of the app + + + + +## Arised qustions + +### User experience +1. will we keep current **reply** and add brand new **reply in a thread** or we will only replace old **reply** with a new **reply in thread** ? +2. Will we give an option to a user to change `ConversationDetails` for a `Conversation` with type `thread`, or we will inherit all the settings from a parent `Conversation`? +3. Is it possible to create a thread within another thread (nested thread)? +4. Is it possible to invite users in a thread if they are not invited to a parent `Conversation` +5. Is it possible to change notifications or other setting in a thread only or in a parent only? or changing some settings in a parent will trigger same change in setting of a thread conversation? + +I think this questions are debatable and at this moment I don't have a strong opinion on that. +Personally I would prefer keeping existing quotes functionality and add a brand new threads. + +### Technical +From https://datatracker.ietf.org/doc/html/rfc9420#name-required-capabilities: + +>At a minimum, all members of the group need to support the cipher suite and protocol version in use." - I think this is about backward compatibility between different versions of the product/APIs etc. + +From https://datatracker.ietf.org/doc/html/rfc9420#name-reinitialization + +>A group may be reinitialized by creating a new group with the same membership and different parameters, and linking it to the old group via a resumption PSK. + +As we are introducing a new technical case **linked conversation**, we need to + +1. Convey a way of handling `ConversationDetails` +2. Ensure that parent `Conversation` and **Thread** `Conversation` using compatible protocol/version + +I started to explore dependencies and was amazed by "Architecture overview" from https://github.com/wireapp/wire-avs but after some exploring I have some open questions + +1. If we need to reinitialize conversations - will we reinitialize all parent and child conversations at the same time? +2. Is it possible that parent and thread conversation support different "protocol/version"? + +## Conclusion + +1. As a complex feature it requires some debates / collaboration +2. Some technical parts are already nailed down such as new `CONVERSATION_TYPE` alterations and some other parts of the code +3. Some user experience questions needs to be decided such as: Can we invite a user into a **Thread** which is not invited in parent conversation? or: Is it possible to have nested thread? Answers to this question will influence feature implementation +4. Some such as compatible protocol/version, reinitialization needs to be communicated with other departments or require more time for investigation + + +## Other thoughts + +1. I explored https://datatracker.ietf.org/doc/html/rfc9420#name-group-creation - I'm very much excited about MLS. Such an amazing structure, I'd happy working on being part of developing following such a high standards. +2. https://wire.com/en/blog/admin-privilege-model-is-broken - When it comes to sensitive data I very much like and support "Zero trust" idea +3. I started to explore dependencies and was amazed by "Architecture overview" from https://github.com/wireapp/wire-avs + +## Other ideas notes and improvement suggestion + +1. Explored little bit a group creating thinking about channels creation which are also yet not implemented `src/script/components/Modals/GroupCreation/GroupCreationModal.tsx` +2. Add some useful information to the `Conversations` displaying for example some part of the last message +3. Split settings->options into **Appearance**, **Notifications** and **Sounds** +4. Move **Set a profile color** from a **Profile** to newly created **appearance** tab. +5. Give more deep settings in newly created **sounds** tab. Different sounds on different events, or customised sounds for example +6. Setup different notifications on different users/groups/1:1/threads, for example I want to snooze notification from users a,b,c but have notifications from users d,e,f +7. In `Notifications` `src/script/page/RightSidebar/Notifications/Notifications.tsx` setting give user more options for more granular settings +8. Add icons in `Context menu` (`src/script/ui/ContextMenu.tsx`) +9. Sometimes it was a bit not intuitive to figure out without reading articles https://support.wire.com/hc/en-us/articles/360002855557-Add-a-conversation-to-your-favourites-folder - I believe in a good interface everything should be clear without it. \ No newline at end of file diff --git a/src/script/components/Conversation/Conversation.tsx b/src/script/components/Conversation/Conversation.tsx index c779fc93a67..b9c03b7e56b 100644 --- a/src/script/components/Conversation/Conversation.tsx +++ b/src/script/components/Conversation/Conversation.tsx @@ -76,7 +76,14 @@ interface ConversationProps { } const CONFIG = Config.getConfig(); - +/* + * Alex Ishenko + * This file is responsible for represenation of all kinds of conversations, + * This file will require 2 different major updates + * TODO: 1. change display of messages with threads so that user can see other users who participating in a thread, number of messages, link to open thread + * TODO: 2. This component will be responsible for displaying thread conversation itself. add relevant to thread functionality such as changing header maybe displaying entire original message, but it's debbatable + * TODO: split this into smaller parts + */ export const Conversation = ({ teamState, selfUser, diff --git a/src/script/components/MessagesList/Message/ContentMessage/ContentMessage.tsx b/src/script/components/MessagesList/Message/ContentMessage/ContentMessage.tsx index 52c2ee7aa1c..30e9daaa2a4 100644 --- a/src/script/components/MessagesList/Message/ContentMessage/ContentMessage.tsx +++ b/src/script/components/MessagesList/Message/ContentMessage/ContentMessage.tsx @@ -66,6 +66,16 @@ export interface ContentMessageProps extends Omit void; onRetry: (message: ContentMessage) => void; + /* + * Alex Ishenko + * This is the quotedMessage functionality, + * TODO: we will need to refactor this adding OR replacing current functionality + * currently message can have a quotedMessage wich is a link to a parent message + * but for threads we will need a link to a Conversation + * may be this will be a replacement for a quotedMessage + * may be this will be a separate functionality see more in feature-threads.md + * this anyway will trigger some changes in other files which are handling adding message + */ quotedMessage?: ContentMessage; selfId: QualifiedId; isMsgElementsFocusable: boolean; diff --git a/src/script/components/MessagesList/Message/ContentMessage/MessageActions/MessageActions.tsx b/src/script/components/MessagesList/Message/ContentMessage/MessageActions/MessageActions.tsx index 7f9fca5ac41..88053ce5daa 100644 --- a/src/script/components/MessagesList/Message/ContentMessage/MessageActions/MessageActions.tsx +++ b/src/script/components/MessagesList/Message/ContentMessage/MessageActions/MessageActions.tsx @@ -152,6 +152,11 @@ const MessageActionsMenu: FC = ({ [currentMsgActionName, handleMenuOpen], ); + /* + * Alex Ishenko + * This is the reply functionality, + * TODO: we need "Reply in Thread" button which will create a new converstaion + */ const handleMessageReply = useCallback( (event: React.MouseEvent) => { toggleActiveMenu(event); diff --git a/src/script/conversation/EventBuilder.ts b/src/script/conversation/EventBuilder.ts index 8c0b3ba465f..172fe4e313b 100644 --- a/src/script/conversation/EventBuilder.ts +++ b/src/script/conversation/EventBuilder.ts @@ -436,7 +436,12 @@ export const EventBuilder = { type: ClientEvent.CONVERSATION.FILE_TYPE_RESTRICTED, }; }, - + /* + * Alex Ishenko + * TODO: create buildThreadCreation + * that will create thread using newly added + * type: ClientEvent.CONVERSATION.THREAD_CREATION, from @wireapp/api-client/lib/conversation/Conversation.d.ts + */ buildGroupCreation( conversationEntity: Conversation, isTemporaryGuest: boolean = false, diff --git a/src/script/entity/Conversation.ts b/src/script/entity/Conversation.ts index f87c52f2fcf..ae2e9d59647 100644 --- a/src/script/entity/Conversation.ts +++ b/src/script/entity/Conversation.ts @@ -83,6 +83,10 @@ enum TIMESTAMP_TYPE { MUTED = 'mutedTimestamp', } +/* + * Alex Ishenko + * Wow! + */ export class Conversation { private readonly teamState: TeamState; public readonly archivedState: ko.Observable; @@ -172,6 +176,11 @@ export class Conversation { public readonly showNotificationsNothing: ko.PureComputed; public status: ko.Observable; public teamId: string; + /* + * Alex Ishenko + * TODO: add new CONVERSATION_TYPE called THREAD + * node_modules/@wireapp/api-client/lib/conversation/Conversation.d.ts + */ public readonly type: ko.Observable; public readonly unreadState: ko.PureComputed; public readonly verification_state = ko.observable(ConversationVerificationState.UNVERIFIED); @@ -182,6 +191,12 @@ export class Conversation { public accessModes?: CONVERSATION_ACCESS[]; public accessRole?: CONVERSATION_LEGACY_ACCESS_ROLE | CONVERSATION_ACCESS_ROLE[]; public domain: string; + /* + * Alex Ishenko + * TODO: Add some more properties depends on feature requirements. for example link to message + * May be name and/or display_name are enough to display the proper header for a thread, may be not + * this may cause also adding relevant functionality in other places and affect API + */ static get TIMESTAMP_TYPE(): typeof TIMESTAMP_TYPE { return TIMESTAMP_TYPE; diff --git a/src/script/event/Client.ts b/src/script/event/Client.ts index 68dc6fd1246..88d7c47c9a7 100644 --- a/src/script/event/Client.ts +++ b/src/script/event/Client.ts @@ -32,6 +32,10 @@ export enum CONVERSATION { DELETE_EVERYWHERE = 'conversation.delete-everywhere', FILE_TYPE_RESTRICTED = 'conversation.file-type-restricted', GROUP_CREATION = 'conversation.group-creation', + /* + * Alex Ishenko + * TODO: add THREAD_CREATION + */ INCOMING_MESSAGE_TOO_BIG = 'conversation.incoming-message-too-big', KNOCK = 'conversation.knock', LEGAL_HOLD_UPDATE = 'conversation.legal-hold-update', diff --git a/src/script/page/LeftSidebar/panels/Conversations/ConversationTabs/ConversationTabs.tsx b/src/script/page/LeftSidebar/panels/Conversations/ConversationTabs/ConversationTabs.tsx index a56b1a9cd84..ca88548cd64 100644 --- a/src/script/page/LeftSidebar/panels/Conversations/ConversationTabs/ConversationTabs.tsx +++ b/src/script/page/LeftSidebar/panels/Conversations/ConversationTabs/ConversationTabs.tsx @@ -101,6 +101,10 @@ export const ConversationTabs = ({ Icon: , unreadConversations: groupConversations.filter(filterUnreadAndArchivedConversations).length, }, + /* + * Alex Ishenko + * TODO: Add menu item for threads + */ { type: SidebarTabs.DIRECTS, title: t('conversationLabelDirects'), diff --git a/src/script/page/LeftSidebar/panels/Conversations/Conversations.tsx b/src/script/page/LeftSidebar/panels/Conversations/Conversations.tsx index 0be6cf4f9b8..595b21d9633 100644 --- a/src/script/page/LeftSidebar/panels/Conversations/Conversations.tsx +++ b/src/script/page/LeftSidebar/panels/Conversations/Conversations.tsx @@ -86,7 +86,11 @@ type ConversationsProps = { teamRepository: TeamRepository; userRepository: UserRepository; }; - +/* + * Alex Ishenko + * TODO: split this into smaller parts + * TODO: add relevant to threads functionality such as displaying threads + */ const Conversations: React.FC = ({ integrationRepository, searchRepository, diff --git a/src/script/util/LocalizerUtil/LocalizerUtil.ts b/src/script/util/LocalizerUtil/LocalizerUtil.ts index 9d10bcb0789..e897d3fbeb7 100644 --- a/src/script/util/LocalizerUtil/LocalizerUtil.ts +++ b/src/script/util/LocalizerUtil/LocalizerUtil.ts @@ -134,7 +134,10 @@ export const setLocale = (newLocale: string): void => { export const setStrings = (newStrings: typeof strings): void => { strings = newStrings; }; - +/* + * Alex Ishenko + * TODO: add all relevant translations and new strings + */ export function t( identifier: StringIdentifer, substitutions?: Substitutes,