feat: implement retry option on message failure to send [FS-1571] #14757

V-Gira · 2023-02-27T13:51:27Z

FS-1571 [web] implement retry mechanism for messages that have completely failed to send

Feature

implement an option to retry sending a message if sending failed
implementation with this PR only works with text messages, asset messages display a discard button instead

… non functional discard button for assets

… method

atomrc · 2023-03-09T06:43:48Z

src/script/conversation/MessageRepository.ts

      quote: quoteEntity,
+      messageId: messageId ? messageId : createRandomUuid(), // We set the id explicitely in order to be able to override the message if we generate a link preview


Suggested change

messageId: messageId ? messageId : createRandomUuid(), // We set the id explicitely in order to be able to override the message if we generate a link preview

messageId: messageId ?? createRandomUuid(), // We set the id explicitely in order to be able to override the message if we generate a link preview

tried to put a more appropriate comment too

codecov · 2023-03-11T15:49:44Z

Codecov Report

Merging #14757 (c2852d5) into dev (0267539) will decrease coverage by 0.10%.
The diff coverage is 29.85%.

@@            Coverage Diff             @@
##              dev   #14757      +/-   ##
==========================================
- Coverage   42.85%   42.76%   -0.10%     
==========================================
  Files         621      622       +1     
  Lines       21212    21265      +53     
  Branches     4868     4882      +14     
==========================================
+ Hits         9090     9093       +3     
- Misses      10965    11011      +46     
- Partials     1157     1161       +4

atomrc

Super solid PR, Just one thing that is slightly misplace but overall great job @V-Gira

atomrc · 2023-03-13T08:07:37Z

src/script/components/MessagesList/Message/ContentMessage/ContentMessage.tsx

+  const mainViewModel = useMainViewModel();
+  const {content: contentViewModel} = mainViewModel;
+  const {messageRepository} = contentViewModel;
+
+  const handleRetrySending = async () => {
+    const firstAsset = message.getFirstAsset();
+
+    if (firstAsset instanceof Text) {
+      const messageId = message.id;
+      const messageText = message.getFirstAsset().text;
+      const mentions: MentionEntity[] = firstAsset.mentions() && firstAsset.mentions();
+      const incomingQuote = message.quote();
+
+      const isOutgoingQuote = (quoteEntity: QuoteEntity): quoteEntity is OutgoingQuote => {
+        return quoteEntity.hash !== undefined;
+      };
+
+      const quote: OutgoingQuote | undefined =
+        incomingQuote && isOutgoingQuote(incomingQuote) ? (incomingQuote as OutgoingQuote) : undefined;
+
+      await messageRepository.sendTextWithLinkPreview(conversation, messageText, mentions, quote, messageId);
+    }
+  };
+
+  const handleDiscard = async () => {
+    await messageRepository.deleteMessageById(conversation, message.id);
+  };
+


This logic doesn't really belong there.
The fact that you needed to inject the messageRepository in this component is an indicator that the logic might not be in the correct layer of the app.

If you look at the codebase, you will see that the MessageWrapper already has access to the MessageRepository, so it should be to one implementing retry and discard.

This component should have 2 callbacks as props onRetry and onDiscard and this logic should live in the MessageWrapper

src/script/conversation/MessageRepository.ts

…apper

atomrc · 2023-03-13T14:19:28Z

src/script/components/MessagesList/Message/ContentMessage/ContentMessage.tsx

+            onDiscard={() => {
+              onDiscard();
+            }}


Suggested change

onDiscard={() => {

onDiscard();

}}

onDiscard={onDiscard}

WDYT?

atomrc · 2023-03-13T14:19:46Z

src/script/components/MessagesList/Message/ContentMessage/ContentMessage.tsx

+            onRetry={() => {
+              onRetry(message);
+            }}


Suggested change

onRetry={() => {

onRetry(message);

}}

onRetry={() => onRetry(message)}

WDYT?

forgot to remove dem curly braces, good catch

atomrc

😎 Looking great. Good job man 👏

src/script/components/MessagesList/Message/MessageWrapper.tsx

src/script/conversation/EventMapper.ts

src/script/conversation/MessageRepository.ts

V-Gira added 3 commits February 22, 2023 16:48

feat: implement retry option on message failure to send

7383625

refactor FailToSend to PartialFailureToSend

f6a1ff5

display error message and retry button on complete failure to send

733edd9

otto-the-bot added type: feature / request ✨ 👕 size: M labels Feb 27, 2023

V-Gira added 6 commits February 27, 2023 15:38

