Remove unsupported notifications from ac notification data by ajayesivan · Pull Request #22595 · status-im/status-legacy

ajayesivan · 2025-05-16T10:08:09Z

Summary

There are several notification types that are not yet supported on mobile. When unsupported notifications are present in the notification data, this was causing a AC 'all' tab blank state.

This PR filters out unsupported notification types and renders only the supported ones to avoid this issue.

See the issue for the complete discussion.

Risk

Described potential risks and worst case scenarios.

Tick one:

Low risk: 2 devs MUST perform testing as specified above and attach their results as comments to this PR before merging.
High risk: QA team MUST perform additional testing in the specified affected areas before merging.

ajayesivan · 2025-05-16T10:09:48Z

@ilmotta @flexsurfer @ulisesmac @Parveshdhull Whenever you have a moment, could you please review this PR? Thanks in advance!

status-im-auto · 2025-05-16T10:13:26Z

Jenkins Builds

Click to see older builds (8)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`4610920`	#1	2025-05-16 10:13:25	~4 min	`tests`	📄`log`
✔️	`4610920`	#1	2025-05-16 10:15:48	~7 min	`android-e2e`	🤖`apk` 📲
✔️	`4610920`	#1	2025-05-16 10:18:01	~9 min	`android`	🤖`apk` 📲
✔️	`4610920`	#1	2025-05-16 10:20:28	~11 min	`ios`	📱`ipa` 📲

✔️	`62783d6`	#2	2025-05-19 12:37:36	~5 min	`tests`	📄`log`
✔️	`62783d6`	#2	2025-05-19 12:41:07	~9 min	`android-e2e`	🤖`apk` 📲
✔️	`62783d6`	#2	2025-05-19 12:41:56	~10 min	`android`	🤖`apk` 📲
✔️	`62783d6`	#2	2025-05-19 12:44:31	~12 min	`ios`	📱`ipa` 📲

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`a3ed6cb`	#3	2025-05-19 12:53:49	~4 min	`tests`	📄`log`
✔️	`a3ed6cb`	#3	2025-05-19 12:58:08	~9 min	`android-e2e`	🤖`apk` 📲
✔️	`a3ed6cb`	#3	2025-05-19 12:58:50	~10 min	`android`	🤖`apk` 📲
✔️	`a3ed6cb`	#3	2025-05-19 12:58:57	~10 min	`ios`	📱`ipa` 📲

✔️	`7fcf83d`	#4	2025-05-20 03:47:51	~4 min	`tests`	📄`log`
✔️	`7fcf83d`	#4	2025-05-20 03:52:21	~9 min	`android-e2e`	🤖`apk` 📲
✔️	`7fcf83d`	#4	2025-05-20 03:52:52	~10 min	`ios`	📱`ipa` 📲
✔️	`7fcf83d`	#4	2025-05-20 03:52:58	~10 min	`android`	🤖`apk` 📲

Parveshdhull

Working perfectly (video), Thank you @ajayesivan

status-im-auto · 2025-05-19T09:38:22Z

88% of end-end tests have passed

Total executed tests: 24
Failed tests: 3
Expected to fail tests: 0
Passed tests: 21

IDs of failed tests: 727230,741841,742016

Failed tests (3)

Click to expand

Rerun failed tests

Class TestWalletCollectibles:

1. test_wallet_collectible_send_from_expanded_info_view, id: 741841

Device 1: Find `CollectibleItemElement` by `xpath`: `//*[@content-desc='collectible-list-item']//*[contains(@text,'Glitch Punks')]/../..`
Device 1: Find `CollectibleItemElement` by `xpath`: `//*[@content-desc='collectible-list-item']//*[contains(@text,'Glitch Punks')]/../..`

wallet/test_collectibles.py:159: in test_wallet_collectible_send_from_expanded_info_view
    self.wallet_view.get_collectible_element('Glitch Punks').scroll_and_click()
../views/base_element.py:209: in scroll_and_click
    self.scroll_to_element(direction=direction)
../views/base_element.py:204: in scroll_to_element
    raise NoSuchElementException(
 Device 1: CollectibleItemElement by xpath: `//*[@content-desc='collectible-list-item']//*[contains(@text,'Glitch Punks')]/../..` is not found on the screen; For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception

Device sessions

Device 1:

Steps, video, logs

Class TestWalletMultipleDevice:

1. test_wallet_send_erc20_from_drawer[Optimism Sepolia-USD Coin-USDC-2-0.01], id: 727230
Device 1: Find `Text` by `xpath`: `//android.view.ViewGroup[@content-desc='container']/android.widget.TextView[@text='USD Coin']/../android.widget.TextView[3]` Device 1: `Text` is `47.06 USDC (EVM)` `wallet_txs/test_wallet_testnet.py:101: in test_wallet_send_erc20_from_drawer self.errors.verify_no_errors() base_test_case.py:207: in verify_no_errors pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))])) Device 1: Verification of last transaction failed.The failed check: amount (Expected: 0.01 USDC, Found: 0.01 USDC (EVM))` Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
2. test_wallet_send_erc20_from_drawer[Sepolia-USD Coin-USDC-2-0.01], id: 742016
Device 1: Find `Text` by `xpath`: `//android.view.ViewGroup[@content-desc='container']/android.widget.TextView[@text='USD Coin']/../android.widget.TextView[3]` Device 1: `Text` is `47.07 USDC (EVM)` `wallet_txs/test_wallet_testnet.py:101: in test_wallet_send_erc20_from_drawer self.errors.verify_no_errors() base_test_case.py:207: in verify_no_errors pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))])) Device 1: Verification of last transaction failed.The failed check: amount (Expected: 0.01 USDC, Found: 0.01 USDC (EVM))` Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Passed tests (21)

Click to expand

Class TestWalletCollectibles:

1. test_wallet_send_collectible, id: 741840
Device sessions Device 1: Steps, video, logs
2. test_wallet_collectibles_balance, id: 741839
Device sessions Device 1: Steps, video, logs

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_send_image_save_and_share, id: 703391
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
2. test_1_1_chat_pin_messages, id: 702731
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
3. test_1_1_chat_message_reaction, id: 702730
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Class TestWalletMultipleDevice:

1. test_send_eth[Status Network Sepolia-0.0002], id: 727229
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
2. test_send_eth[Arbitrum Sepolia-0.0001], id: 742015
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Class TestCommunityOneDeviceMerged:

1. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions Device 1: Steps, video, logs

Class TestWalletOneDeviceTwo:

1. test_wallet_add_remove_regular_account, id: 727231
Device sessions Device 1: Steps, video, logs

Class TestProfileMultipleDevices:

1. test_profile_change_profile_photo, id: 741969
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
2. test_profile_change_username, id: 741968
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_delete, id: 702839
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
2. test_community_message_send_check_timestamps_sender_username, id: 702838
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
3. test_community_one_image_send_reply_set_reaction, id: 702859
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
4. test_community_message_edit, id: 702843
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Class TestGroupChatMultipleDeviceMergedNewUI:

1. test_group_chat_join_send_text_messages_push, id: 702807
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs Device 3: Steps, video, logs

Class TestWalletCustomParamOneDevice:

1. test_send_snt_custom_tx_params, id: 742910
Device sessions Device 1: Steps, video, logs

Class TestWalletOneDevice:

1. test_wallet_swap_flow_mainnet, id: 741555
Device sessions Device 1: Steps, video, logs
2. test_wallet_balance_mainnet, id: 740490
Device sessions Device 1: Steps, video, logs
3. test_wallet_bridge_flow_mainnet, id: 741612
Device sessions Device 1: Steps, video, logs
4. test_wallet_send_flow_mainnet, id: 741554
Device sessions Device 1: Steps, video, logs

ilmotta

Nice fix 💯

ilmotta · 2025-05-19T09:45:35Z

src/status_im/subs/activity_center.cljs

+ :<- [:activity-center/notifications]
+ (fn [notifications]
+   (->> notifications
+        (filter #(types/all-supported (:type %))))))


Minor detail: the thread macro is unnecessary with just one transformation.

Yep, I used it because it felt a bit clearer to me. But if you prefer removing the macro, I’m happy to update it!

It tends to be seen as superfluous in my experience from other Clojure repos. In the CSG https://guide.clojure.style/#threading-macros there's a recommendation to use it to reduce heavy nesting. I've seen most Clojure code tends to use the thread macros when there are two or more transformations (but it's not a rule).

If you add a line to separate the transformation it can look clean.

(filter #(types/all-supported (:type %)) notifications)

Or if you create a predicate it looks cleaner:

(filter supported-notification? notifications)

Thank you, @ilmotta, for the explanation. I think the last approach is the best one, and I’ve updated the code accordingly.

@ilmotta This is something I've seen multiple times in our repo: using threading macros with only one step. As you said, it's not a rule but is superfluous.

ulisesmac

Looks fine! Thanks @ajayesivan 👍

ajayesivan marked this pull request as ready for review May 16, 2025 10:08

status-github-bot-v2 bot added this to Pipeline for QA May 16, 2025

status-github-bot-v2 bot moved this to CONTRIBUTOR in Pipeline for QA May 16, 2025

flexsurfer added the external label May 16, 2025

flexsurfer requested review from Parveshdhull and ilmotta May 16, 2025 11:15

flexsurfer moved this from CONTRIBUTOR to REVIEW in Pipeline for QA May 16, 2025

Parveshdhull approved these changes May 19, 2025

View reviewed changes

churik added the request-manual-qa label May 19, 2025

churik moved this from REVIEW to E2E Tests in Pipeline for QA May 19, 2025

ilmotta approved these changes May 19, 2025

View reviewed changes

ajayesivan force-pushed the 22418-load-supported-ac-notifications branch from 4610920 to 62783d6 Compare May 19, 2025 12:31

ajayesivan added 2 commits May 20, 2025 09:12

fix: remove unsupported notifications from ac notification data

7674cb9

remove threaded marco

7fcf83d

ajayesivan force-pushed the 22418-load-supported-ac-notifications branch from a3ed6cb to 7fcf83d Compare May 20, 2025 03:42

ulisesmac approved these changes May 20, 2025

View reviewed changes

ulisesmac assigned ajayesivan May 20, 2025

Horupa-Olena self-assigned this May 20, 2025

Conversation

ajayesivan commented May 16, 2025

Summary

Risk

Uh oh!

ajayesivan commented May 16, 2025

Uh oh!

status-im-auto commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Jenkins Builds

Uh oh!

Parveshdhull left a comment

Choose a reason for hiding this comment

Uh oh!

status-im-auto commented May 19, 2025

88% of end-end tests have passed

Failed tests (3)

Class TestWalletCollectibles:

Class TestWalletMultipleDevice:

Passed tests (21)

Class TestWalletCollectibles:

Class TestOneToOneChatMultipleSharedDevicesNewUi:

Class TestWalletMultipleDevice:

Class TestCommunityOneDeviceMerged:

Class TestWalletOneDeviceTwo:

Class TestProfileMultipleDevices:

Class TestCommunityMultipleDeviceMerged:

Class TestGroupChatMultipleDeviceMergedNewUI:

Class TestWalletCustomParamOneDevice:

Class TestWalletOneDevice:

Uh oh!

ilmotta left a comment

Choose a reason for hiding this comment

Uh oh!

ilmotta May 19, 2025

Choose a reason for hiding this comment

Uh oh!

ajayesivan May 19, 2025

Choose a reason for hiding this comment

Uh oh!

ilmotta May 19, 2025

Choose a reason for hiding this comment

Uh oh!

ajayesivan May 19, 2025

Choose a reason for hiding this comment

Uh oh!

ulisesmac May 20, 2025

Choose a reason for hiding this comment

Uh oh!

ulisesmac left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

status-im-auto commented May 16, 2025 •

edited

Loading