feat: add power users survey support #27361
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
This only happen for the account that is logged in when opening the app. We should probably store: |
| return; | ||
| } | ||
|
|
||
| const surveyId = 1; |
There was a problem hiding this comment.
This is set to 1, we should remove this
| const internalAccount = useSelector(getSelectedInternalAccount); | ||
|
|
||
| useEffect(() => { | ||
| if (!basicFunctionality || !internalAccount?.address) { |
There was a problem hiding this comment.
Do we want to do this request if participateInMetaMetrics is off? I don't think so but we should check with Hester.
There was a problem hiding this comment.
The task says that if participateInMetametrics would affect the tracking, not the survey; I'll check with Hester though.
There was a problem hiding this comment.
@jonybur we would not show the survey if participateInMetaMetrics is off. We would not have the MMID to check if the survey should be shown. On the ticket I described an ambiguous "Handle case where user has not opted into MetaMetrics and does not have a MetaMetrics ID". We should ensure that not getting an MMID results in a bug. Probably best way to do that is to first check if the user has participateInMetaMetrics ON.
There was a problem hiding this comment.
Just added a new case where the survey is not fetched if participateInMetametrics is off
ui/components/ui/survey-toast/__snapshots__/survey-toast.test.tsx.snap
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,113 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
We should create an E2E with a mocked API endpoint to ensure the survey displays when it should.
There was a problem hiding this comment.
I added it as a unit test, that mocks the return value and checks that the toast is rendered/not rendered
Builds ready [2c3338d]
Page Load Metrics (1890 ± 133 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
| `${ACCOUNTS_PROD_API_BASE_URL}/v1/users/fake-metrics-id/surveys`, | ||
| `${ACCOUNTS_PROD_API_BASE_URL}/v1/users/fake-metrics-fd20/surveys`, | ||
| `${ACCOUNTS_PROD_API_BASE_URL}/v1/users/test-metrics-id/surveys`, | ||
| `${ACCOUNTS_PROD_API_BASE_URL}/v1/users/invalid-metrics-id/surveys`, |
There was a problem hiding this comment.
We should do it so that any calls to /v1/users/:any-metrics-id/surveys returns:
{
statusCode: 200,
json: {
userId: '0x123',
surveys: {},
}
}
Builds ready [a7b9f4b]
Page Load Metrics (1828 ± 55 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
really like the usage of given-when-then in the manual steps, if I would suggest a small improvement for added clarity, would be to use 'given' as well, something like this:
|
| _survey.id <= lastViewedUserSurvey | ||
| ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
nit: You can improve the readability and efficiency of this code by simplifying the condition. Instead of checking if the object keys length is zero, you can directly check if the survey is falsy or if the survey ID is less than or equal to the last viewed survey ID.
if (!_survey?.id || _survey.id <= lastViewedUserSurvey) { return; }
|
|
||
| const controller = new AbortController(); | ||
|
|
||
| const fetchSurvey = async () => { |
There was a problem hiding this comment.
you can useCallback for fetchSurvey
const fetchSurvey = useCallback(async () => { ... }, [ internalAccount?.address, lastViewedUserSurvey, basicFunctionality, metaMetricsId, fetchSurvey, ]);
Builds ready [4e8c2c8]
Page Load Metrics (1703 ± 72 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
Builds ready [4e8c2c8]
Page Load Metrics (1703 ± 72 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [887de62]
Page Load Metrics (1720 ± 62 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
metamaskbot
added
the
release-12.6.0
Description
2.mp4
Related issues
Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3259
Manual testing steps
Show toast
WHEN Popup is shown << Toast should not be shown on Notification, i.e. Confirmation screen
AND User is logged in
AND API is called with hashed user id
AND API returns PowerUser (true)
THEN Toast is shown
Open survey
WHEN User clicks on survey link
THEN Open url
AND Close toast
Close toast
WHEN User clicks close icon
THEN Close toast and don't show again
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist