Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix issue that requires multiple logout clicks #123

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/components/Logout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ export default function Logout() {

const handleLogout = () => {
logout()
navigate('/', { replace: true })
navigate(0)
// Wait for next tick to ensure token cleanup has completed
setTimeout(() => {
navigate('/', { replace: true })
}, 0)
}

return (
Expand Down
6 changes: 6 additions & 0 deletions src/providers/AuthProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,14 @@ const AuthProvider = ({ children }: IAuthProviderProps) => {
}

const logout = (returnTo?: string | null) => {
// Clear token first
delete API.defaults.headers.common['Authorization']
localStorage.removeItem('token')
// Then update state
setToken()
setReturnTo(returnTo ?? null)
// Dispatch event immediately
window.dispatchEvent(new Event('tokenChange'))
}

const contextValue = useMemo(
Expand Down
76 changes: 76 additions & 0 deletions src/tests/components/Logout.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { render, screen, fireEvent } from '@testing-library/react'
import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'
import { BrowserRouter } from 'react-router-dom'
import Logout from '@/components/Logout'
import { AuthContext } from '@/providers/AuthProvider'

// Mock the navigate function
const mockNavigate = vi.fn()
vi.mock('react-router-dom', async () => {
const actual = await vi.importActual('react-router-dom')
return {
...actual,
useNavigate: () => mockNavigate,
}
})

describe('Logout Component', () => {
const mockLogout = vi.fn()
const mockSetToken = vi.fn()
const mockLogin = vi.fn()
const mockToken = 'test-token'

const mockAuthContext = {
token: mockToken,
setToken: mockSetToken,
login: mockLogin,
logout: mockLogout,
}

beforeEach(() => {
// Clear all mocks before each test
vi.clearAllMocks()
// Reset timers before each test
vi.useFakeTimers()
})

afterEach(() => {
vi.useRealTimers()
})

it('signs out with a single click', () => {
// Arrange
render(
<BrowserRouter>
<AuthContext.Provider value={mockAuthContext}>
<Logout />
</AuthContext.Provider>
</BrowserRouter>,
)

// Act
const signOutButton = screen.getByRole('button', { name: /sign out/i })
fireEvent.click(signOutButton)
vi.runAllTimers()

// Assert
expect(mockLogout).toHaveBeenCalledTimes(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

I also wonder if we could replace all these mocks with the fake API? have a look in the mocks directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's a fab idea, though I don't think we have a logout route in our API?

expect(mockNavigate).toHaveBeenCalledTimes(1)
expect(mockNavigate).toHaveBeenCalledWith('/', { replace: true })
})

it('does not show sign out button when not logged in', () => {
// Arrange
render(
<BrowserRouter>
<AuthContext.Provider value={{ ...mockAuthContext, token: null }}>
<Logout />
</AuthContext.Provider>
</BrowserRouter>,
)

// Assert
const signOutButton = screen.queryByRole('button', { name: /sign out/i })
expect(signOutButton).not.toBeInTheDocument()
})
})
Loading