[#17572] fix: 'Join Community' notification is not getting dismissed #17834
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (49)
|
mohsen-ghafouri
force-pushed
the
fix/join-community-notification
branch
from
5637373
to
c0c6937
Compare
mohsen-ghafouri
force-pushed
the
fix/join-community-notification
branch
3 times, most recently
from
94c2cf9
to
e7533b8
Compare
ilmotta
reviewed
Nov 8, 2023
| @@ -21,7 +22,9 @@ | |||
| (rf/merge | |||
| cofx | |||
| {:db (all-screens-params db go-to-view-id screen-params) | |||
| :dispatch-n [[:hide-bottom-sheet]]} | |||
| :dispatch-n [[:hide-bottom-sheet] | |||
| (when (= go-to-view-id shell.constants/community-screen) | |||
There was a problem hiding this comment.
I see this navigate-to event as a general utility, so we shouldn't couple it with specific screens.
We can probably dispatch this event when the community overview screen is loaded, more or less here
You will need to convert the component to a Reagent form-2 component. It's a common technique we use to do something effectful when some component is mounted.
(defn overview
[]
(rf/dispatch :mark-as-read)
(fn [id]
...))
mohsen-ghafouri
force-pushed
the
fix/join-community-notification
branch
3 times, most recently
from
38fbbcf
to
0962255
Compare
|
@ilmotta Thank you for all the informative feedbacks; it has been incredibly helpful. could you please check again. |
ilmotta
approved these changes
Nov 9, 2023
71% of end-end tests have passedFailed tests (10)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Expected to fail tests (3)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (32)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
|
|
@mohsen-ghafouri thanx for the PR. Please take a look at the issue ISSUE 1 AC notification should not appear is request has been accepted while user is still on the same community screenSteps:
Actual result: notification remains unread in AC telegram-cloud-document-2-5364227025273895859.mp4Expected result: notification should be read 
|
mohsen-ghafouri
force-pushed
the
fix/join-community-notification
branch
from
0962255
to
1b3697d
Compare
|
@pavloburykh yeah i was checking and i couldn't find anything in my code related to login part and i thought so . okay shall i wait for a fix? |
Yes. You can follow the discussion in here https://discord.com/channels/1103692771585433630/1103692773363810317/1173923198748868688 @mohsen-ghafouri |
mohsen-ghafouri
force-pushed
the
fix/join-community-notification
branch
from
3bf94c4
to
0309ad2
Compare
|
@pavloburykh could you please recheck |
it turned out that release go branch already contains those 2 blockers. So we will wait until they are fixed. |
|
hey @mohsen-ghafouri! We decided to exclude this PR from release. It means that it will be remained blocked until #17899 and #17898 are fixed in go develop. After that we will get back to this PR and merge it into develop. Thank you and sorry for confusion. |
|
Hey @mohsen-ghafouri! Time to come back to this PR)) Could you please base it again against develop branch (both go and mobile)? Cause currently it is based on release branches. After that I will re-run e2e and hopefully we will be able to merge it. |
mohsen-ghafouri
force-pushed
the
fix/join-community-notification
branch
3 times, most recently
from
ca9dc09
to
c5ff645
Compare
|
Hey @pavloburykh it's ready |
mohsen-ghafouri
force-pushed
the
fix/join-community-notification
branch
2 times, most recently
from
09ac2ce
to
ceaa69c
Compare
88% of end-end tests have passedFailed tests (2)Click to expandClass TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Expected to fail tests (4)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (42)Click to expandClass TestDeepLinksOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
|
|
@mohsen-ghafouri thank you! PR is ready for merge. |
mohsen-ghafouri
force-pushed
the
fix/join-community-notification
branch
from
ceaa69c
to
13b7d09
Compare
yevh-berdnyk
pushed a commit
that referenced
this pull request
Dec 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #17572
Summary
After joining a community, the 'Join Community' notification remains persistent in the notification center, even after accessing the community. Additionally, tapping on the notification has no effect.
Steps to test
Before and after screenshots comparison
Simulator.Screen.Recording.-.iPhone.11.Pro.-.2023-11-07.at.17.32.59.mp4
Simulator.Screen.Recording.-.iPhone.11.Pro.-.2023-11-07.at.17.34.04.mp4
https://github.com/status-im/status-mobile/assets/71308738/cd2932fa-521c-457a-9af9-e811db362741
status: ready