Skip to content

Commit

Permalink
Project not found or no access rights messaging (#298)
Browse files Browse the repository at this point in the history
# What's Changed?

- Added 404 and access denied modals that show when project loading
fails
- Slightly amended the readProject request to include the project type
but needs more discussion around URL structure at a later date

## Screenshots

<img width="807" alt="Screenshot 2022-12-15 at 14 47 43"
src="https://user-images.githubusercontent.com/88904316/207890961-d1611c4e-4a22-4a86-86c9-ba625e809dea.png">
<img width="987" alt="Screenshot 2022-12-15 at 14 45 42"
src="https://user-images.githubusercontent.com/88904316/207891160-568fba87-6b50-48c3-bc65-854824b1e145.png">
<img width="811" alt="Screenshot 2022-12-15 at 14 47 03"
src="https://user-images.githubusercontent.com/88904316/207891167-019c0f76-c999-4baf-8769-e4547a142dd9.png">

closes #297

Co-authored-by: loiswells97 <loiswells97@users.noreply.github.com>
Co-authored-by: Lois Wells <lois.wells@raspberrypi.org>
Co-authored-by: Lois Wells <88904316+loiswells97@users.noreply.github.com>
  • Loading branch information
4 people committed Dec 20, 2022
1 parent 736f288 commit f14b31f
Show file tree
Hide file tree
Showing 23 changed files with 519 additions and 28 deletions.
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
### Added

- Plausbile event tracking for login, remix, save and code run (#250)
- Message prompting users to login or save if they make non-autosaved changes (#298)
- Unit tests for the autosave trigger (#298)
- Message prompting users to login or save if they make non-autosaved changes (#291)
- Unit tests for the autosave trigger (#291)
- Project not found and access denied modals shown on project loading error (#298)

## [0.9.0]

Expand Down
2 changes: 1 addition & 1 deletion src/Modal.scss
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
margin-left: calc(-1 * var(--spacing-half));
margin-right: calc(-1 * var(--spacing-half));

button {
button, a {
flex: 1;
margin: 0 var(--spacing-half);

Expand Down
2 changes: 2 additions & 0 deletions src/components/Button/Button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
font-family: sans-serif;
font-size: inherit;
font-weight: var(--font-weight-regular);
justify-content: center;
margin: 6px;
padding: 10px;
position: relative;
Expand All @@ -33,6 +34,7 @@

&--secondary {
background-color: $editor-light-grey;
color: $editor-black;
&:hover {
background-color: $editor-mid-light-grey;
}
Expand Down
36 changes: 34 additions & 2 deletions src/components/Editor/EditorSlice.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import { createOrUpdateProject, readProject, createRemix } from '../../utils/api

export const syncProject = (actionName) => createAsyncThunk(
`editor/${actionName}Project`,
async({ project, identifier, accessToken, autosave }, { rejectWithValue }) => {
async({ project, identifier, projectType, accessToken, autosave }, { rejectWithValue }) => {
let response
switch(actionName) {
case 'load':
response = await readProject(identifier, accessToken)
response = await readProject(identifier, projectType, accessToken)
break
case 'remix':
response = await createRemix(project, accessToken)
Expand Down Expand Up @@ -65,8 +65,11 @@ export const EditorSlice = createSlice({
lastSavedTime: null,
senseHatAlwaysEnabled: false,
senseHatEnabled: false,
accessDeniedNoAuthModalShowing: false,
accessDeniedWithAuthModalShowing: false,
betaModalShowing: false,
loginToSaveModalShowing: false,
notFoundModalShowing: false,
renameFileModalShowing: false,
modals: {},
},
Expand Down Expand Up @@ -167,6 +170,13 @@ export const EditorSlice = createSlice({
setProjectListLoaded: (state, action) => {
state.projectListLoaded = action.payload;
},
closeAccessDeniedNoAuthModal: (state) => {
state.accessDeniedNoAuthModalShowing = false
state.modals = {}
},
closeAccessDeniedWithAuthModal: (state) => {
state.accessDeniedWithAuthModalShowing = false
},
showBetaModal: (state) => {
state.betaModalShowing = true
},
Expand All @@ -179,6 +189,9 @@ export const EditorSlice = createSlice({
closeLoginToSaveModal: (state) => {
state.loginToSaveModalShowing = false
},
closeNotFoundModal: (state) => {
state.notFoundModalShowing = false
},
showRenameFileModal: (state, action) => {
state.modals.renameFile = action.payload
state.renameFileModalShowing = true
Expand Down Expand Up @@ -215,6 +228,8 @@ export const EditorSlice = createSlice({
})
builder.addCase('editor/loadProject/pending', (state, action) => {
state.loading = 'pending'
state.accessDeniedNoAuthModalShowing = false
state.modals = {}
state.currentLoadingRequestId = action.meta.requestId
})
builder.addCase('editor/loadProject/fulfilled', (state, action) => {
Expand All @@ -226,10 +241,24 @@ export const EditorSlice = createSlice({
state.currentLoadingRequestId = undefined
}
})

builder.addCase('editor/loadProject/rejected', (state, action) => {
if (state.loading === 'pending' && state.currentLoadingRequestId === action.meta.requestId) {
state.loading = 'failed'
state.saving = 'idle'
const splitErrorMessage = action.error.message.split(' ')
const errorCode = splitErrorMessage[splitErrorMessage.length - 1]
if (errorCode === '404') {
state.notFoundModalShowing = true
} else if ((errorCode === '500' || errorCode === '403') && action.meta.arg.accessToken) {
state.accessDeniedWithAuthModalShowing = true
} else if ((errorCode === '500' || errorCode === '403') && !action.meta.arg.accessToken) {
state.accessDeniedNoAuthModalShowing = true
state.modals.accessDenied = {
identifier: action.meta.arg.identifier,
projectType: action.meta.arg.projectType
}
}
state.currentLoadingRequestId = undefined
}
})
Expand Down Expand Up @@ -261,10 +290,13 @@ export const {
updateImages,
updateProjectComponent,
updateProjectName,
closeAccessDeniedNoAuthModal,
closeAccessDeniedWithAuthModal,
showBetaModal,
closeBetaModal,
showLoginToSaveModal,
closeLoginToSaveModal,
closeNotFoundModal,
showRenameFileModal,
closeRenameFileModal,
} = EditorSlice.actions
Expand Down
6 changes: 3 additions & 3 deletions src/components/Editor/EditorSlice.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ describe('When requesting a project', () => {

beforeEach(() => {
loadThunk = syncProject('load')
loadAction = loadThunk({ identifier: 'my-project-identifier', accessToken: 'my_token' })
loadAction = loadThunk({ identifier: 'my-project-identifier', projectType: 'python', accessToken: 'my_token' })

loadFulfilledAction = loadThunk.fulfilled({ project })
loadFulfilledAction.meta.requestId='my_request_id'
Expand All @@ -251,7 +251,7 @@ describe('When requesting a project', () => {

test('Reads project from database', async () => {
await loadAction(dispatch, () => initialState)
expect(readProject).toHaveBeenCalledWith('my-project-identifier', 'my_token')
expect(readProject).toHaveBeenCalledWith('my-project-identifier', 'python', 'my_token')
})

test('If loading status pending, loading success updates status', () => {
Expand All @@ -264,7 +264,7 @@ describe('When requesting a project', () => {
justLoaded: true,
saving: 'idle',
project: project,
currentLoadingRequestId: undefined
currentLoadingRequestId: undefined,
}
expect(reducer(initialState, loadFulfilledAction)).toEqual(expectedState)
})
Expand Down
2 changes: 1 addition & 1 deletion src/components/Editor/Hooks/useProject.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const useProject = (projectType, projectIdentifier = null, accessToken =
}

if (projectIdentifier) {
dispatch(syncProject('load')({identifier: projectIdentifier, accessToken}));
dispatch(syncProject('load')({identifier: projectIdentifier, projectType, accessToken}));
return;
}

Expand Down
6 changes: 3 additions & 3 deletions src/components/Editor/Hooks/useProject.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const loadProject = jest.fn()
jest.mock('../EditorSlice')

jest.mock('../../../utils/apiCallHandler', () => ({
readProject: async (identifier) => Promise.resolve({'data': {'identifier': identifier, 'project_type': 'python'}})
readProject: async (identifier, projectType) => Promise.resolve({'data': {'identifier': identifier, 'project_type': projectType}})
}))

const cachedProject = {
Expand Down Expand Up @@ -64,7 +64,7 @@ test("If cached project does not match identifer loads correct uncached project"
syncProject.mockImplementationOnce(jest.fn((_) => (loadProject)))
localStorage.setItem('project', JSON.stringify(cachedProject))
renderHook(() => useProject('python', project1.identifier, accessToken))
await waitFor(() => expect(loadProject).toHaveBeenCalledWith({ identifier: project1.identifier, accessToken }))
await waitFor(() => expect(loadProject).toHaveBeenCalledWith({ identifier: project1.identifier, projectType: project1.project_type, accessToken }))
})

test("If cached project does not match identifer clears cached project", () => {
Expand All @@ -76,7 +76,7 @@ test("If cached project does not match identifer clears cached project", () => {
test("If no cached project loads uncached project", async () => {
syncProject.mockImplementationOnce(jest.fn((_) => (loadProject)))
renderHook(() => useProject('python', 'hello-world-project', accessToken))
await waitFor(() => expect(loadProject).toHaveBeenCalledWith({ identifier: 'hello-world-project', accessToken }))
await waitFor(() => expect(loadProject).toHaveBeenCalledWith({ identifier: 'hello-world-project', projectType: 'python', accessToken }))
})

afterEach(() => {
Expand Down
9 changes: 9 additions & 0 deletions src/components/Editor/Project/Project.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import RenameFile from '../../Modals/RenameFile'
import RunnerControls from '../../RunButton/RunnerControls'
import { expireJustLoaded, setHasShownSavePrompt, syncProject } from '../EditorSlice';
import { isOwner } from '../../../utils/projectHelpers'
import NotFoundModal from '../../Modals/NotFoundModal';
import AccessDeniedNoAuthModal from '../../Modals/AccessDeniedNoAuthModal';
import AccessDeniedWithAuthModal from '../../Modals/AccessDeniedWithAuthModal';
import { showLoginPrompt, showSavePrompt } from '../../../utils/Notifications';

const Project = (props) => {
Expand All @@ -22,6 +25,9 @@ const Project = (props) => {
const project = useSelector((state) => state.editor.project)
const modals = useSelector((state) => state.editor.modals)
const renameFileModalShowing = useSelector((state) => state.editor.renameFileModalShowing)
const notFoundModalShowing = useSelector((state) => state.editor.notFoundModalShowing)
const accessDeniedNoAuthModalShowing = useSelector((state) => state.editor.accessDeniedNoAuthModalShowing)
const accessDeniedWithAuthModalShowing = useSelector((state) => state.editor.accessDeniedWithAuthModalShowing)
const justLoaded = useSelector((state) => state.editor.justLoaded)
const hasShownSavePrompt = useSelector((state) => state.editor.hasShownSavePrompt)

Expand Down Expand Up @@ -72,6 +78,9 @@ const Project = (props) => {
<Output />
</div>
{(renameFileModalShowing && modals.renameFile) ? <RenameFile /> : null}
{(notFoundModalShowing) ? <NotFoundModal /> : null}
{(accessDeniedNoAuthModalShowing) ? <AccessDeniedNoAuthModal /> : null}
{(accessDeniedWithAuthModalShowing) ? <AccessDeniedWithAuthModal /> : null}
</div>
)
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,16 @@ const ProjectComponentLoader = (props) => {
if (loading === 'idle' && project.identifier) {
history.push(`/${project.project_type}/${project.identifier}`)
}
if (loading === 'failed') {
history.push('/')
}
}, [loading, project, history])


return loading === 'success' ? (
<Project />
) : loading === 'failed' ? (
<p>{t('project.notFound')}</p>
) : <p>{t('project.loading')}</p>;
) : loading === 'pending' ? (
<p>{t('project.loading')}</p>
) : null
};

export default ProjectComponentLoader;
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,17 @@ import { render, screen } from "@testing-library/react"
import { Provider } from 'react-redux';
import configureStore from 'redux-mock-store';
import ProjectComponentLoader from "./ProjectComponentLoader";
import { setProject } from "../EditorSlice";
import { defaultPythonProject } from "../../../utils/defaultProjects";

test("Renders loading message if projectloaded is pending", () => {
jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useHistory: () => ({
push: jest.fn()
})
}));

test("Renders loading message if loading is pending", () => {
const middlewares = []
const mockStore = configureStore(middlewares)
const initialState = {
Expand All @@ -18,7 +27,7 @@ test("Renders loading message if projectloaded is pending", () => {
expect(screen.queryByText('project.loading')).toBeInTheDocument()
})

test("Renders failed message if projectloaded is failed", () => {
test("Loads default project if loading fails", () => {
const middlewares = []
const mockStore = configureStore(middlewares)
const initialState = {
Expand All @@ -29,10 +38,11 @@ test("Renders failed message if projectloaded is failed", () => {
}
const store = mockStore(initialState);
render(<Provider store={store}><ProjectComponentLoader match={{params: {}}}/></Provider>)
expect(screen.queryByText('project.notFound')).toBeInTheDocument()
const expectedActions = [setProject(defaultPythonProject)]
expect(store.getActions()).toEqual(expectedActions)
})

test("Does not render loading or failed message if projectloaded is success", () => {
test("Does not render loading message if loading is success", () => {
const middlewares = []
const mockStore = configureStore(middlewares)
const initialState = {
Expand All @@ -47,5 +57,4 @@ test("Does not render loading or failed message if projectloaded is success", ()
const store = mockStore(initialState);
render(<Provider store={store}><div id='app'></div><ProjectComponentLoader match={{params: {}}}/></Provider>)
expect(screen.queryByText('project.loading')).not.toBeInTheDocument()
expect(screen.queryByText('project.notFound')).not.toBeInTheDocument()
})
9 changes: 7 additions & 2 deletions src/components/Login/LoginButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,17 @@ const LoginButton = (props) => {
const { buttonText, className } = props;
const location = useLocation()
const project = useSelector((state) => state.editor.project)
const accessDeniedData = useSelector((state) => state.editor.modals.accessDenied)

const onLoginButtonClick = (event) => {
event.preventDefault();
window.plausible('Login button')
localStorage.setItem('location', location.pathname)
localStorage.setItem(project.identifier || 'project', JSON.stringify(project))
if (accessDeniedData) {
localStorage.setItem('location', `/${accessDeniedData.projectType}/${accessDeniedData.identifier}`)
} else {
localStorage.setItem('location', location.pathname)
localStorage.setItem(project.identifier || 'project', JSON.stringify(project))
}
userManager.signinRedirect();
}

Expand Down
3 changes: 2 additions & 1 deletion src/components/Login/LoginButton.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ beforeEach(() => {
const mockStore = configureStore(middlewares)
const initialState = {
editor: {
project: project
project: project,
modals: {}
},
auth: {
user: null
Expand Down
3 changes: 2 additions & 1 deletion src/components/Login/LoginMenu.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ describe('When not logged in', () => {
const mockStore = configureStore(middlewares)
const initialState = {
editor: {
project: {}
project: {},
modals: {}
},
auth: {
user: null
Expand Down
1 change: 1 addition & 0 deletions src/components/Menus/Dropdown/Dropdown.scss
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
color: inherit;
cursor: pointer;
font: inherit;
justify-content: left;
font-weight: var(--font-weight-regular);
text-decoration: none;
padding: var(--spacing-1) var(--spacing-half);
Expand Down
50 changes: 50 additions & 0 deletions src/components/Modals/AccessDeniedNoAuthModal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import React from "react";
import Modal from 'react-modal';
import { useDispatch, useSelector } from "react-redux";
import { useTranslation } from "react-i18next";

import Button from "../Button/Button";
import '../../Modal.scss';
import { closeAccessDeniedNoAuthModal } from "../Editor/EditorSlice";
import { CloseIcon } from "../../Icons";
import LoginButton from "../Login/LoginButton";

const AccessDeniedNoAuthModal = () => {
const dispatch = useDispatch()
const { t } = useTranslation();

const isModalOpen = useSelector((state) => state.editor.accessDeniedNoAuthModalShowing)
const closeModal = () => dispatch(closeAccessDeniedNoAuthModal());

return (
<>
<Modal
isOpen={isModalOpen}
onRequestClose={closeModal}
className='modal-content'
overlayClassName='modal-overlay'
contentLabel={t('project.accessDeniedNoAuthModal.heading')}
parentSelector={() => document.querySelector('#app')}
appElement={document.getElementById('app') || undefined}
>
<div className='modal-content__header'>
<h2 className='modal-content__heading'>{t('project.accessDeniedNoAuthModal.heading')}</h2>
<button onClick={closeModal}>
<CloseIcon/>
</button>
</div>
<p className='modal-content__text'>{t('project.accessDeniedNoAuthModal.text')}</p>

<div className='modal-content__buttons' >
<a className='btn btn--secondary' href='https://projects.raspberrypi.org'>{t('project.accessDeniedNoAuthModal.projectsSiteLinkText')}</a>
<LoginButton className='btn--primary' buttonText={t('project.accessDeniedNoAuthModal.loginButtonText')} />
</div>
<div className='modal-content__links'>
<Button buttonText = {t('project.accessDeniedNoAuthModal.newProject')} className='btn--tertiary' onClickHandler={closeModal}/>
</div>
</Modal>
</>
);
}

export default AccessDeniedNoAuthModal;
Loading

0 comments on commit f14b31f

Please sign in to comment.