Skip to content

Handle confirmations from mailservers #12824

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 22, 2021

Conversation

cammellos
Copy link
Contributor

@cammellos cammellos commented Nov 16, 2021

Makes confirmations a bit more robust.

It should fix an issue with being offline and having your messages confirmed.

On develop currently the behavior is:

  1. Have app open and ready on a chat
  2. Cut off wifi and immediately after send a message
    Expected:
    Message not confirmed
    Actual:
    Message is confirmed

After this change it will not mark the message as confirmed.

Testing

This PR will work just as in develop against prod & staging fleet (so it will result in the Actual behavior from above).
The Expected behavior needs to be tested against the test fleet.

It should though not have any issue (other than the above) when testing with prod/staging.

Develop though won't work with the test fleet, messages won't get confirmed, that's know, as it's a breaking change.

status: ready

@cammellos cammellos self-assigned this Nov 16, 2021
@status-im-auto
Copy link
Member

status-im-auto commented Nov 16, 2021

Jenkins Builds

Click to see older builds (12)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 6fcd9f5 #1 2021-11-16 16:37:48 ~15 min android-e2e 📦apk 📲
✔️ 6fcd9f5 #1 2021-11-16 16:37:53 ~15 min ios 📦ipa 📲
✔️ 6fcd9f5 #1 2021-11-16 16:39:15 ~16 min android 📦apk 📲
✔️ 14bf3c5 #2 2021-11-17 11:25:29 ~18 min ios 📦ipa 📲
✔️ 14bf3c5 #3 2021-11-18 15:44:44 ~12 min android-e2e 📦apk 📲
✔️ 14bf3c5 #3 2021-11-18 15:47:44 ~16 min android 📦apk 📲
✔️ b5e3bac #4 2021-11-18 16:14:23 ~13 min android 📦apk 📲
✔️ b5e3bac #3 2021-11-18 16:17:10 ~15 min ios 📦ipa 📲
✔️ b5e3bac #4 2021-11-18 16:19:10 ~17 min android-e2e 📦apk 📲
✔️ 6a2675a #4 2021-11-19 13:23:40 ~14 min ios 📦ipa 📲
✔️ 6a2675a #5 2021-11-19 13:24:17 ~15 min android 📦apk 📲
✔️ 6a2675a #5 2021-11-19 13:25:47 ~17 min android-e2e 📦apk 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 388aed6 #6 2021-11-19 16:44:25 ~15 min android-e2e 📦apk 📲
✔️ 388aed6 #5 2021-11-19 16:45:06 ~15 min ios 📦ipa 📲
✔️ 388aed6 #6 2021-11-19 16:46:08 ~16 min android 📦apk 📲
✔️ 61910d2 #7 2021-11-22 11:37:51 ~17 min android 📦apk 📲
✔️ 61910d2 #7 2021-11-22 11:37:55 ~18 min android-e2e 📦apk 📲

@cammellos cammellos force-pushed the feature/wait-for-ack-confirmations branch from 6fcd9f5 to 14bf3c5 Compare November 17, 2021 11:06
@cammellos cammellos marked this pull request as ready for review November 17, 2021 11:56
@churik
Copy link
Member

churik commented Nov 18, 2021

@cammellos can we do e2e build with default eth.test fleet to run e2e aginst it?

@churik churik self-assigned this Nov 18, 2021
@cammellos
Copy link
Contributor Author

@churik yes, I think so, I'll push a commit

@cammellos cammellos force-pushed the feature/wait-for-ack-confirmations branch from 14bf3c5 to b5e3bac Compare November 18, 2021 16:01
@cammellos cammellos requested a review from jakubgs as a code owner November 18, 2021 16:01
@cammellos cammellos force-pushed the feature/wait-for-ack-confirmations branch 2 times, most recently from 6a2675a to 388aed6 Compare November 19, 2021 16:29
@churik
Copy link
Member

churik commented Nov 22, 2021

Tested confirmations (flows New > Sent > Delivered, New (not sent), New > Sent, Sent > Delivered) on group chats + 1-1 chats (with offline resending) and on public chat (no resending) after offline 1min, 5mins, 30mins, 48h +.
Couldn't find any issues, works robustly.

I noticed that 'stuck' messages in public chats couldn't be resent but I suppose it is not the part of this PR, right @cammellos ?

e2e test results: here and here

Devices:

  • IPhone 12 Mini (IOS 14)
  • IPhone 7 (IOS 13)
  • Xiaomi Mi Note 9 Pro (Android 10)

@cammellos cammellos force-pushed the feature/wait-for-ack-confirmations branch from 388aed6 to 61910d2 Compare November 22, 2021 11:19
Signed-off-by: Andrea Maria Piana <andrea.maria.piana@gmail.com>
@cammellos cammellos force-pushed the feature/wait-for-ack-confirmations branch from 61910d2 to fdadee6 Compare November 22, 2021 11:20
@cammellos cammellos merged commit fdadee6 into develop Nov 22, 2021
@cammellos cammellos deleted the feature/wait-for-ack-confirmations branch November 22, 2021 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants