-
Notifications
You must be signed in to change notification settings - Fork 984
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
Check eligibility status after enabling the share all current and future addresses toggle #18870
Check eligibility status after enabling the share all current and future addresses toggle #18870
Conversation
67b7d87
to
f877804
Compare
Jenkins BuildsClick to see older builds (7)
|
f877804
to
67683dd
Compare
48% of end-end tests have passed
Failed tests (24)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (23)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestDeepLinksOneDevice:
|
(assoc :selected-permission-addresses addresses)))))})) | ||
assoc | ||
:share-all-addresses? next-share-all-addresses? | ||
:selected-permission-addresses addresses) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ajayesivan,
wouldn't this always share all addresses when we are toggling? (even if we are disabling sharing)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok got, this is expected behavior. We keep shared all addresses, but only change if we are allowed to deselect or not, based on share-all-addresses?
flag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabling the toggle implies that the previous state was enabled, indicating that all addresses were selected. Consequently, in both situations, whether the toggle is enabled or disabled, the sharing of all addresses doesn't make any difference. Do you see any potential issues with this, or does it make sense to you?
67683dd
to
69e3d9a
Compare
Hey @ajayesivan, thanks for the fix, works as expected for me. |
69e3d9a
to
ae08cb1
Compare
fixes #18867
status: ready