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

User lookup consent #1737

Merged
merged 13 commits into from
Nov 15, 2021
Merged

User lookup consent #1737

merged 13 commits into from
Nov 15, 2021

Conversation

FSM1
Copy link
Contributor

@FSM1 FSM1 commented Nov 12, 2021

closes #1601

There is currently a known issue at the API, which does not allow for turning the flag to false but this is being investigated by the API team.

API Issue has now been resolved

@render
Copy link

render bot commented Nov 12, 2021

@render
Copy link

render bot commented Nov 12, 2021

@FSM1 FSM1 requested review from tanmoyAtb and Tbaut November 12, 2021 12:46
@render
Copy link

render bot commented Nov 12, 2021

@github-actions github-actions bot added the Type: Feature Added to PRs to identify that the change is a new feature. label Nov 12, 2021
@asnaith
Copy link
Member

asnaith commented Nov 12, 2021

This is working well, we just need to ensure we remember to test the ability to uncheck to remove yourself from lookup once the API issue is resolved.

To test the scenario when it's not enabled I used a brand new account. Whilst doing this I also noticed that until lookup via share key does not work until consent is checked. I'm not sure if this is intended? Only wallet address and username are mentioned on the label.

I'll add the below scenarios to our documentation.

  • User can be looked up via wallet address when consent is enabled (no username)
  • User can be looked up via wallet address when consent is enabled (with username)
  • User can be looked up via username when consent is enabled
  • User cannot be looked up via username when consent is not enabled
  • User cannot be looked up via wallet address when consent is not enabled

...and I'll add a scenario for the expected share key too once expected behaviour is confirmed :)

@FSM1 FSM1 self-assigned this Nov 15, 2021
@FSM1
Copy link
Contributor Author

FSM1 commented Nov 15, 2021

This is working well, we just need to ensure we remember to test the ability to uncheck to remove yourself from lookup once the API issue is resolved.

Tested this now and all is resolved.

Whilst doing this I also noticed that until lookup via share key does not work until consent is checked. Only wallet address and username are mentioned on the label.

@ohmpatel1997 @dhyaniarun1993 Is this the correct behavior of the user lookup endpoint? If so, we should update the label of the checkbox.

@ohmpatel1997
Copy link

Is this the correct behavior of the user lookup endpoint? If so, we should update the label of the checkbox.

I think thats expected, since the flag should work like a global permission to make themselves searchable. do we want to change this?

@FSM1
Copy link
Contributor Author

FSM1 commented Nov 15, 2021

Is this the correct behavior of the user lookup endpoint? If so, we should update the label of the checkbox.

I think that's expected, since the flag should work like a global permission to make themselves searchable. do we want to change this?

No need, I will just update the label to more accurately reflect what the checkbox does. Just wanted to make sure that this was intended behavior. There were a lot of discussions around whether lookup by the User Pub Key were to be allowed even if the lookup flag was set to false.

@ohmpatel1997
Copy link

@kalambet @dhyaniarun1993 yours will be the final call, what do you think?

@dhyaniarun1993
Copy link
Member

@kalambet @dhyaniarun1993 yours will be the final call, what do you think?

Yep. I agree there should be only 1 flag that decide if user is searchable or not

@FSM1 FSM1 merged commit 24ef31c into dev Nov 15, 2021
@FSM1 FSM1 deleted the feat/user-lookup-flag-1601 branch November 15, 2021 13:45
@FSM1 FSM1 mentioned this pull request Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Added to PRs to identify that the change is a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow users to control whether they can be looked up by wallet address
6 participants