Do not store messages from extension if app is receiving events #2481

polqf · 2023-02-07T12:12:59Z

🔗 Issue Links

Resolves #2309
Resolves https://github.com/GetStream/ios-issues-tracking/issues/264

🎯 Goal

Make channel unread counts accurate

📝 Summary

Unread counts were not accurate when the app was in the foreground handling the updates both using WS events and Push Notifications

🛠 Implementation

This PR stops the notification extension from saving data to the DB if the host app is connected through WS, and processing those events.
This communication is done using a shared UserDefaults based on App Groups.

🧪 Manual Testing Notes

Open the app and leave it on the channel list
Using another device/simulator, send messages to the previous device
The unread count should have the correct amount

The issue was frequently visible when sending replies to a thread (sending those messages to the channel, as well)

☑️ Contributor Checklist

I have signed the Stream CLA (required)
This change follows zero ⚠️ policy (required)
This change should be manually QAed
Changelog is updated with client-facing changes
New code is covered by unit tests
Comparison screenshots added for visual changes
Affected documentation updated (docusaurus, tutorial, CMS)

🎁 Meme

github-actions · 2023-02-07T12:29:51Z

	1 Warning
⚠️	The changes should be manually QAed before the Pull Request will be merged

	1 Message
📖	There seems to be app changes but CHANGELOG wasn't modified. Please include an entry if the PR includes user-facing changes. You can find it at CHANGELOG.md.

Generated by 🚫 Danger

Sources/StreamChat/Repositories/MessageRepository.swift

Sources/StreamChat/Utils/NotificationExtension+Lifecycle.swift

.../Mocks/StreamChat/WebSocketClient/EventMiddlewares/NotificationExtensionLifecycle_Mock.swift

Tests/StreamChatTests/APIClient/ChatPushNotificationContent_Tests.swift

Tests/StreamChatTests/Repositories/MessageRepository_Tests.swift

ipavlidakis · 2023-02-08T09:00:04Z

Tests/StreamChatTests/Utils/Database/NSManagedObject_Tests.swift

@@ -14,6 +15,52 @@ final class NSManagedObject_Tests: XCTestCase {
    func test_entityName_custom() {
        XCTAssertEqual(EntityWithCustomName.entityName, "CustomName")
    }
+
+    func test_discardChanges_shouldClearChangesForTheObject() throws {


Instead of ClearChanges consider using Discard to match the action you are performing and help the reader understand better

Isn't it like trying to describe a word with the word itself? 😂 That's why a went for it, but happy to change it if is not that readable

It's not that it isn't readable just for consistency :). Up to you :)

Tests/StreamChatTests/Workers/Background/ConnectionRecoveryHandler_Tests.swift

.../Mocks/StreamChat/WebSocketClient/EventMiddlewares/NotificationExtensionLifecycle_Mock.swift

ipavlidakis

LGTM

testableapple

👍

emerge-tools · 2023-02-09T15:42:50Z

📏 Size Analysis

Total install size 10.6 MB | This change: ⬆️ +179.3 kB (+1.71%)

🗂 See size breakdown

Item	Install size
ChatSample/Strings.CFStrings	🔺 12.4 kB
ChatSample/External Methods	🔺 8.3 kB
➕ ChatSample/SentryProfiler.captureEnvelope	🔺 7.5 kB
ChatSample/Strings.Unmapped	🔺 5.1 kB
➖ ChatSample/SentryTraceState	⬇️ 4.0 kB
ChatSample/Code Signature	🔺 3.4 kB
➕ StreamChat.framework/StreamChat.MessageRepository.getMessage(cid,messageId,store,completion)	🔺 3.2 kB
➕ ChatSample/SentryNetworkTracker.captureFailedRequests:	🔺 3.2 kB
➖ ChatSample/SentryProfiler.buildEnvelopeItemForTransaction:	⬇️ 2.7 kB
➕ ChatSample/CL.CLLocationManagerDelegate.Objc Metadata	🔺 2.4 kB
➕ ChatSample/SentryFileManager.initWithOptions:andCurrentDateProvider:dispatchQueueWrapper:error:	🔺 2.3 kB
➖ StreamChat.framework/StreamChat.MessageRepository.getMessage(cid,messageId,completion)	⬇️ 2.2 kB
➖ ChatSample/SentryCrashDeadlockMonitor	⬇️ 1.9 kB
➕ ChatSample/processBacktrace	🔺 1.8 kB
ChatSample/SentryNetworkTracker.urlSessionTaskResume:	🔺 1.8 kB
ChatSample/DYLD.Exports	🔺 1.7 kB
➕ ChatSample/SentryUIViewControllerSwizzling.swizzleUIViewControllersOfClassesInImageOf:	🔺 1.6 kB
ChatSample/SentryOptions.Objc Metadata	🔺 1.6 kB
ChatSample/SentryOptions.validateOptions:didFailWithError:	🔺 1.5 kB
➕ ChatSample/SentryTracer.addMeasurements:	🔺 1.4 kB

