Skip to content

Commit dfa284b

Browse files
authored
Merge pull request #2344 from lindapaiste/refactor/async-exists
Convert server "object exists" functions to async/await.
2 parents b6cb6e7 + 312d333 commit dfa284b

File tree

5 files changed

+99
-91
lines changed

5 files changed

+99
-91
lines changed
Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,17 @@
11
import Collection from '../../models/collection';
22
import User from '../../models/user';
33

4-
export default function collectionForUserExists(
5-
username,
6-
collectionId,
7-
callback
8-
) {
9-
function sendFailure() {
10-
callback(false);
11-
}
12-
13-
function sendSuccess(collection) {
14-
callback(collection != null);
15-
}
16-
17-
function findUser() {
18-
return User.findByUsername(username);
19-
}
20-
21-
function findCollection(owner) {
22-
if (owner == null) {
23-
throw new Error('User not found');
24-
}
25-
26-
return Collection.findOne({ _id: collectionId, owner });
27-
}
28-
29-
return findUser().then(findCollection).then(sendSuccess).catch(sendFailure);
4+
/**
5+
* @param {string} username
6+
* @param {string} collectionId
7+
* @return {Promise<boolean>}
8+
*/
9+
export default async function collectionForUserExists(username, collectionId) {
10+
const user = await User.findByUsername(username);
11+
if (!user) return false;
12+
const collection = await Collection.findOne({
13+
_id: collectionId,
14+
owner: user
15+
}).exec();
16+
return collection != null;
3017
}

server/controllers/project.controller.js

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -169,29 +169,28 @@ export function getProjects(req, res) {
169169
}
170170
}
171171

172-
export function projectExists(projectId, callback) {
173-
Project.findById(projectId, (err, project) =>
174-
project ? callback(true) : callback(false)
175-
);
172+
/**
173+
* @param {string} projectId
174+
* @return {Promise<boolean>}
175+
*/
176+
export async function projectExists(projectId) {
177+
const project = await Project.findById(projectId);
178+
return project != null;
176179
}
177180

178-
export function projectForUserExists(username, projectId, callback) {
179-
User.findByUsername(username, (err, user) => {
180-
if (!user) {
181-
callback(false);
182-
return;
183-
}
184-
Project.findOne(
185-
{ user: user._id, $or: [{ _id: projectId }, { slug: projectId }] },
186-
(innerErr, project) => {
187-
if (!project) {
188-
callback(false);
189-
return;
190-
}
191-
callback(true);
192-
}
193-
);
181+
/**
182+
* @param {string} username
183+
* @param {string} projectId
184+
* @return {Promise<boolean>}
185+
*/
186+
export async function projectForUserExists(username, projectId) {
187+
const user = await User.findByUsername(username);
188+
if (!user) return false;
189+
const project = await Project.findOne({
190+
user: user._id,
191+
$or: [{ _id: projectId }, { slug: projectId }]
194192
});
193+
return project != null;
195194
}
196195

197196
function bundleExternalLibs(project) {

server/controllers/user.controller.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,10 +308,13 @@ export function updatePassword(req, res) {
308308
// eventually send email that the password has been reset
309309
}
310310

311-
export function userExists(username, callback) {
312-
User.findByUsername(username, (err, user) =>
313-
user ? callback(true) : callback(false)
314-
);
311+
/**
312+
* @param {string} username
313+
* @return {Promise<boolean>}
314+
*/
315+
export async function userExists(username) {
316+
const user = await User.findByUsername(username);
317+
return user != null;
315318
}
316319

317320
export function saveUser(res, user) {

server/routes/server.routes.js

Lines changed: 43 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { Router } from 'express';
2-
import { renderIndex } from '../views/index';
3-
import { get404Sketch } from '../views/404Page';
2+
import sendHtml, { renderIndex } from '../views/index';
43
import { userExists } from '../controllers/user.controller';
54
import {
65
projectExists,
@@ -27,40 +26,46 @@ router.get('/signup', (req, res) => {
2726
return res.send(renderIndex());
2827
});
2928

30-
router.get('/projects/:project_id', (req, res) => {
31-
projectExists(req.params.project_id, (exists) =>
32-
exists ? res.send(renderIndex()) : get404Sketch((html) => res.send(html))
33-
);
29+
router.get('/projects/:project_id', async (req, res) => {
30+
const exists = await projectExists(req.params.project_id);
31+
sendHtml(req, res, exists);
3432
});
3533

36-
router.get('/:username/sketches/:project_id/add-to-collection', (req, res) => {
37-
projectForUserExists(req.params.username, req.params.project_id, (exists) =>
38-
exists ? res.send(renderIndex()) : get404Sketch((html) => res.send(html))
39-
);
40-
});
34+
router.get(
35+
'/:username/sketches/:project_id/add-to-collection',
36+
async (req, res) => {
37+
const exists = await projectForUserExists(
38+
req.params.username,
39+
req.params.project_id
40+
);
41+
sendHtml(req, res, exists);
42+
}
43+
);
4144

42-
router.get('/:username/sketches/:project_id', (req, res) => {
43-
projectForUserExists(req.params.username, req.params.project_id, (exists) =>
44-
exists ? res.send(renderIndex()) : get404Sketch((html) => res.send(html))
45+
router.get('/:username/sketches/:project_id', async (req, res) => {
46+
const exists = await projectForUserExists(
47+
req.params.username,
48+
req.params.project_id
4549
);
50+
sendHtml(req, res, exists);
4651
});
4752

48-
router.get('/:username/sketches', (req, res) => {
49-
userExists(req.params.username, (exists) =>
50-
exists ? res.send(renderIndex()) : get404Sketch((html) => res.send(html))
51-
);
53+
router.get('/:username/sketches', async (req, res) => {
54+
const exists = await userExists(req.params.username);
55+
sendHtml(req, res, exists);
5256
});
5357

54-
router.get('/:username/full/:project_id', (req, res) => {
55-
projectForUserExists(req.params.username, req.params.project_id, (exists) =>
56-
exists ? res.send(renderIndex()) : get404Sketch((html) => res.send(html))
58+
router.get('/:username/full/:project_id', async (req, res) => {
59+
const exists = await projectForUserExists(
60+
req.params.username,
61+
req.params.project_id
5762
);
63+
sendHtml(req, res, exists);
5864
});
5965

60-
router.get('/full/:project_id', (req, res) => {
61-
projectExists(req.params.project_id, (exists) =>
62-
exists ? res.send(renderIndex()) : get404Sketch((html) => res.send(html))
63-
);
66+
router.get('/full/:project_id', async (req, res) => {
67+
const exists = await projectExists(req.params.project_id);
68+
sendHtml(req, res, exists);
6469
});
6570

6671
router.get('/login', (req, res) => {
@@ -98,15 +103,11 @@ router.get('/assets', (req, res) => {
98103
}
99104
});
100105

101-
router.get('/:username/assets', (req, res) => {
102-
userExists(req.params.username, (exists) => {
103-
const isLoggedInUser =
104-
req.user && req.user.username === req.params.username;
105-
const canAccess = exists && isLoggedInUser;
106-
return canAccess
107-
? res.send(renderIndex())
108-
: get404Sketch((html) => res.send(html));
109-
});
106+
router.get('/:username/assets', async (req, res) => {
107+
const exists = await userExists(req.params.username);
108+
const isLoggedInUser = req.user && req.user.username === req.params.username;
109+
const canAccess = exists && isLoggedInUser;
110+
sendHtml(req, res, canAccess);
110111
});
111112

112113
router.get('/account', (req, res) => {
@@ -121,16 +122,17 @@ router.get('/about', (req, res) => {
121122
res.send(renderIndex());
122123
});
123124

124-
router.get('/:username/collections/:id', (req, res) => {
125-
collectionForUserExists(req.params.username, req.params.id, (exists) =>
126-
exists ? res.send(renderIndex()) : get404Sketch((html) => res.send(html))
125+
router.get('/:username/collections/:id', async (req, res) => {
126+
const exists = await collectionForUserExists(
127+
req.params.username,
128+
req.params.id
127129
);
130+
sendHtml(req, res, exists);
128131
});
129132

130-
router.get('/:username/collections', (req, res) => {
131-
userExists(req.params.username, (exists) =>
132-
exists ? res.send(renderIndex()) : get404Sketch((html) => res.send(html))
133-
);
133+
router.get('/:username/collections', async (req, res) => {
134+
const exists = await userExists(req.params.username);
135+
sendHtml(req, res, exists);
134136
});
135137

136138
router.get('/privacy-policy', (req, res) => {

server/views/index.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import get404Sketch from './404Page';
2+
13
export function renderIndex() {
24
const assetsManifest = process.env.webpackAssets && JSON.parse(process.env.webpackAssets);
35
return `
@@ -45,3 +47,18 @@ export function renderIndex() {
4547
</html>
4648
`;
4749
}
50+
51+
/**
52+
* Send a 404 page if `exists` is false.
53+
* @param {import('express').e.Request} req
54+
* @param {import('express').e.Response} res
55+
* @param {boolean} [exists]
56+
*/
57+
export default function sendHtml(req, res, exists = true) {
58+
if (!exists) {
59+
res.status(404);
60+
get404Sketch((html) => res.send(html));
61+
} else {
62+
res.send(renderIndex());
63+
}
64+
};

0 commit comments

Comments
 (0)