[FEQ] / Ameerul / FEQ-1969 Refactor useAdvertiserInfo hook #14611
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
A production App ID was automatically generated for this PR. (log)
Click here to copy & paste above information. |
|
🚨 Lighthouse report for the changes in this PR:
Lighthouse ran with https://deriv-app-git-fork-ameerul-deriv-feq-1969-refactor-use-a-cfc209.binary.sx/ |
| <OnlineStatusIcon isOnline={!!isOnline} isRelative size='0.8em' /> | ||
| <OnlineStatusLabel | ||
| isOnline={!!isOnline} | ||
| lastOnlineTime={lastOnlineTime === null ? undefined : lastOnlineTime} |
There was a problem hiding this comment.
because the type is number | null | undefined and the function in OnlineStatusLabel, getLastOnlineLabel prop lastOnlineTime can be undefined but doesn't take null. If undefined, it will return a default string, so we shouldn't do lastOnlineTime ?? 0 to return a default value.
| sendbirdServiceToken?.app_id && | ||
| advertiserInfo?.chat_user_id | ||
| ) { | ||
| if (isSuccessSendbirdServiceToken && sendbirdServiceToken?.app_id && advertiserInfo?.chat_user_id) { |
There was a problem hiding this comment.
no need to check for isSubscribed, since checking for advertiserInfo.chat_user_id is already enough
…9-refactor-use-advertiser-info
|
| * Custom hook to access the current logged in user's advertiser info state. | ||
| * @returns {TContextValue} The current logged in user's advertiser info state. | ||
| */ | ||
| export const useAdvertiserInfoState = (): TContextValue => useContext(AdvertiserInfoStateContext); |
There was a problem hiding this comment.
Because the state isn't cached or stored locally to the useGetInfo hook, the state will always be returned as the default values, which will affect how we try to get these states when trying to access it in different components when calling the hook.
Because we need to know the state of the current logged in user, in a lot of different components, especially to determine if the user is an advertiser or not by checking for AdvertiserNotFound error, it is necessary to have this provider.
| useEffect(() => { | ||
| if (isSuccess) { | ||
| subscribeAdvertiserInfo(); | ||
| } | ||
| }, [isSuccess, subscribeAdvertiserInfo]); | ||
|
|
||
| // Need this to subscribe to advertiser info after user has created an advertiser. | ||
| // setHasCreatedAdvertiser is triggered inside of NicknameModal. | ||
| useEffect(() => { | ||
| if (isSuccess && hasCreatedAdvertiser) { | ||
| subscribeAdvertiserInfo(); | ||
| } | ||
| }, [hasCreatedAdvertiser, isSuccess, subscribeAdvertiserInfo]); |
There was a problem hiding this comment.
When the user has not added their nickname yet, and are already POA + POI verified, the error from p2p_advertiser_info api endpoint will return AdvertiserNotFound. Because of this, the first useEffect's subscribe function will try to subscribe, but then it will fail. So the second useEffect is a must as when the user creates their account for p2p, we will trigger the above state hasCreatedAdvertiser to subscribe to the newly created user.
Prior we would use invalidation, but we can't do that for useSubscription
|
⏳ Generating Lighthouse report... |
ameerul-deriv
requested review from
adrienne-deriv,
farhan-nurzi-deriv and
thisyahlen-deriv
as code owners
ameerul-deriv
requested review from
ali-hosseini-deriv,
amina-deriv,
balakrishna-deriv,
farrah-deriv,
hirad-deriv,
mahdiyeh-deriv,
markw-deriv,
maryia-deriv,
matin-deriv,
nijil-deriv,
sandeep-deriv and
yashim-deriv
as code owners
6 participants
Changes:
Please provide a summary of the change.
Screenshots:
Please provide some screenshots of the change.
useAdvertiserInfo.localstorage.mov