🔎 See the full size analysis (c53fbe1) merging into develop (2c2ef9e)

Provide a baseSha and pullRequestNumber with your upload to ensure diffs are correct. Read more in the docs

sonarcloud · 2023-02-13T11:37:48Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

95.8% Coverage
0.0% Duplication

* Do not store messages from extension if app is receiving events * Add tests for NotificationExtensionLifecycle * Update tests for ChatRemoteNotificationHandler * Add tests for NSManagedObjectContext.discard * Address PR comments * Do not set a default value for 'store' in MessageRepository.getMessage * Fix specs compilation issue

polqf force-pushed the bugfix/unread-counts-when-push-extension branch from 5bb3c82 to b1625ac Compare February 7, 2023 12:13

polqf marked this pull request as ready for review February 7, 2023 17:15

polqf requested a review from a team as a code owner February 7, 2023 17:15

testableapple added the 🤞 Ready For QA A PR that is Ready for QA label Feb 7, 2023

ipavlidakis reviewed Feb 8, 2023

View reviewed changes

testableapple removed the 🤞 Ready For QA A PR that is Ready for QA label Feb 8, 2023

ipavlidakis approved these changes Feb 9, 2023

View reviewed changes

polqf force-pushed the bugfix/unread-counts-when-push-extension branch from 82b719c to 90d725a Compare February 9, 2023 12:16

testableapple added the 🤞 Ready For QA A PR that is Ready for QA label Feb 9, 2023

testableapple approved these changes Feb 9, 2023

View reviewed changes

polqf added 6 commits February 13, 2023 12:01

Do not store messages from extension if app is receiving events

340fbc4

Add tests for NotificationExtensionLifecycle

47eaaf9

Update tests for ChatRemoteNotificationHandler

8293611

Add tests for NSManagedObjectContext.discard

c74a370

Address PR comments

8cab288

Do not set a default value for 'store' in MessageRepository.getMessage

c454f3b

polqf force-pushed the bugfix/unread-counts-when-push-extension branch from c53fbe1 to c454f3b Compare February 13, 2023 11:01

Fix specs compilation issue

b60bf7d

polqf mentioned this pull request Feb 13, 2023

Unread count is broken in stream-chat. #2309

Closed

polqf merged commit 90b6ac8 into develop Feb 14, 2023

polqf deleted the bugfix/unread-counts-when-push-extension branch February 14, 2023 10:19

polqf mentioned this pull request Feb 16, 2023

4.27.0 Release #2495

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not store messages from extension if app is receiving events #2481

Do not store messages from extension if app is receiving events #2481

polqf commented Feb 7, 2023 •

edited

github-actions bot commented Feb 7, 2023 •

edited

ipavlidakis Feb 8, 2023

polqf Feb 8, 2023

ipavlidakis Feb 9, 2023

ipavlidakis left a comment

testableapple left a comment

emerge-tools bot commented Feb 9, 2023 •

edited

sonarcloud bot commented Feb 13, 2023

Do not store messages from extension if app is receiving events #2481

Do not store messages from extension if app is receiving events #2481

Conversation

polqf commented Feb 7, 2023 • edited

🔗 Issue Links

🎯 Goal

📝 Summary

🛠 Implementation

🧪 Manual Testing Notes

☑️ Contributor Checklist

🎁 Meme

github-actions bot commented Feb 7, 2023 • edited

ipavlidakis Feb 8, 2023

Choose a reason for hiding this comment

polqf Feb 8, 2023

Choose a reason for hiding this comment

ipavlidakis Feb 9, 2023

Choose a reason for hiding this comment

ipavlidakis left a comment

Choose a reason for hiding this comment

testableapple left a comment

Choose a reason for hiding this comment

emerge-tools bot commented Feb 9, 2023 • edited

📏 Size Analysis

Total install size 10.6 MB | This change: ⬆️ +179.3 kB (+1.71%)

sonarcloud bot commented Feb 13, 2023

polqf commented Feb 7, 2023 •

edited

github-actions bot commented Feb 7, 2023 •

edited

emerge-tools bot commented Feb 9, 2023 •

edited