-
Notifications
You must be signed in to change notification settings - Fork 303
fix(profile): persist pending social links on save #6663
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
Merged
rebelchris
merged 6 commits into
main
from
eng-2011-feedback-ux-issue-link-to-github-in-profile-details-does-not
Sep 17, 2026
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
2c5d3c0
fix(profile): persist pending social links on save
rebelchris 2756eea
Merge branch 'main' into eng-2011-feedback-ux-issue-link-to-github-in…
rebelchris 6004bf4
Merge branch 'main' into eng-2011-feedback-ux-issue-link-to-github-in…
rebelchris c4b9b75
fix(profile): stop social link saves from clearing server links
rebelchris 3001676
Merge branch 'main' into eng-2011-feedback-ux-issue-link-to-github-in…
rebelchris 03fd147
refactor(profile): share the profile form hint type and parser
rebelchris File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| import { test, expect, type Page } from '@playwright/test'; | ||
| import { login } from './helpers'; | ||
|
|
||
| /** | ||
| * This is the only spec in the package that writes to the shared CI account, | ||
| * and it runs against live production. Cleanup is best effort: if Save itself | ||
| * fails the link was never stored, and if it stored but the redirect did not | ||
| * happen the row is still removed below. A hard failure mid-cleanup leaves one | ||
| * `dailydev-e2e-*` link behind for a maintainer to delete. | ||
| */ | ||
| const openProfileSettings = async (page: Page): Promise<void> => { | ||
| await page.goto('/settings/profile'); | ||
|
|
||
| // The links section stays disabled until the profile query settles, so the | ||
| // list is only trustworthy once the input is enabled. | ||
| await expect(page.getByRole('textbox', { name: 'Add link' })).toBeEnabled({ | ||
| timeout: 20000, | ||
| }); | ||
| }; | ||
|
|
||
| const removeSocialLink = async (page: Page, url: string): Promise<void> => { | ||
| await openProfileSettings(page); | ||
|
|
||
| const linkRow = page.getByTestId('social-link-row').filter({ hasText: url }); | ||
|
|
||
| if ((await linkRow.count()) === 0) { | ||
| return; | ||
| } | ||
|
|
||
| await linkRow.getByRole('button', { name: 'Remove link' }).click(); | ||
| await page.getByRole('button', { name: 'Save' }).click(); | ||
| await page.waitForURL( | ||
| (currentUrl) => !currentUrl.pathname.startsWith('/settings/profile'), | ||
| { timeout: 20000 }, | ||
| ); | ||
| }; | ||
|
|
||
| test.skip( | ||
| !process.env.USER_NAME || !process.env.PASSWORD, | ||
| 'Credentials are required', | ||
| ); | ||
| test.skip( | ||
| ({ browserName, isMobile }) => browserName !== 'chromium' || isMobile, | ||
| 'Run the mutating profile regression once', | ||
| ); | ||
|
|
||
| test('persists a pasted GitHub link when saving without clicking Add', async ({ | ||
| page, | ||
| }) => { | ||
| const handle = `dailydev-e2e-${Date.now()}`; | ||
| const typedUrl = `github.com/${handle}`; | ||
| const savedUrl = `https://github.com/${handle}`; | ||
| const savedLink = page.locator( | ||
| `[data-testid="social-link-github"][href="${savedUrl}"]`, | ||
| ); | ||
|
|
||
| await login(page); | ||
|
|
||
| try { | ||
| await openProfileSettings(page); | ||
| await page.getByRole('textbox', { name: 'Add link' }).fill(typedUrl); | ||
| await page.getByRole('button', { name: 'Save' }).click(); | ||
|
|
||
| await page.waitForURL( | ||
| (currentUrl) => !currentUrl.pathname.startsWith('/settings/profile'), | ||
| { timeout: 20000 }, | ||
| ); | ||
| await expect(savedLink).toBeVisible(); | ||
|
|
||
| await page.reload(); | ||
| await expect(savedLink).toBeVisible(); | ||
| } finally { | ||
| await removeSocialLink(page, savedUrl); | ||
| } | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
164 changes: 164 additions & 0 deletions
164
packages/shared/src/components/profile/SocialLinksInput.spec.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,164 @@ | ||
| import React, { useRef } from 'react'; | ||
| import { render, screen, waitFor } from '@testing-library/react'; | ||
| import userEvent from '@testing-library/user-event'; | ||
| import { FormProvider, useForm } from 'react-hook-form'; | ||
| import type { UserSocialLink } from '../../lib/user'; | ||
| import { | ||
| SocialLinksInput, | ||
| type SocialLinksInputHandle, | ||
| } from './SocialLinksInput'; | ||
|
|
||
| const mockDisplayToast = jest.fn(); | ||
|
|
||
| jest.mock('../../hooks/useToastNotification', () => ({ | ||
| useToastNotification: () => ({ displayToast: mockDisplayToast }), | ||
| })); | ||
|
|
||
| type FormValues = { | ||
| socialLinks: UserSocialLink[]; | ||
| }; | ||
|
|
||
| const TestForm = ({ | ||
| defaultLinks = [], | ||
| isError = false, | ||
| isLoading = false, | ||
| onSubmit, | ||
| }: { | ||
| defaultLinks?: UserSocialLink[]; | ||
| isError?: boolean; | ||
| isLoading?: boolean; | ||
| onSubmit: (values: FormValues) => void; | ||
| }) => { | ||
| const methods = useForm<FormValues>({ | ||
| defaultValues: { | ||
| socialLinks: defaultLinks, | ||
| }, | ||
| }); | ||
| const socialLinksRef = useRef<SocialLinksInputHandle>(null); | ||
|
|
||
| const handleSubmit = methods.handleSubmit(() => { | ||
| if (socialLinksRef.current && !socialLinksRef.current.flushPendingUrl()) { | ||
| return; | ||
| } | ||
|
|
||
| onSubmit(methods.getValues()); | ||
| }); | ||
|
|
||
| return ( | ||
| <FormProvider {...methods}> | ||
| <form onSubmit={handleSubmit}> | ||
| <SocialLinksInput | ||
| ref={socialLinksRef} | ||
| name="socialLinks" | ||
| isLoading={isLoading} | ||
| isError={isError} | ||
| /> | ||
| <button type="submit">Save</button> | ||
| </form> | ||
| </FormProvider> | ||
| ); | ||
| }; | ||
|
|
||
| describe('SocialLinksInput', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('commits pending text before submitting', async () => { | ||
| const onSubmit = jest.fn(); | ||
| render(<TestForm onSubmit={onSubmit} />); | ||
|
|
||
| await userEvent.type( | ||
| screen.getByPlaceholderText('Paste a URL (e.g., github.com/username)'), | ||
| 'github.com/testuser', | ||
| ); | ||
| await userEvent.click(screen.getByRole('button', { name: 'Save' })); | ||
|
|
||
| await waitFor(() => | ||
| expect(onSubmit).toHaveBeenCalledWith({ | ||
| socialLinks: [ | ||
| { | ||
| platform: 'github', | ||
| url: 'https://github.com/testuser', | ||
| }, | ||
| ], | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| it('blocks submit and renders an inline error for invalid pending text', async () => { | ||
| const onSubmit = jest.fn(); | ||
| render(<TestForm onSubmit={onSubmit} />); | ||
|
|
||
| await userEvent.type( | ||
| screen.getByPlaceholderText('Paste a URL (e.g., github.com/username)'), | ||
| '://', | ||
| ); | ||
| await userEvent.click(screen.getByRole('button', { name: 'Save' })); | ||
|
|
||
| expect(onSubmit).not.toHaveBeenCalled(); | ||
| await screen.findByText('Please enter a valid URL'); | ||
| }); | ||
|
|
||
| it('does not commit pending text on blur', async () => { | ||
| const onSubmit = jest.fn(); | ||
| render(<TestForm onSubmit={onSubmit} />); | ||
|
|
||
| const input = screen.getByPlaceholderText( | ||
| 'Paste a URL (e.g., github.com/username)', | ||
| ); | ||
| await userEvent.type(input, 'github.com/testuser'); | ||
| await userEvent.tab(); | ||
|
|
||
| expect( | ||
| screen.queryByText('https://github.com/testuser'), | ||
| ).not.toBeInTheDocument(); | ||
| expect(input).toHaveValue('github.com/testuser'); | ||
| }); | ||
|
|
||
| it('toasts once when submitting a duplicate of an existing link', async () => { | ||
| const onSubmit = jest.fn(); | ||
| render( | ||
| <TestForm | ||
| onSubmit={onSubmit} | ||
| defaultLinks={[ | ||
| { platform: 'github', url: 'https://github.com/testuser' }, | ||
| ]} | ||
| />, | ||
| ); | ||
|
|
||
| await userEvent.type( | ||
| screen.getByPlaceholderText('Paste a URL (e.g., github.com/username)'), | ||
| 'github.com/testuser', | ||
| ); | ||
| await userEvent.click(screen.getByRole('button', { name: 'Save' })); | ||
|
|
||
| await waitFor(() => expect(onSubmit).toHaveBeenCalled()); | ||
| expect(mockDisplayToast).toHaveBeenCalledTimes(1); | ||
| expect(mockDisplayToast).toHaveBeenCalledWith( | ||
| 'This link has already been added', | ||
| ); | ||
| }); | ||
|
|
||
| it('disables adding links while the saved links are loading', () => { | ||
| render(<TestForm onSubmit={jest.fn()} isLoading />); | ||
|
|
||
| expect( | ||
| screen.getByPlaceholderText('Paste a URL (e.g., github.com/username)'), | ||
| ).toBeDisabled(); | ||
| expect(screen.getByRole('button', { name: 'Add' })).toBeDisabled(); | ||
| }); | ||
|
|
||
| it('explains why links are missing when they failed to load', () => { | ||
| render(<TestForm onSubmit={jest.fn()} isError />); | ||
|
|
||
| expect( | ||
| screen.getByText( | ||
| 'We could not load your links. Refresh the page to try again.', | ||
| ), | ||
| ).toBeVisible(); | ||
| expect( | ||
| screen.getByPlaceholderText('Paste a URL (e.g., github.com/username)'), | ||
| ).toBeDisabled(); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Non-blocking: this commit removes the multi-line why-comments from
useUserInfoForm.tsbut adds a seven-line block of the same kind here (why cleanup is best effort, what happens on each failure mode). PerAGENTS.mdthe reasoning belongs in the commit message / PR description, which already carry it. The two//lines insideopenProfileSettingsare enough; suggest dropping this block or cutting it to one line naming the fact ("Only spec that writes to the shared CI account; cleanup is best effort").Reviewed by AI.