correct logic to display gray message for failed status

38f467f

add duplicate event handling for retrying to send a failed message

2e06869

correct typo

545340a

Implement retry option for text messages (no mentions, no quotes) and…

90a45bf

… non functional discard button for assets

move logic to the ContentMessage component, add messageId to sentText…

5ef048f

… method

delete message on discard

f5293e9

V-Gira marked this pull request as ready for review March 7, 2023 11:26

V-Gira requested review from a team and otto-the-bot as code owners March 7, 2023 11:26

V-Gira and others added 3 commits March 7, 2023 12:27

Merge branch 'dev' into virgile/generic-failure-to-send

c17c6fe

add hash to outgoing quote

dcc9fd6

remove unused method in message repo

e21c826

atomrc reviewed Mar 9, 2023

View reviewed changes

pass hash in quoted message middleware

67b35d4

V-Gira marked this pull request as draft March 9, 2023 14:50

V-Gira added 4 commits March 10, 2023 12:25

handle quotes only if they are valid outgoing quotes

be51634

address code review and clarify comment

9777021

fix tests

be2bc3e

remove commented code

15b54d5

V-Gira marked this pull request as ready for review March 11, 2023 16:00

atomrc requested changes Mar 13, 2023

View reviewed changes

V-Gira added 2 commits March 13, 2023 15:00

refactor retry and discard callbacks from ContentMessage to MessageWr…

63d4f01

…apper

remove unused context from tests

038085a

atomrc reviewed Mar 13, 2023

View reviewed changes

atomrc approved these changes Mar 13, 2023

View reviewed changes

remove superfluous curly braces

c2852d5

przemvs reviewed Mar 13, 2023

View reviewed changes

src/script/components/MessagesList/Message/MessageWrapper.tsx Show resolved Hide resolved

przemvs reviewed Mar 13, 2023

View reviewed changes

src/script/components/MessagesList/Message/MessageWrapper.tsx Show resolved Hide resolved

V-Gira merged commit 23d0231 into dev Mar 13, 2023

V-Gira deleted the virgile/generic-failure-to-send branch March 13, 2023 14:46

przemvs reviewed Mar 13, 2023

View reviewed changes

src/script/conversation/EventMapper.ts Show resolved Hide resolved

przemvs reviewed Mar 13, 2023

View reviewed changes

src/script/conversation/MessageRepository.ts Show resolved Hide resolved

V-Gira mentioned this pull request Mar 13, 2023

refactor: code legibility improvements for failure to send handling [FS-1571] #14826

Merged

V-Gira mentioned this pull request Mar 27, 2023

feat: implement retry mechanism for asset messages [FS-1634] #14890

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement retry option on message failure to send [FS-1571] #14757

feat: implement retry option on message failure to send [FS-1571] #14757

V-Gira commented Feb 27, 2023 •

edited by github-actions bot

atomrc Mar 9, 2023

V-Gira Mar 11, 2023

codecov bot commented Mar 11, 2023 •

edited

atomrc left a comment

atomrc Mar 13, 2023

atomrc Mar 13, 2023

atomrc Mar 13, 2023

V-Gira Mar 13, 2023

atomrc left a comment

		quote: quoteEntity,
		messageId: messageId ? messageId : createRandomUuid(), // We set the id explicitely in order to be able to override the message if we generate a link preview

	messageId: messageId ? messageId : createRandomUuid(), // We set the id explicitely in order to be able to override the message if we generate a link preview
	messageId: messageId ?? createRandomUuid(), // We set the id explicitely in order to be able to override the message if we generate a link preview

feat: implement retry option on message failure to send [FS-1571] #14757

feat: implement retry option on message failure to send [FS-1571] #14757

Conversation

V-Gira commented Feb 27, 2023 • edited by github-actions bot

Feature

atomrc Mar 9, 2023

Choose a reason for hiding this comment

V-Gira Mar 11, 2023

Choose a reason for hiding this comment

codecov bot commented Mar 11, 2023 • edited

Codecov Report

atomrc left a comment

Choose a reason for hiding this comment

atomrc Mar 13, 2023

Choose a reason for hiding this comment

atomrc Mar 13, 2023

Choose a reason for hiding this comment

atomrc Mar 13, 2023

Choose a reason for hiding this comment

V-Gira Mar 13, 2023

Choose a reason for hiding this comment

atomrc left a comment

Choose a reason for hiding this comment

V-Gira commented Feb 27, 2023 •

edited by github-actions bot

codecov bot commented Mar 11, 2023 •

edited