refactor: Cleanup (#21)

* refactor(routes): remove unused sitemap route

* style(comments): enforce consistent comment style

* style(exports): enforce consistent export style

* chore: Add jsDoc for contentful service

* chore: Add jsDoc for contentful service \n \n closes #23

* refactor(app): move query parameter comment to right position and mention comment route middleware

* test(npm): add temporary test script

* refactor(middlewares): split up bootstrap middleware - fixes #23759

* refactor(cookies): use constances to give context the maxAge cookie setting

* refactor(variables): use more descriptive names for variables

* space became spaceId when it was just the id not the full space instance
* all (access) token variable name variants became [api-type]Token
* all clients are now called deliveryClient or previewClient
* cpa and cda only remain when they are used as actual API id
* env variables names adjusted

* perf(helpers): only run marked when content is not empty

* refactor(comments): fix typos

* refactor(comments): add hint why error is logged to console in settings

* chore: Add comments to routes and services

* refactor(requires): order and group requires

* fix(settings): add validation for wrong preview token

* chore: Add comments to routes

* chore: fix typo

* chore: console.error -> throw

* chore: move cookie name to a constant

* chore: Fix app.js comment

* typo: removing t

* chore: typos

* chore: removed unnecessary comment line sign

* chore: newline for readabillity

* chore: remove dangling `t`

* chore: remove `t`, add `l`

* chore: typos

* Fleshed out title

* build(npm): remove unused dependencies

* build(npm): upgrade to latest stable contentful sdk

* chore: Addressing David feedbak

* fix(credentials): update to match the new space

* chore: Addressing code review comments

* chore: Addressing code review comments

* chore: res -> response, req-> request

* chore: include exactly what we need

* chore: Address JPs comments

* chore: Address JPs comments

* chore: Address Fredericks comments

* chore: Address Fredericks comments

* fixup! chore: Address Fredericks comments

* fixup! fixup! chore: Address Fredericks comments

* fixup! fixup! fixup! chore: Address Fredericks comments

* fixup! fixup! fixup! fixup! chore: Address Fredericks comments

* fixup! fixup! fixup! fixup! fixup! chore: Address Fredericks comments

* fixup! fixup! fixup! fixup! fixup! fixup! chore: Address Fredericks comments
This commit is contained in:
Benedikt Rötsch
2017-11-03 10:58:42 +01:00
parent 730ce24cc9
commit 7ad2c1fa4f
25 changed files with 507 additions and 340 deletions

View File

@@ -1,5 +1,9 @@
/* GET category listing. */
exports.getCategories = async (req, res, next) => {
res.render('categories', { title: 'Categories' })
/*
* The purpose of this module is to render the category page when the route is requested
*/
// Renders categories page when `/categories` route is requested
module.exports.getCategories = async (request, response, next) => {
response.render('categories', { title: 'Categories' })
}

View File

@@ -1,80 +1,124 @@
const {getCourses, getCourse, getCategories, getCoursesByCategory} = require('./../services/contentful')
const attachEntryState = require('./../lib/entry-state')
/*
* The purpose of this module is to render the category page when the route is requested
*/
exports.getCourses = async (req, res, next) => {
// We get all the entries with the content type `course`
const {
getCourses,
getCourse,
getCategories,
getCoursesByCategory
} = require('./../services/contentful')
const attachEntryState = require('../lib/entry-state')
const { updateCookie } = require('../lib/cookies')
/**
* Renders courses list when `/courses` route is requested
*
* @param request - Object - Express request object
* @param response - Object - Express response object
* @param next - Function - express callback
*
* @returns {undefined}
*/
module.exports.getCourses = async (request, response, next) => {
// Get all the entries of content type course
let courses = []
let categories = []
courses = await getCourses(res.locals.currentLocale.code, res.locals.currentApi.id)
courses = await getCourses(response.locals.currentLocale.code, response.locals.currentApi.id)
// Attach entry state flags when using preview API
if (res.locals.settings.editorialFeatures && res.locals.currentApi.id === 'cpa') {
if (response.locals.settings.editorialFeatures && response.locals.currentApi.id === 'cpa') {
courses = await Promise.all(courses.map(attachEntryState))
}
categories = await getCategories(res.locals.currentLocale.code, res.locals.currentApi.id)
res.render('courses', { title: `All Courses (${courses.length})`, categories, courses })
categories = await getCategories(response.locals.currentLocale.code, response.locals.currentApi.id)
response.render('courses', { title: `All Courses (${courses.length})`, categories, courses })
}
exports.getCourse = async (req, res, next) => {
let course = await getCourse(req.params.slug, res.locals.currentLocale.code, res.locals.currentApi.id)
/**
* Renders a course when `/courses/:slug` route is requested
*
* @param request - Object - Express request object
* @param response - Object - Express response object
* @param next - Function - express callback
*
* @returns {undefined}
*/
module.exports.getCourse = async (request, response, next) => {
let course = await getCourse(request.params.slug, response.locals.currentLocale.code, response.locals.currentApi.id)
// Get lessons
const lessons = course.fields.lessons
const lessonIndex = lessons.findIndex((lesson) => lesson.fields.slug === req.params.lslug)
const lesson = lessons[lessonIndex]
let {lesson, lessonIndex} = getNextLesson(lessons, request.params.lslug)
// Save visited lessons
const cookie = req.cookies.visitedLessons
// Manage state of viewed lessons
const cookie = request.cookies.visitedLessons
let visitedLessons = cookie || []
visitedLessons.push(course.sys.id)
visitedLessons = [...new Set(visitedLessons)]
res.cookie('visitedLessons', visitedLessons, { maxAge: 900000, httpOnly: true })
updateCookie(response, 'visitedLessons', visitedLessons)
// Attach entry state flags when using preview API
if (res.locals.settings.editorialFeatures && res.locals.currentApi.id === 'cpa') {
if (response.locals.settings.editorialFeatures && response.locals.currentApi.id === 'cpa') {
course = await attachEntryState(course)
}
res.render('course', {title: course.fields.title, course, lesson, lessons, lessonIndex, visitedLessons})
response.render('course', {title: course.fields.title, course, lesson, lessons, lessonIndex, visitedLessons})
}
exports.getCoursesByCategory = async (req, res, next) => {
/**
* Renders a courses list by a category when `/courses/category/:category` route is requested
*
* @param request - Object - Express request object
* @param response - Object - Express response object
* @param next - Function - Express callback
*
* @returns {undefined}
*/
module.exports.getCoursesByCategory = async (request, response, next) => {
// We get all the entries with the content type `course` filtered by a category
let courses = []
let categories = []
let activeCategory = ''
try {
categories = await getCategories()
activeCategory = categories.find((category) => category.fields.slug === req.params.category)
courses = await getCoursesByCategory(activeCategory.sys.id, res.locals.currentLocale.code, res.locals.currentApi.id)
activeCategory = categories.find((category) => category.fields.slug === request.params.category)
courses = await getCoursesByCategory(activeCategory.sys.id, response.locals.currentLocale.code, response.locals.currentApi.id)
} catch (e) {
console.log('Error ', e)
}
res.render('courses', { title: `${activeCategory.fields.title} (${courses.length})`, categories, courses })
response.render('courses', { title: `${activeCategory.fields.title} (${courses.length})`, categories, courses })
}
/* GET course lesson detail. */
exports.getLesson = async (req, res, next) => {
let course = await getCourse(req.params.cslug, res.locals.currentLocale.code, res.locals.currentApi.id)
/**
* Renders a lesson details when `/courses/:courseSlug/lessons/:lessonSlug` route is requested
*
* @param request - Object - Express request object
* @param response - Object - Express response object
* @param next - Function - express callback
*
* @returns {undefined}
*/
module.exports.getLesson = async (request, response, next) => {
let course = await getCourse(request.params.cslug, response.locals.currentLocale.code, response.locals.currentApi.id)
const lessons = course.fields.lessons
const lessonIndex = lessons.findIndex((lesson) => lesson.fields.slug === req.params.lslug)
let lesson = lessons[lessonIndex]
const nextLesson = lessons[lessonIndex + 1] || null
let {lesson, nextLesson} = getNextLesson(lessons, request.params.lslug)
// Save visited lessons
const cookie = req.cookies.visitedLessons
const cookie = request.cookies.visitedLessons
let visitedLessons = cookie || []
visitedLessons.push(lesson.sys.id)
visitedLessons = [...new Set(visitedLessons)]
res.cookie('visitedLessons', visitedLessons, { maxAge: 900000, httpOnly: true })
updateCookie(response, 'visitedLessons', visitedLessons)
// Attach entry state flags when using preview API
if (res.locals.settings.editorialFeatures && res.locals.currentApi.id === 'cpa') {
if (response.locals.settings.editorialFeatures && response.locals.currentApi.id === 'cpa') {
lesson = await attachEntryState(lesson)
}
res.render('course', {
response.render('course', {
title: `${course.fields.title} | ${lesson.fields.title}`,
course,
lesson,
@@ -84,3 +128,14 @@ exports.getLesson = async (req, res, next) => {
})
}
function getNextLesson (lessons, lslug) {
const lessonIndex = lessons.findIndex((lesson) => lesson.fields.slug === lslug)
let lesson = lessons[lessonIndex]
const nextLesson = lessons[lessonIndex + 1] || null
return {
lessonIndex,
lesson,
nextLesson
}
}

View File

@@ -1,4 +1,11 @@
exports.getImprint = (req, res, next) => {
res.render('imprint', { title: 'Imprint' })
/**
* Renders imprint page when `/imprint` route is requested
* @param request - Object - Express request
* @param response - Object - Express response
* @param next - Function - Express callback
* @returns {undefined}
*/
module.exports.getImprint = (request, response, next) => {
response.render('imprint', { title: 'Imprint' })
}

View File

@@ -1,30 +1,32 @@
/**
* This module connects rendering modules to routes
*/
const express = require('express')
const { catchErrors } = require('../handlers/errorHandlers')
const { getCourses, getCourse, getLesson, getCoursesByCategory } = require('./courses')
const { getSettings, postSettings } = require('./settings')
const { getSitemap } = require('./sitemap')
const { getLandingPage } = require('./landingPage')
const { getImprint } = require('./imprint')
const router = express.Router()
/* GET the home landing page. */
const { catchErrors } = require('../handlers/errorHandlers')
const { getCourses, getCourse, getLesson, getCoursesByCategory } = require('./courses')
const { getSettings, postSettings } = require('./settings')
const { getLandingPage } = require('./landingPage')
const { getImprint } = require('./imprint')
// GET the home landing page
router.get('/', catchErrors(getLandingPage))
/* Courses Routes */
// Courses routes
router.get('/courses', catchErrors(getCourses))
router.get('/courses/categories/:category', catchErrors(getCoursesByCategory))
router.get('/courses/:slug', catchErrors(getCourse))
router.get('/courses/:slug/lessons', catchErrors(getCourse))
router.get('/courses/:cslug/lessons/:lslug', catchErrors(getLesson))
/* Settings Routes */
// Settings routes
router.get('/settings', catchErrors(getSettings))
router.post('/settings', catchErrors(postSettings))
/* Sitemap Route */
router.get('/sitemap', catchErrors(getSitemap))
/* Imprint Route */
// Imprint route
router.get('/imprint', catchErrors(getImprint))
module.exports = router

View File

@@ -1,21 +1,36 @@
const { getLandingPage } = require('../services/contentful')
const attachEntryState = require('./../lib/entry-state')
/**
* This module renders a layout when its route is requested
* It is used for pages like home page
*/
const url = require('url')
exports.getLandingPage = async (req, res, next) => {
let pathname = url.parse(req.url).pathname.split('/').filter(Boolean)[0]
const { getLandingPage } = require('../services/contentful')
const attachEntryState = require('./../lib/entry-state')
/**
* Renders a landing page when `/` route is requested
* based on the pathname an entry is queried from contentful
* and a view is rendered from the pulled data
*
* @param request - Object - Express request
* @param response - Object - Express response
* @param next - Function - Express callback
* @returns {undefined}
*/
module.exports.getLandingPage = async (request, response, next) => {
let pathname = url.parse(request.url).pathname.split('/').filter(Boolean)[0]
pathname = pathname || 'home'
let landingPage = await getLandingPage(
pathname,
res.locals.currentLocale.code,
res.locals.currentApi.id
response.locals.currentLocale.code,
response.locals.currentApi.id
)
// Attach entry state flags when using preview APIgs
if (res.locals.settings.editorialFeatures && res.locals.currentApi.id === 'cpa') {
// Attach entry state flags when using preview API
if (response.locals.settings.editorialFeatures && response.locals.currentApi.id === 'cpa') {
landingPage = await attachEntryState(landingPage)
}
res.render('landingPage', { title: pathname, landingPage })
response.render('landingPage', { title: pathname, landingPage })
}

View File

@@ -1,16 +1,25 @@
/**
* This module renders the settings page when `settings` route is requested
* it also saves the settings to a cookie
*/
const { createClient } = require('contentful')
const { initClient, getSpace } = require('./../services/contentful')
const { initClients, getSpace } = require('./../services/contentful')
const { updateCookie } = require('../lib/cookies')
async function renderSettings (res, opts) {
// Get connectred space to display the space name on top of the settings
const SETTINGS_NAME = 'theExampleAppSettings'
async function renderSettings (response, opts) {
// Get connected space to display the space name on top of the settings
let space = false
try {
space = await getSpace()
} catch (error) {
console.error(error)
// We throw the error here, it will be handled by the error middleware
// We keep space false to ensure the "Connected to" box is not shown.
throw (error)
}
res.render('settings', {
response.render('settings', {
title: 'Settings',
errors: {},
hasErrors: false,
@@ -20,68 +29,85 @@ async function renderSettings (res, opts) {
})
}
/* GET settings page. */
exports.getSettings = async (req, res, next) => {
const { settings } = res.locals
await renderSettings(res, {
/**
* Renders the settings page when `/settings` route is requested
*
* @param request - Object - Express request
* @param response - Object - Express response
* @param next - Function - Express callback
*
* @returns {undefined}
*/
module.exports.getSettings = async (request, response, next) => {
const { settings } = response.locals
await renderSettings(response, {
settings
})
}
/* POST settings page. */
exports.postSettings = async (req, res, next) => {
/**
* Save settings when POST request is triggered to the `/settings` route
* and render the settings page
*
* @param request - Object - Express request
* @param response - Object - Express response
* @param next - Function - Express callback
*
* @returns {undefined}
*/
module.exports.postSettings = async (request, response, next) => {
const errorList = []
const { space, cda, cpa, editorialFeatures } = req.body
const { spaceId, deliveryToken, previewToken, editorialFeatures } = request.body
const settings = {
space,
cda,
cpa,
spaceId,
deliveryToken,
previewToken,
editorialFeatures: !!editorialFeatures
}
// Validate required fields.
if (!space) {
if (!spaceId) {
errorList.push({
field: 'space',
field: 'spaceId',
message: 'This field is required'
})
}
if (!cda) {
if (!deliveryToken) {
errorList.push({
field: 'cda',
field: 'deliveryToken',
message: 'This field is required'
})
}
if (!cpa) {
if (!previewToken) {
errorList.push({
field: 'cpa',
field: 'previewToken',
message: 'This field is required'
})
}
// Validate space and CDA access token.
if (space && cda) {
// Validate space and delivery access token.
if (spaceId && deliveryToken) {
try {
await createClient({
space,
accessToken: cda
space: spaceId,
accessToken: deliveryToken
}).getSpace()
} catch (err) {
if (err.response.status === 401) {
errorList.push({
field: 'cda',
field: 'deliveryToken',
message: 'Your Delivery API key is invalid.'
})
} else if (err.response.status === 404) {
errorList.push({
field: 'space',
field: 'spaceId',
message: 'This space does not exist or your access token is not associated with your space.'
})
} else {
errorList.push({
field: 'cda',
field: 'deliveryToken',
message: `Something went wrong: ${err.response.data.message}`
})
}
@@ -89,24 +115,27 @@ exports.postSettings = async (req, res, next) => {
}
// Validate space and CPA access token.
if (space && cpa) {
if (spaceId && previewToken) {
try {
await createClient({
space,
accessToken: cpa,
space: spaceId,
accessToken: previewToken,
host: 'preview.contentful.com'
}).getSpace()
} catch (err) {
if (err.response.status === 401) {
errorList.push({
field: 'cpa',
field: 'previewToken',
message: 'Your Preview API key is invalid.'
})
} else if (err.response.status === 404) {
// Already validated via CDA
errorList.push({
field: 'spaceId',
message: 'This space does not exist or your delivery token is not associated with your space.'
})
} else {
errorList.push({
field: 'cpa',
field: 'previewToken',
message: `Something went wrong: ${err.response.data.message}`
})
}
@@ -116,11 +145,11 @@ exports.postSettings = async (req, res, next) => {
// When no errors occurred
if (!errorList.length) {
// Store new settings
res.cookie('theExampleAppSettings', settings, { maxAge: 31536000, httpOnly: true })
res.locals.settings = settings
updateCookie(response, SETTINGS_NAME, settings)
response.locals.settings = settings
// Reinit clients
initClient(settings)
initClients(settings)
}
// Generate error dictionary
@@ -135,7 +164,7 @@ exports.postSettings = async (req, res, next) => {
}
}, {})
await renderSettings(res, {
await renderSettings(response, {
settings,
errors,
hasErrors: errorList.length > 0,

View File

@@ -1,5 +0,0 @@
/* GET sitemap page. */
exports.getSitemap = async (req, res, next) => {
res.render('sitemap', { title: 'Sitemap' })
}