-
Notifications
You must be signed in to change notification settings - Fork 10
Linxiaoli/vsl 236 and vsl 263: integrate with API and show error message and implement subscribe all community flow #698
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
base: notifications
Are you sure you want to change the base?
Conversation
| }; | ||
|
|
||
| export const startLeanplum = () => { | ||
| const getDesiredAttributes = (communitySubIntentions) => { |
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.
This is to convert the array in line 18 into a format of {community1:'True', community2:'False'}
| } catch (e) { | ||
| throw new Error(e); | ||
| } | ||
| return new Promise((resolve, reject) => { |
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.
Returning a promise is necessary for ensuring the sequence of execution, we want start leanplum first then get user settings
| email: data.userAttributes.email, | ||
| }; | ||
| let isSubscribedToCommunityUpdates = true; | ||
| let isSubscribedFromCommunityUpdates = true; |
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.
Naming correction to align the names with the notion document
| count: 100, | ||
| initialLoading: false, | ||
| }); | ||
| const displayCommunities = isLoadingUserCommunities |
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.
This is a list of all communities a user participates, if leanplum userAttributes doesn't have a community, it will render that community still but with notification turned off
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.
under Settings it should only display list of communities that user subscribed before, looks like previous code is correct. Any reason we are changing this to be all communities?
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.
Got it, I misunderstood the design, will change it back
|
hi @linxiao-li for |
...end/packages/client/src/components/Settings/NotificationSettingsSection/EmailAddressInput.js
Show resolved
Hide resolved
| count: 100, | ||
| initialLoading: false, | ||
| }); | ||
| const displayCommunities = isLoadingUserCommunities |
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.
under Settings it should only display list of communities that user subscribed before, looks like previous code is correct. Any reason we are changing this to be all communities?
|
@amyliu6 Got it, I misread the design. Just to double check, for the subscription flow I'm under the impression I only need to implement the functions for subscribe all communities and one community api call, the complete flow of subscription is going to be handled by this ticket? |
hi @linxiao-li the plan is to have all API integration in one ticket(236) as discussed during planning, hence we updated 232 to be testing only to verify the E2E flow. Given Manny is on leave this week, could you help with sanity test for subscription flow too so that QA can start testing both subscription flow and settings flow later this sprint? |
|
@amyliu6 Updated the user flows for updating subscriptions
sign.up.all.from.community.page.mov
update.subscription.from.proposal.page.mov
update.subscription.after.vote.mov
error.modal.mov |
| }; | ||
| }); | ||
| await updateCommunity(communitySubIntentions); | ||
| await new Promise((r) => setTimeout(r, 500)); |
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.
This is to throttle the api calls, allow the data to get registered with leanplum before calling it again to get the updated data
| email, | ||
| })); | ||
| } catch { | ||
| throw new Error('cannot get user settings'); |
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.
in case getUserSettings error, will user see error modal when they land on Settings page?
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.
Not right now, if for any reason any of the user init functions failed (startLeanplum, setUserId, getUserSetting) we are only console logging the error. However, on the flow of updating subscriptions, getUserSetting error is caught and error modal shows up
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.
i see, if getUserSettings failed, we won't be able to get user data, how is the current flow look like when user visit Settings page? Is it possible to display the error modal for that case too?
|
|
||
| /* @param: communitySubIntentions : [{communityId:"1", subscribeIntention:"subscribe"},{communityId:"2",subscribeIntention:"unsubscribe"}] | ||
| * @return: {communityId1:'True', communityId2:'False'} | ||
| */ |
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.
👍
This pr includes:
Demo:
connect.wallet.and.populate.settings.page.mov
subscribe.to.all.email.notifications.mov
change.email.flow.mov
subscribe.to.community.mov
subscribe.from.community.page.mov