fix: handle unknown contract community #20198
Merged
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 (19)
|
yqrashawn
force-pushed
the
fix/slow-discover-community
branch
2 times, most recently
from
cef39ae to
e31af78
Compare
CleanShot.2024-05-28.at.15.59.03.2.mp4 |
yqrashawn
force-pushed
the
fix/slow-discover-community
branch
from
e31af78 to
8f57588
Compare
flexsurfer
reviewed
May 28, 2024
| @@ -84,7 +83,9 @@ | |||
| (local-notifications/process cofx (transforms/js->clj event-js)) | |||
|
|
|||
| "community.found" | |||
| (link-preview/cache-community-preview-data (transforms/js->clj event-js)) | |||
| (let [community (transforms/js->clj event-js)] | |||
| {:fx [[:dispatch [:chat.ui/cache-link-preview-data-by-community community]] | |||
There was a problem hiding this comment.
why do we use dispatch here?
There was a problem hiding this comment.
cache-link-preview-data fx should be used directly here
| @@ -19,6 +19,10 @@ | |||
| data)))] | |||
| {:db (assoc-in db [:profile/profile :link-previews-cache] link-previews-cache)}))) | |||
|
|
|||
| (rf/reg-event-fx :chat.ui/cache-link-preview-data-by-community | |||
| (fn [_ [community]] | |||
| {:fx [[:dispatch [:chat.ui/cache-link-preview-data (community-link (:id community)) community]]]})) | |||
There was a problem hiding this comment.
why double dispatch is needed?
ilmotta
approved these changes
May 28, 2024
There was a problem hiding this comment.
Thanks for the PR description and adding tests @yqrashawn.
Except for the things @flexsurfer already commented, I'm approving in advance because the code LGTM.
I
yqrashawn
force-pushed
the
fix/slow-discover-community
branch
2 times, most recently
from
426f264 to
0e8186e
Compare
|
Thanks for the review, I've addressed your feedback. @flexsurfer |
41% of end-end tests have passedNot executed tests (1)Failed tests (26)Click to expandClass TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
Class TestWalletMultipleDevice:
Class TestActivityMultipleDevicePRTwo:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Expected to fail tests (4)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
Passed tests (21)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestWalletOneDevice:
|
yqrashawn
force-pushed
the
fix/slow-discover-community
branch
from
0e8186e to
c059f63
Compare
flexsurfer
reviewed
May 29, 2024
| (link-preview/cache-community-preview-data (transforms/js->clj event-js)) | ||
| (let [community (transforms/js->clj event-js)] | ||
| (link-preview/cache-community-preview-data community) | ||
| (rf/dispatch [:discover-community/maybe-found-unknown-contract-community community])) |
There was a problem hiding this comment.
if we still want to use dispatch it should be :fx :dispatch
There was a problem hiding this comment.
I've updated the code, do you mean like this?
Signed-off-by: yqrashawn <namy.19@gmail.com>
yqrashawn
force-pushed
the
fix/slow-discover-community
branch
from
c059f63 to
0d76176
Compare
55% of end-end tests have passedNot executed tests (1)Failed tests (19)Click to expandClass TestWalletOneDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePRTwo:
Class TestWalletMultipleDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Expected to fail tests (4)Click to expandClass TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (28)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
|
flexsurfer
reviewed
May 29, 2024
| (link-preview/cache-community-preview-data (transforms/js->clj event-js)) | ||
| (let [community (transforms/js->clj event-js)] | ||
| (link-preview/cache-community-preview-data community) | ||
| {:fx [[:dispatch [:discover-community/maybe-found-unknown-contract-community community]]]}) |
There was a problem hiding this comment.
sorry for the confusion, here you have to dispatch cache-community-preview-data as well as it was in the first iteration of your PR.
|
@yqrashawn thanks for the PR! Failed e2e are not PR related. Ready for merge and cherrypick into release branch cc @jo-mut |
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 #20092
Summary
status-go give mobile this discover community list looks like this
{:communities {"a" {:id "a"}} :contractFeaturedCommunities ["a" "b"] :contractCommunities ["a" "b" "c"] :unknownCommunities ["b" "c"]}we save it into db like this, discover community view will stay loading forever since there's only one community in the list and it's unknown
{:featured {"a" {:id "a"}} ;; cause there's no known data about "b" and "c" :other {}}this PR save the unknown communities into db and update them into
featuredorotheronce their"community.found"signal received{:featured {"a" {:id "a"}} :other {} :unknown-featured '("b") :unknown-other '("c")}Testing notes
endpoint xxxxxx has passed its daily relay limiton mainnet (in geth.log), this prevents status-go fetching the list as well, which leads to infinite loadingPOKT_TOKENto a newly created one, not sure if PR/nightly build uses different tokenPlatforms
Areas that maybe impacted
discover community
Steps to test
status: ready