Skip to content
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

feat(communities): enable selecting addresses to pass when joining #3656

Merged
merged 1 commit into from
Jun 22, 2023

Conversation

jrainville
Copy link
Member

Needed for status-im/status-desktop#11154

Improves RequestToJoinCommunity to accept Addresses in the request. If Addresses is not empty, we then only pass to the owner the selected addresses. The others are ignored. Does not validate that the addresses in the slice are part of the user's wallet. Those not part of the wallet are just ignored.

@ghost
Copy link

ghost commented Jun 21, 2023

Pull Request Checklist

  • Have you updated the documentation, if impacted (e.g. docs.status.im)?
  • Have you tested changes with mobile?
  • Have you tested changes with desktop?

@jrainville jrainville requested review from 0x-r4bbit and mprakhov June 21, 2023 20:36
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I improved the utils functions in this test to accept passing multiple addresses. That enabled me to create the new test that verifies that when passing an address in the request, we indeed only receive that address in the owner's messenger.

@status-im-auto
Copy link
Member

status-im-auto commented Jun 21, 2023

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
✖️ 1011b17 #1 2023-06-21 20:39:20 ~2 min tests 📄log
✔️ 1011b17 #1 2023-06-21 20:39:54 ~3 min linux 📦zip
✔️ 1011b17 #1 2023-06-21 20:40:36 ~3 min ios 📦zip
✔️ 1011b17 #1 2023-06-21 20:41:26 ~4 min android 📦aar
✔️ 8ead8e6 #2 2023-06-22 16:16:01 ~2 min ios 📦zip
✔️ 8ead8e6 #2 2023-06-22 16:16:51 ~3 min linux 📦zip
✔️ 8ead8e6 #2 2023-06-22 16:20:16 ~6 min android 📦aar
✖️ 8ead8e6 #2 2023-06-22 16:37:44 ~23 min tests 📄log
✖️ 8ead8e6 #3 2023-06-22 18:42:05 ~23 min tests 📄log
✔️ 8ead8e6 #4 2023-06-22 18:53:56 ~11 min tests 📄log

Copy link
Member

@0x-r4bbit 0x-r4bbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine to me.
I've added some comments but they can be ignored


type RequestToJoinCommunity struct {
CommunityID types.HexBytes `json:"communityId"`
ENSName string `json:"ensName"`
Password string `json:"password"`
Addresses []string `json:"addresses"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you think about calling this AddressesToReveal ?

continue
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just another way to do it, but would be cool if we could change the check to something like

if wantsToRevealWalletAddress := addressToRevealMap[walletAccount.Address.Hex()]; hasAddressesToReveal && wantsToRevealWalletAddress {
 ...
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or for more readiness:

if len(request.Addresses) > 0 && !containsAddress(request.Addresses, walletAccount.Address.Hex()) {
	continue
}

func containsAddress(addresses []string, targetAddress string) bool {
	for _, address := range addresses {
		if address == targetAddress {
			return true
		}
	}
	return false
}

Improve `RequestToJoinCommunity`  to accept `Addresses` in the request. If `Addresses` is not empty, we then only pass to the owner the selected addresses. The others are ignored.
Does not validate that the addresses in the slice are part of the user's wallet. Those not part of the wallet are just ignored.
@jrainville jrainville force-pushed the feat/pass-selected-addresses-join-api branch from 1011b17 to 8ead8e6 Compare June 22, 2023 16:13
@jrainville jrainville merged commit 9c59634 into develop Jun 22, 2023
@jrainville jrainville deleted the feat/pass-selected-addresses-join-api branch June 22, 2023 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants