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

[C-3450] Native Social SignUp #7000

Merged
merged 17 commits into from
Jan 3, 2024
Merged

[C-3450] Native Social SignUp #7000

merged 17 commits into from
Jan 3, 2024

Conversation

DejayJD
Copy link
Contributor

@DejayJD DejayJD commented Dec 20, 2023

Description

  • Add logic for handling all the oauth flows
    • Utilized existing saga flows for the most part
  • Add the ReviewHandle screen
  • Add the FinalizeLoginDetails screen

TODO:

  • More QA
  • I've noticed in both my testing and even our prod app sometimes Insta API calls flake, not sure why. Not sure if its a code issue on our end or something on the insta side
  • Fix bug with repeating the flow (clicking a social, then going back to do it again). Error shown is [object Object] so thats fun
  • Loading screen not going away on popup close 😢 (alternatively maybe I rework to only show loading popup after submitting on the popup? 🤔)
  • Solve for existing login behavior on FinalizeLoginDetails
  • TikTok flow not showing loading state 🤨

How Has This Been Tested?

  • Local simulator with ios:prod + my personal socials. Luckily my instagram triggers the "review handle" flow while my others go to the "finalize login" flow.

Copy link

vercel bot commented Dec 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
audius-web-ssr ⬜️ Ignored (Inspect) Visit Preview Jan 3, 2024 11:06pm

…jd/social-media-signon

# Conflicts:
#	packages/common/src/messages/sign-on/pages.ts
#	packages/common/src/schemas/index.ts
#	packages/mobile/src/screens/sign-on-screen/SignOnStack.tsx
@DejayJD DejayJD requested a review from dylanjeffers December 20, 2023 22:32
@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/jd/social-media-signon

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/jd/social-media-signon

@@ -39,6 +39,7 @@ import IconEarningsSVG from '../assets/icons/Earnings.svg'
import IconEmailAddressSVG from '../assets/icons/EmailAddress.svg'
import IconEmbedSVG from '../assets/icons/Embed.svg'
import IconErrorSVG from '../assets/icons/Error.svg'
import IconExclamationCircleSVG from '../assets/icons/ExclamationCircle.svg'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was previously in the temp-harmony directory I had

@@ -169,11 +169,25 @@ const TIKTOK_POLLER = `
})();
`

const OAuth = () => {
type OAuthProps = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the webview popup, I just added in props to control it via component rather than via redux

Comment on lines +13 to +23
type TikTokAuthButtonProps = Partial<SocialButtonProps> & {
onSuccess?: ({
profileData,
requiresReview
}: {
profileData: TikTokProfileData
requiresReview: boolean
}) => void
onError?: (e: unknown) => void
onClose?: () => void
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in callbacks to use instead of redux

onClose?: () => void
}

const instagramAppId = Config.INSTAGRAM_APP_ID
Copy link
Contributor Author

@DejayJD DejayJD Dec 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converted logic from saga to component + hooks


type CredentialsType = 'omit' | 'same-origin' | 'include'

const useSetProfileFromTwitter = () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

converted logic from saga to component/hooks

}

