From 964bdf0d2a16a2b71b7ea132410df815c690c72b Mon Sep 17 00:00:00 2001 From: cv <34649812+cpvalente@users.noreply.github.com> Date: Sun, 11 Apr 2021 19:17:54 +0200 Subject: [PATCH] cleanup + small refract - websockets - event api --- client/src/app/api/eventsApi.js | 4 +- .../common/components/countdown/Countdown.jsx | 3 - client/src/common/input/EditableText.jsx | 16 +++- .../src/common/input/EditableText.module.css | 13 +++ .../src/features/control/MessageControl.jsx | 10 +- .../src/features/control/PlaybackControl.jsx | 9 +- .../editors/list/EventListWrapper.jsx | 39 ++++---- server/app.js | 93 +------------------ server/controllers/eventsController.js | 17 +--- server/controllers/socketController.js | 76 +++++++++++++++ server/routes/eventsRouter.js | 5 +- server/routes/playbackRouter.js | 9 +- 12 files changed, 145 insertions(+), 149 deletions(-) create mode 100644 server/controllers/socketController.js diff --git a/client/src/app/api/eventsApi.js b/client/src/app/api/eventsApi.js index 2fa80fd1d..d55ede305 100644 --- a/client/src/app/api/eventsApi.js +++ b/client/src/app/api/eventsApi.js @@ -1,8 +1,8 @@ import { serverURL } from './apiConstants'; -export const eventsURL = serverURL + 'events/'; +export const eventsURL = serverURL + 'events'; export const fetchAllEvents = async () => { - const res = await fetch(eventsURL + 'all'); + const res = await fetch(eventsURL); // TODO: Safe json convert return res.json(); }; \ No newline at end of file diff --git a/client/src/common/components/countdown/Countdown.jsx b/client/src/common/components/countdown/Countdown.jsx index 73cd9ce22..a4b77b6e2 100644 --- a/client/src/common/components/countdown/Countdown.jsx +++ b/client/src/common/components/countdown/Countdown.jsx @@ -14,11 +14,8 @@ export default function Countdown(props) { const [clock, setClock] = useState(time); let display = '-- : -- : --'; - console.log('websocket: time and t', time); - useEffect(() => { setClock(time); - console.log('websocket: time changed', time) }, [time]); // prepare display string diff --git a/client/src/common/input/EditableText.jsx b/client/src/common/input/EditableText.jsx index 3ec553e0c..dbc773213 100644 --- a/client/src/common/input/EditableText.jsx +++ b/client/src/common/input/EditableText.jsx @@ -2,9 +2,17 @@ import { Editable, EditableInput, EditablePreview } from '@chakra-ui/editable'; import style from './EditableText.module.css'; export default function EditableText(props) { - const { label, defaultValue, placeholder, handleSubmit } = props; + const { label, defaultValue, placeholder, submitHandler } = props; + + const handleSubmit = (submitedVal) => { + // No need to update if it hasnt changed + if (submitedVal === defaultValue) return; + + submitHandler(submitedVal); + }; + return ( -
+
{label} @@ -12,10 +20,10 @@ export default function EditableText(props) { onSubmit={(v) => handleSubmit(v)} defaultValue={defaultValue} placeholder={placeholder} - style={{ display: 'inline' }} + className={style.inline} > - +
); diff --git a/client/src/common/input/EditableText.module.css b/client/src/common/input/EditableText.module.css index 52383f94a..43659424e 100644 --- a/client/src/common/input/EditableText.module.css +++ b/client/src/common/input/EditableText.module.css @@ -10,4 +10,17 @@ .titleUnderlined { border-bottom: 1px solid #0001; +} + +.block { + display: 'block'; +} + +.inline { + display: inline; +} + +.editable13 { + width: '13em'; + min-width: '13em'; } \ No newline at end of file diff --git a/client/src/features/control/MessageControl.jsx b/client/src/features/control/MessageControl.jsx index fc30fbb0a..0c486b269 100644 --- a/client/src/features/control/MessageControl.jsx +++ b/client/src/features/control/MessageControl.jsx @@ -23,19 +23,16 @@ export default function MessageControl() { visible: false, }); - // Torbjorn: why is this not updating? useEffect(() => { if (socket == null) return; // Handle presenter messages socket.on('messages-presenter', (data) => { - console.log('websocket: got data', data); setPres({ ...data }); }); // Handle public messages socket.on('messages-public', (data) => { - console.log('websocket: got data', data); setPubl({ ...data }); }); @@ -43,8 +40,11 @@ export default function MessageControl() { socket.emit('get-presenter'); socket.emit('get-public'); - // Clear listener - return () => socket.off('messages-presenter', 'messages-public'); + // Clear listeners + return () => { + socket.off('messages-public'); + socket.off('messages-presenter'); + }; }, [socket]); const messageControl = async (action, payload) => { diff --git a/client/src/features/control/PlaybackControl.jsx b/client/src/features/control/PlaybackControl.jsx index fe8d581a4..35ea80f7d 100644 --- a/client/src/features/control/PlaybackControl.jsx +++ b/client/src/features/control/PlaybackControl.jsx @@ -31,8 +31,8 @@ const size = { }; export default function PlaybackControl() { - const [playback, setPlayback] = useState(null); const socket = useSocket(); + const [playback, setPlayback] = useState(null); const [timer, setTimer] = useState({ currentSeconds: null, startedAt: null, @@ -42,13 +42,10 @@ export default function PlaybackControl() { // handle incoming messages useEffect(() => { if (socket == null) return; - - // Subscribe to timer event - socket.emit('subscribe-to-timer'); - // ask for playstate socket.emit('get-playstate'); + // Handle playstate socket.on('playstate', (data) => { setPlayback(data); }); @@ -60,7 +57,7 @@ export default function PlaybackControl() { // Clear listener return () => { - socket.emit('release-timer'); + socket.off('playstate'); socket.off('timer'); }; }, [socket]); diff --git a/client/src/features/editors/list/EventListWrapper.jsx b/client/src/features/editors/list/EventListWrapper.jsx index 4a35351e0..5b8138330 100644 --- a/client/src/features/editors/list/EventListWrapper.jsx +++ b/client/src/features/editors/list/EventListWrapper.jsx @@ -1,4 +1,4 @@ -import { useMutation, useQuery, useQueryClient } from 'react-query'; +import { useMutation, useQuery } from 'react-query'; import { useEffect } from 'react'; import { fetchAllEvents } from '../../../app/api/eventsApi.js'; import EventList from './EventList'; @@ -10,13 +10,12 @@ import { Skeleton } from '@chakra-ui/skeleton'; import style from './List.module.css'; export default function EventListWrapper() { - const { data, status, isError } = useQuery('events', fetchAllEvents); - const queryClient = useQueryClient(); - // TODO: Move to events API? + const { data, status, isError, refetch } = useQuery('events', fetchAllEvents); const addEvent = useMutation((data) => axios.post(eventsURL, data)); const updateEvent = useMutation((data) => axios.put(eventsURL, data)); + const patchEvent = useMutation((data) => axios.patch(eventsURL, data)); const deleteEvent = useMutation((eventId) => - axios.delete(eventsURL + eventId) + axios.delete(eventsURL + '/' + eventId) ); // Show toasts on errors @@ -28,34 +27,35 @@ export default function EventListWrapper() { // Events API const eventsHandler = async (action, payload) => { - // Torbjorn: is this a good way to do it? - // How do I handle the mutation thing - // https://react-query.tanstack.com/guides/invalidations-from-mutations - // https://react-query.tanstack.com/guides/updates-from-mutation-responses + let needsRefetch = false; switch (action) { case 'add': try { - await addEvent - .mutateAsync(payload) - .then(queryClient.invalidateQueries('events')); + await addEvent.mutateAsync(payload).then((needsRefetch = true)); } catch (error) { showErrorToast('Error creating event', error.message); } break; case 'update': try { - await updateEvent - .mutateAsync(payload) - .then(queryClient.invalidateQueries('events')); + await updateEvent.mutateAsync(payload).then((needsRefetch = true)); + // TODO: instead of refetching, update the item here + } catch (error) { + showErrorToast('Error updating event', error.message); + } + break; + case 'patch': + try { + await patchEvent.mutateAsync(payload).then((needsRefetch = true)); + // TODO: instead of refetching, update the item here } catch (error) { showErrorToast('Error updating event', error.message); } break; case 'delete': try { - await deleteEvent - .mutateAsync(payload) - .then(queryClient.invalidateQueries('events')); + await deleteEvent.mutateAsync(payload).then((needsRefetch = true)); + needsRefetch = true; } catch (error) { showErrorToast('Error deleting event', error.message); } @@ -64,6 +64,9 @@ export default function EventListWrapper() { showErrorToast('Unrecognised request', action); break; } + if (needsRefetch) { + refetch(); + } }; return ( diff --git a/server/app.js b/server/app.js index c28980545..428db510e 100644 --- a/server/app.js +++ b/server/app.js @@ -4,7 +4,6 @@ const config = require('./config.json'); // dependencies const express = require('express'); const http = require('http'); -const socketIo = require('socket.io'); const cors = require('cors'); // Import Routes @@ -21,6 +20,9 @@ let durationForNow = 5400; global.timer = new EventTimer(); timer.setupWithSeconds(durationForNow, true); +// Socket +const initiateSocket = require('./controllers/socketController.js'); + // Create express APP const app = express(); @@ -44,93 +46,8 @@ app.use((err, req, res, next) => { // create HTTP server const server = http.createServer(app); -// initialise socketIO server -const io = socketIo(server, { - cors: { - origin: 'http://localhost:3000', - methods: ['GET', 'POST'], - }, -}); - -// Torbjorn: should the interval be here or inside the connection? -// I am guessing one interval per timer -// interval function -let interval; - -io.on('connection', (socket) => { - console.log('New client connected'); - - // let interval = null; - - // subscribe to timer - socket.on('subscribe-to-timer', () => { - console.log('New subscription'); - // avoid multiple intervals - if (interval) clearInterval(interval); - - // send current data - socket.emit('timer', timer.getObject()); - - // set callback for timer events - interval = setInterval(() => emitTimer(socket), config.timer.refresh); - }); - - // unsubscribe to timer - socket.on('release-timer', () => { - console.log('Releasing subscription'); - // avoid multiple intervals - if (interval) clearInterval(interval); - }); - - socket.on('get-timer', () => { - socket.emit('timer', timer.getObject()); - }); - - socket.on('get-playstate', () => { - socket.emit('playstate', timer.playState); - }); - - // playback API - socket.on('set-presenter-text', (data) => { - timer.presenterText = data; - socket.emit('messages-presenter', timer.presenter); - }); - - socket.on('set-presenter-visible', (data) => { - timer.presenterVisible = data; - socket.emit('messages-presenter', timer.presenter); - }); - - socket.on('get-presenter', () => { - socket.emit('messages-presenter', timer.presenter); - }); - - socket.on('set-public-text', (data) => { - timer.publicText = data; - socket.emit('messages-public', timer.public); - }); - - socket.on('set-public-visible', (data) => { - timer.publicVisible = data; - socket.emit('messages-public', timer.public); - }); - - socket.on('get-public', () => { - socket.emit('messages-public', timer.public); - }); - - // handle client disconnect - socket.on('disconnect', () => { - console.log('Client disconnected'); - if (interval) clearInterval(interval); - }); -}); - -// send timer events -const emitTimer = (socket) => { - // send current timer - socket.emit('timer', timer.getObject()); -}; +// start socket server +initiateSocket(server, config); // Start server server.listen(port, () => console.log(`Listening on port ${port}`)); diff --git a/server/controllers/eventsController.js b/server/controllers/eventsController.js index f95fe91b3..78052ea22 100644 --- a/server/controllers/eventsController.js +++ b/server/controllers/eventsController.js @@ -12,12 +12,6 @@ const replaceAt = (array, index, value) => { return ret; }; -// Create controller for GET request to '/events' -// Returns ACK message -exports.eventsGet = async (req, res) => { - res.send({ response: 'Events Controller API' }); -}; - // Create controller for GET request to '/events/all' // Returns - exports.eventsGetAll = async (req, res) => { @@ -41,7 +35,7 @@ exports.eventsPost = async (req, res) => { // ensure structure let newEvent = {}; - req.body.id = nanoid(10); + req.body.id = nanoid(6); switch (req.body.type) { case 'event': @@ -131,17 +125,14 @@ exports.eventsDelete = async (req, res) => { const itemIndex = events.findIndex((e) => e.id == req.params.id); - // Torbjorn: this syntax is very bad if (itemIndex === -1) { res.sendStatus(400); return; } + else if (itemIndex === 0) events.shift(); + else events.splice(itemIndex, 1); - if (itemIndex === 0) { - const e = events.shift(); - } else { - const e = events.splice(itemIndex, 1); - } + // Update events events = [...events]; res.sendStatus(200); }; diff --git a/server/controllers/socketController.js b/server/controllers/socketController.js new file mode 100644 index 000000000..d909066ab --- /dev/null +++ b/server/controllers/socketController.js @@ -0,0 +1,76 @@ +const socketIo = require('socket.io'); + +const initiateSocket = (server, config) => { + // initialise socketIO server + const io = socketIo(server, { + cors: { + origin: 'http://localhost:3000', + methods: ['GET', 'POST'], + }, + }); + + let interval = null; + + // set callback for timer events + interval = setInterval(() => emitTimer(io), config.timer.refresh); + + io.on('connection', (socket) => { + console.log('New client connected'); + + // send current data + socket.emit('timer', global.timer.getObject()); + + // unsubscribe to timer + socket.on('release-timer', () => { + console.log('Releasing subscription'); + // avoid multiple intervals + if (interval) clearInterval(interval); + }); + + socket.on('get-timer', () => { + socket.emit('timer', global.timer.getObject()); + }); + + socket.on('get-playstate', () => { + socket.emit('playstate', global.timer.playState); + }); + + // playback API + socket.on('set-presenter-text', (data) => { + global.timer.presenterText = data; + socket.emit('messages-presenter', global.timer.presenter); + }); + + socket.on('set-presenter-visible', (data) => { + global.timer.presenterVisible = data; + socket.emit('messages-presenter', global.timer.presenter); + }); + + socket.on('get-presenter', () => { + socket.emit('messages-presenter', global.timer.presenter); + }); + + socket.on('set-public-text', (data) => { + global.timer.publicText = data; + socket.emit('messages-public', global.timer.public); + }); + + socket.on('set-public-visible', (data) => { + global.timer.publicVisible = data; + socket.emit('messages-public', global.timer.public); + }); + + socket.on('get-public', () => { + socket.emit('messages-public', global.timer.public); + }); + + }); +}; + +// send timer events +const emitTimer = (socket) => { + // send current timer + socket.emit('timer', global.timer.getObject()); +}; + +module.exports = initiateSocket; diff --git a/server/routes/eventsRouter.js b/server/routes/eventsRouter.js index 11f16b030..1535e877b 100644 --- a/server/routes/eventsRouter.js +++ b/server/routes/eventsRouter.js @@ -5,10 +5,7 @@ const router = express.Router(); const eventsController = require('../controllers/eventsController'); // create route between controller and '/events' endpoint -router.get('/', eventsController.eventsGet); - -// create route between controller and '/events/all' endpoint -router.get('/all', eventsController.eventsGetAll); +router.get('/', eventsController.eventsGetAll); // create route between controller and '/events/:id' endpoint router.get('/:id', eventsController.eventsGetById); diff --git a/server/routes/playbackRouter.js b/server/routes/playbackRouter.js index a066fd770..757b8ced4 100644 --- a/server/routes/playbackRouter.js +++ b/server/routes/playbackRouter.js @@ -4,11 +4,8 @@ const router = express.Router(); // import event controller const playbackController = require('../controllers/playbackController'); -// create route between controller and '/playback' endpoint -router.get('/', playbackController.pbGet); - -// create route between controller and '/playback/all' endpoint -router.get('/all', playbackController.pbGetAll); +// create route between controller and '/playback/' endpoint +router.get('/', playbackController.pbGetAll); // create route between controller and '/playback/start' endpoint router.get('/start', playbackController.pbStart); @@ -23,7 +20,7 @@ router.get('/stop', playbackController.pbStop); router.get('/roll', playbackController.pbRoll); // create route between controller and '/playback/previous' endpoint -router.get('/previous',playbackController.pbPrevious); +router.get('/previous', playbackController.pbPrevious); // create route between controller and '/playback/next' endpoint router.get('/next', playbackController.pbNext);