Skip to content

Commit f4cefae

Browse files
tudddorrrtudddorrr
authored
Merge pull request #250 from TaloDev/404mess
redo the way intended routes are handled
2 parents ab02dc5 + 766f13d commit f4cefae

File tree

11 files changed

+108
-49
lines changed

11 files changed

+108
-49
lines changed

‎cypress/e2e/pages/Login.spec.cy.js‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe('Login', () => {
88
})
99

1010
cy.visitAsGuest()
11-
cy.findByLabelText('Email').type('admin@trytalo.com')
11+
cy.findByLabelText('Email').type('dev@trytalo.com')
1212
cy.findByLabelText('Password').type('password')
1313
cy.findByText('Login').click()
1414

@@ -24,10 +24,41 @@ describe('Login', () => {
2424
})
2525

2626
cy.visitAsGuest()
27-
cy.findByLabelText('Email').type('admin@trytalo.com')
27+
cy.findByLabelText('Email').type('dev@trytalo.com')
2828
cy.findByLabelText('Password').type('passwor')
2929
cy.findByText('Login').click()
3030

3131
cy.findByText('Incorrect email address or password').should('exist')
3232
})
33+
34+
it('should redirect to the intended route after logging in', () => {
35+
cy.intercept('POST', 'http://talo.api/public/users/login', {
36+
statusCode: 200,
37+
fixture: 'responses/auth/dev'
38+
})
39+
40+
cy.visitAsGuest('/account')
41+
cy.findByLabelText('Email').type('[email protected]')
42+
cy.findByLabelText('Password').type('password')
43+
cy.findByText('Login').click()
44+
45+
cy.findByText('Dev').should('exist')
46+
cy.findByText('[email protected]').should('exist')
47+
cy.location('pathname').should('eq', '/account')
48+
})
49+
50+
it('should redirect to the dashboard after logging in if the intended route does not exist', () => {
51+
cy.intercept('POST', 'http://talo.api/public/users/login', {
52+
statusCode: 200,
53+
fixture: 'responses/auth/dev'
54+
})
55+
56+
cy.visitAsGuest('/does-not-exist')
57+
cy.findByLabelText('Email').type('[email protected]')
58+
cy.findByLabelText('Password').type('password')
59+
cy.findByText('Login').click()
60+
61+
cy.findByText('Superstatic dashboard').should('exist')
62+
cy.location('pathname').should('eq', '/')
63+
})
3364
})

‎cypress/fixtures/responses/auth/dev.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
"id": 1,
55
"type": 2,
66
"username": "Dev",
7+
"email": "[email protected]",
78
"organisation": {
89
"games": [
910
{

‎src/App.jsx‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { useState, useEffect, Suspense } from 'react'
2-
import { useNavigate } from 'react-router-dom'
32
import * as Sentry from '@sentry/react'
43
import { useRecoilState, useRecoilValue, useSetRecoilState } from 'recoil'
54
import refreshAccess from './api/refreshAccess'
@@ -28,8 +27,6 @@ function App() {
2827
const games = useRecoilValue(gamesState)
2928
const [activeGame, setActiveGame] = useRecoilState(activeGameState)
3029

31-
const navigate = useNavigate()
32-
3330
const handleRefreshSession = async () => {
3431
try {
3532
const res = await refreshAccess()
@@ -53,8 +50,6 @@ function App() {
5350
useEffect(() => {
5451
if (!hasTriedRefreshing && intendedUrl) {
5552
handleRefreshSession()
56-
} else if (hasTriedRefreshing) {
57-
navigate(intendedUrl, { replace: true })
5853
}
5954
}, [intendedUrl, hasTriedRefreshing])
6055

‎src/Router.jsx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import { lazy } from 'react'
22
import PropTypes from 'prop-types'
3-
import { Navigate, Route, Routes } from 'react-router-dom'
3+
import { Route, Routes } from 'react-router-dom'
44
import { useRecoilValue } from 'recoil'
55
import userState from './state/userState'
66
import NavBar from './components/NavBar'
77
import routes from './constants/routes'
88
import activeGameState from './state/activeGameState'
99
import AuthService from './services/AuthService'
1010
import canViewPage from './utils/canViewPage'
11+
import NotFoundHandler from './components/NotFoundHandler'
1112

1213
const Login = lazy(() => import(/* webpackChunkName: 'login' */ './pages/Login'))
1314
const Dashboard = lazy(() => import(/* webpackChunkName: 'dashboard' */ './pages/Dashboard'))
@@ -59,7 +60,7 @@ function Router({ intendedUrl }) {
5960
<Route exact path={routes.forgotPassword} element={<ForgotPassword />} />
6061
<Route exact path={routes.resetPassword} element={<ResetPassword />} />
6162

62-
<Route path='*' element={<Navigate to={`${routes.login}?next=${encodeURIComponent(intendedUrl)}`} replace />} />
63+
<Route path='*' element={<NotFoundHandler baseRoute={routes.login} intendedUrl={intendedUrl} />} />
6364
</Routes>
6465
</main>
6566
}
@@ -98,7 +99,7 @@ function Router({ intendedUrl }) {
9899
</>
99100
}
100101

101-
<Route path='*' element={<Navigate to={routes.dashboard} replace />} />
102+
<Route path='*' element={<NotFoundHandler baseRoute={routes.dashboard} intendedUrl={intendedUrl} />} />
102103
</Routes>
103104
</main>
104105
</div>

‎src/components/NavBar.jsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@ const NavBar = () => {
1919
const onLogoutClick = async () => {
2020
try {
2121
/* v8ignore next */
22-
Sentry.configureScope((scope) => scope.setUser(null))
22+
Sentry.setUser(null)
2323
setActiveGame(null)
24+
2425
window.localStorage.removeItem('loggedOut')
26+
window.sessionStorage.removeItem('intendedRouteChecked')
27+
2528
await logout()
2629
} catch (err) {
2730
console.warn('Logout failed:', err.message)

‎src/components/NotFoundHandler.jsx‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { useEffect } from 'react'
2+
import PropTypes from 'prop-types'
3+
import { Navigate } from 'react-router-dom'
4+
5+
// 1. user hits dashboard.trytalo.com/leaderboards but is logged out
6+
// 2. user is redirected to dashboard.trytalo.com/login?next=%2Fleaderboards
7+
// 3. "next" search param is put into sessionStorage
8+
// 4. user logs in, /login is no longer a route so is redirected to dashboard.trytalo.com/?next=%2Fleaderboards
9+
// 5. useIntendedRoute hook picks up logic to redirect based on the sessionStorage
10+
export default function NotFoundHandler({ baseRoute, intendedUrl }) {
11+
useEffect(() => {
12+
const intendedRoute = new URLSearchParams(window.location.search).get('next')
13+
if (intendedRoute) {
14+
window.sessionStorage.setItem('intendedRoute', intendedRoute)
15+
}
16+
}, [])
17+
18+
return (
19+
<Navigate to={`${baseRoute}?next=${encodeURIComponent(intendedUrl)}`} replace />
20+
)
21+
}
22+
23+
NotFoundHandler.propTypes = {
24+
intendedUrl: PropTypes.string.isRequired,
25+
baseRoute: PropTypes.string.isRequired
26+
}

‎src/pages/Dashboard.jsx‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11

2-
import { useState, useEffect } from 'react'
3-
import { useNavigate } from 'react-router-dom'
42
import { useRecoilValue } from 'recoil'
53
import useHeadlines from '../api/useHeadlines'
64
import ErrorMessage from '../components/ErrorMessage'
@@ -16,6 +14,7 @@ import devDataState from '../state/devDataState'
1614
import SecondaryNav from '../components/SecondaryNav'
1715
import DevDataStatus from '../components/DevDataStatus'
1816
import SecondaryTitle from '../components/SecondaryTitle'
17+
import useIntendedRoute from '../utils/useIntendedRoute'
1918

2019
export const secondaryNavRoutes = [
2120
{ title: 'Dashboard', to: routes.dashboard },
@@ -25,22 +24,8 @@ export const secondaryNavRoutes = [
2524
]
2625

2726
const Dashboard = () => {
28-
const navigate = useNavigate()
29-
30-
const [intendedRouteChecked, setIntendedRouteChecked] = useState(false)
31-
3227
const includeDevData = useRecoilValue(devDataState)
3328

34-
useEffect(() => {
35-
const intended = window.localStorage.getItem('intendedRoute')
36-
if (intended) {
37-
window.localStorage.removeItem('intendedRoute')
38-
navigate(intended, { replace: true })
39-
} else {
40-
setIntendedRouteChecked(true)
41-
}
42-
}, [])
43-
4429
const activeGame = useRecoilValue(activeGameState)
4530

4631
const timePeriods = [
@@ -56,6 +41,8 @@ const Dashboard = () => {
5641
const { headlines, loading: headlinesLoading, error: headlinesError } = useHeadlines(activeGame, startDate, endDate, includeDevData)
5742
const { stats, loading: statsLoading, error: statsError } = useStats(activeGame, includeDevData)
5843

44+
const intendedRouteChecked = useIntendedRoute()
45+
5946
if (!intendedRouteChecked) return null
6047

6148
if (!activeGame) {

‎src/pages/Login.jsx‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,6 @@ const Login = () => {
2525
const navigate = useNavigate()
2626
const location = useLocation()
2727

28-
useEffect(() => {
29-
const intendedRoute = new URLSearchParams(window.location.search).get('next')
30-
31-
if (intendedRoute) {
32-
window.localStorage.setItem('intendedRoute', intendedRoute)
33-
navigate(window.location.pathname, { replace: true })
34-
} else {
35-
window.localStorage.removeItem('intendedRoute')
36-
}
37-
}, [])
38-
3928
useEffect(() => {
4029
if (wasLoggedOut) {
4130
setTimeout(() => {

‎src/pages/__tests__/Dashboard.test.jsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,9 @@ describe('<Dashboard />', () => {
138138
const locationMock = vi.fn()
139139
window.localStorage.setItem('intendedRoute', '/players')
140140

141+
// easier to mock localStorage
142+
window.sessionStorage = window.localStorage
143+
141144
render(
142145
<KitchenSink
143146
states={[

‎src/utils/handleAccessToken.js‎

Lines changed: 0 additions & 11 deletions
This file was deleted.

0 commit comments

Comments
 (0)