// Wrapper around TikTokAuthButton that
export const SignUpFlowTikTokAuth = ({
Copy link
Contributor Author

@DejayJD DejayJD Dec 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love this abstraction around the TikTokAuthButton but for the sake of backwards compatibility I didn't want to mess with the existing component and instead just added a wrapper (this is also how we have it on web rn)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed not in love with that pattern and i appreciate you working through the details here. maybe one day we can migrate away from this pattern, but good looks with a wrapper for now.

}

export const ReadOnlyField = (props: ReadOnlyFieldProps) => {
const { label, value } = props

return (
<Box>
<Flex gap='xs'>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's no gap in the figma but for some reason the text is way too squished together 🤔
Adding a gap here seems to fix it

import { useDispatch } from 'react-redux'
import type { AnyAction } from 'redux'

export const useSocialMediaLoader = ({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

almost the same logic as web but a few diffs

const [email, setEmail] = useState('')
const [screen, setScreen] = useState<SignOnScreenType>('sign-up')
const [screen, setScreen] = useState<SignOnScreenType>(params.screen)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to link back to sign-in from the "Create Login Details" screen (which also has an email field), I had to add a param for the screen

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

smart idea, just double checking we want to link back from create-login-details?

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/jd/social-media-signon

Copy link
Contributor

@dylanjeffers dylanjeffers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really impressive lift here. love all the improvements you made along the way.

onClose?: () => void
}

const OAuth = (props: OAuthProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want a better name for this component? like if its a modal, maybe OauthWebView like you import it as?

const isOpenStore = useSelector(getIsOpen)
const providerStore = useSelector(getAuthProvider)

const useProps = props.isOpen !== undefined
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fun

/>
<Stack.Screen name='CreatePassword' component={CreatePasswordScreen} />
<Stack.Screen name='PickHandle' component={PickHandleScreen} />
<Stack.Screen name='FinishProfile' component={FinishProfileScreen} />
<Stack.Screen name='SelectGenre' component={SelectGenreScreen} />
<Stack.Screen name='SelectArtists' component={SelectArtistsScreen} />
<Stack.Screen name='ReviewHandle' component={ReviewHandleScreen} />
<Stack.Screen
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, im down to move review handle and create login details earlier in the render area, maybe after pick-handle? pure nit but a thought. we could also "group" them using Stack.Group, or just add comments, whatever you think.

setIsModalOpen(false)
if (!('error' in payload)) {
const { code } = payload
if (code) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow this is all complex, good job thinking this all through.

const useSetProfileFromTwitter = () => {
const audiusQueryContext = useAudiusQueryContext()

return async ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excellent job with this, not sure if worth callbacking these?

size='m'
style={css({ textAlign: 'center' })}
>
{emailSchemaMessages.emailInUse}{' '}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh interesting point that we might have a matching email at this point for log in... do we want that on this page you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking is to keep the behavior consistent and help avoid people making dupe accounts 🤷.
I can see a scenario where someone forgot how they signed up with Audius, clicks one of the social sign ups, then comes here and types in their email and sees that they already signed up before. (Personally I've been guilty of this exact scenario before lol)
If you're feeling otherwise, happy to discuss

@@ -139,6 +163,9 @@ export const PickHandleScreen = () => {
</Text>
</Divider>
<SocialMediaSection
onStart={handleStartSocialMediaLogin}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good stuff! everything is connected now :)

const navigation = useNavigation<SignUpScreenParamList>()
const isLinkingSocialOnFirstPage = useSelector(getLinkedSocialOnFirstPage)

const validationSchema = useMemo(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small callout, we might want to useMemo the schema in create-login-details like this. ultimately prob doesnt matter much, but maybe for consistency?

const [email, setEmail] = useState('')
const [screen, setScreen] = useState<SignOnScreenType>('sign-up')
const [screen, setScreen] = useState<SignOnScreenType>(params.screen)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

smart idea, just double checking we want to link back from create-login-details?

@@ -59,7 +59,7 @@ export const SignupFlowTikTokAuth = ({
<TikTokAuth
onClick={() => {
onStart()
dispatch(make(Name.CREATE_ACCOUNT_START_INSTAGRAM, {}))
dispatch(make(Name.CREATE_ACCOUNT_COMPLETE_TIKTOK, {}))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

complete or start?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅

</Text>
</Hint>
) : null}
<TextField
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, im adding a password-field that we can migrate to

…jd/social-media-signon

# Conflicts:
#	packages/harmony/src/assets/icons/ExclamationCircle.svg
#	packages/mobile/src/screens/sign-on-screen/screens/CreateEmailScreen.tsx
Copy link

gitguardian bot commented Jan 3, 2024

⚠️ GitGuardian has uncovered 6 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id Secret Commit Filename
688750 Generic High Entropy Secret e5a8a48 packages/ddex/.env.dev View secret
688750 Generic High Entropy Secret e5a8a48 packages/ddex/.env.stage.local View secret
2416684 Generic High Entropy Secret e5a8a48 packages/ddex/.env.stage.local View secret
2416685 Generic High Entropy Secret e5a8a48 packages/ddex/.env.stage.local View secret
2416686 Generic High Entropy Secret e5a8a48 packages/ddex/.env.stage.local View secret
3939057 Generic High Entropy Secret e5a8a48 packages/ddex/.env.dev View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/jd/social-media-signon

@DejayJD DejayJD merged commit 5bb8bb6 into main Jan 3, 2024
16 of 19 checks passed
@DejayJD DejayJD deleted the jd/social-media-signon branch January 3, 2024 23:56
audius-infra pushed a commit that referenced this pull request Jan 6, 2024
[0f7a486] [C-3478] Backlink to album from track page (#7074) Andrew Mendelsohn
[bc375b1] DDEX NODE_ENV production -> prod (#7103) Michelle Brier
[ac831b6] Actually fix discovery notif tests (#7101) Raymond Jacobson
[c9fd31e] Move uploads out of DDEX client (#7100) Michelle Brier
[d162abc] [C-3571] Fix modal header styles (#7091) Dylan Jeffers
[3e22ea8] [C-3515] Fix desktop sign up page transitions (#7084) Dylan Jeffers
[dcd6c7d] Bump version to 0.6.12 audius-infra
[dd4de97] [Web] Fix OAuth sig timestamp (#7099) nicoback2
[6aada56] Add default ddex stage keys and enforce prettier (#7098) Theo Ilie
[80ec2c0] Fix copyright year for notif test (#7079) Isaac Solo
[6e3fe0c] Increment mobile versions ahead of 1.5.59 (#7097) Raymond Jacobson
[53722a8] Add ddex upload logic to backend (#7096) Michelle Brier
[1149d51] Add 8cpu resource class to circleci runners (#7094) Danny
[7dc3969] Make Stripe the default PurchaseVendor for AddFunds on mobile (#7085) Marcus Pasell
[33e1b72] [C-3560 C-3564] Improve mobile playlist image generation perf. (#7080) Dylan Jeffers
[9d82f82] Fix harmony build (#7093) Dylan Jeffers
[e8bb46a] fix: Fix horizontal color logo clipping & resize img (#7082) JD Francis
[c8fea06] Try again to bust cache (#7090) nicoback2
[9ad8bc5] Fix esbuild on intel (#7089) Dylan Jeffers
[78460d7] Perform full disk cleanup hourly for self-hosted runners (#7088) Danny
[90f3b13] Add DDEX MVC + DB scaffolding (#7086) Theo Ilie
[a679e5b] Fix uptime optimizely env (#7083) Michelle Brier
[62a618a] Bust turbo web cache (#7081) nicoback2
[643f1d6] [C-3524] Mobile sign-up QA (#7069) Dylan Jeffers
[0cd9acf] [C-3462] Disable native font scaling (#7072) Dylan Jeffers
[9ff4ac9] [C-3546] Upgrade electron (#7071) Dylan Jeffers
[e44ec48] [PROTO-1557] Add DDEX NodeJS backend (#7064) Theo Ilie
[3d25793] Bump version to 0.6.11 audius-infra
[7840090] [Web] Enable connecting wallet to user via new OAuth `write_once` scope [C-3499] (#7045) nicoback2
[34934d7] [C-3425] Add Button component to native harmony (#7038) Kyle Shanks
[a065073] Fix svgr template when no theme (#7076) Dylan Jeffers
[c92b4c6] [Discovery API] Add transaction signature to OAuth JWT payload type (#7075) nicoback2
[f66e31c] Scheduled release UI fixes from QA (#7070) Isaac Solo
[e3ead7f] Sanitize h1 (#7073) Raymond Jacobson
[5bb8bb6] [C-3450] Native Social SignUp (#7000) JD Francis
[26f3249] Add is_scheduled_release field (#7061) Isaac Solo
[d715d6f] [PAY-2289] Fix premium/usdc modal close issues (#7060) Raymond Jacobson
[c2e15dc] Fix native storybook (#7068) Dylan Jeffers
[32333e9] [C-3553] Upgrade all react native deps to latest (#7048) Dylan Jeffers
[954437d] Restrict DDEX logins (#7062) Michelle Brier
[5acff7d] [EM] Allow dashboard wallet to sign EM CreateDashboardWalletUser tx and user to sign `user_signature` in the metadata C-3568 (#7067) nicoback2
[9102cbe] [PAY-2321] Prevent crash if nsure RN module isn't available (#7065) Randy Schott
[1bb061a] [C-3490] Improve profile touch targets (#7053) Dylan Jeffers
[d82052a] [C-3492] Fix podcast popup position (#7054) Dylan Jeffers
[e430ee8] [C-3565] Reduce saga apk size (#7058) Dylan Jeffers
[8d2d6df] Clamp upload percentage (#7049) Dylan Jeffers
[89bf61d] Fix payment method existing balance (#7063) Saliou Diallo
[6e69ecf] [PAY-2281] Fix mobile web purchase modal footer placement (#7059) Raymond Jacobson
[733f140] Bump version to 0.6.10 audius-infra
[7af8ba5] [C-3513] Private albums + show PublishButton for albums (#7055) Andrew Mendelsohn
[5c034eb] [PAY-2320] Fix explore tx link callouts (#7057) Raymond Jacobson
[f7e0852] [C-3566] UploadChip - fix text wrapping (#7056) Andrew Mendelsohn
[7811bb7] Fix schedule release gating (#7051) Isaac Solo
[eccbe31] Tag protocol dashboard `-dev` image (for audius-d) (#7052) endline
[b68a4e4] Fix react-native-svg version (#7050) Dylan Jeffers
[21ffecd] [PAY-2297] Add extra purchase metadata on mobile (#7010) Randy Schott
[9815067] Fix button color darkening (#7047) Dylan Jeffers
[1380e77] Bump version to 0.6.9 audius-infra
[7928e12] Improve sdk track upload logs (#7007) Sebastian Klingler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants