diff --git a/CHANGELOG.md b/CHANGELOG.md index 00fb7f7..eded6a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.1.0] + +- Fix TTL bug resulting in the data retention policy not being followed. +- Add migration to fix TTL on existing database entries. +- Add a basic stats page showing all booking counts per office, per day. +- Improved usability and accessibility of login journey. +- Increase test coverage of critical path to booking. + ## [2.0.2] - Add check for user before attempting to retrieve office details. diff --git a/client/package.json b/client/package.json index 0a4395b..496006a 100644 --- a/client/package.json +++ b/client/package.json @@ -1,6 +1,6 @@ { "name": "office-booker", - "version": "0.1.0", + "version": "2.1.0", "private": true, "scripts": { "start": "react-scripts start", @@ -22,6 +22,7 @@ "@reach/router": "^1.3.3", "@types/loadable__component": "^5.13.1", "amazon-cognito-identity-js": "^4.2.2", + "array-fns": "^1.0.0", "core-js": "^3.6.5", "date-fns": "^2.12.0", "lodash": "^4.17.19", diff --git a/client/src/components/App/Home.test.tsx b/client/src/components/App/Home.test.tsx index a9c77d2..3851e86 100644 --- a/client/src/components/App/Home.test.tsx +++ b/client/src/components/App/Home.test.tsx @@ -1,18 +1,33 @@ import React from 'react'; -import { render, fireEvent, screen } from '@testing-library/react'; +import { render, fireEvent, screen, within } from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; import { server } from '../../../test/server.mock'; -import { createFakeOfficeWithSlots, createFakeSystemAdminUser } from '../../../test/data'; +import { + createFakeConfig, + createFakeOfficeWithSlots, + createFakeSystemAdminUser, +} from '../../../test/data'; import { TestContext } from '../../../test/TestContext'; -import { mockGetBookings, mockGetOffice, mockGetOffices } from '../../../test/handlers'; +import { + mockGetBookings, + mockGetConfig, + mockGetOffice, + mockGetOffices, +} from '../../../test/handlers'; import Home from './Home'; test('Selecting an office', async () => { - const office = createFakeOfficeWithSlots(); - const user = createFakeSystemAdminUser([office]); - server.use(mockGetOffices([office]), mockGetBookings([]), mockGetOffice(office)); + const config = createFakeConfig(); + const office = createFakeOfficeWithSlots(config); + const user = createFakeSystemAdminUser([office], { quota: 2 }); + server.use( + mockGetConfig(config), + mockGetOffices([office]), + mockGetBookings([]), + mockGetOffice(office) + ); render( @@ -21,11 +36,20 @@ test('Selecting an office', async () => { await screen.findByRole('heading', { level: 2, name: 'Select your office' }); - fireEvent.click(screen.getByRole('button', { name: office.name })); + const main = within(screen.getByRole('main')); + + fireEvent.click(main.getByRole('button', { name: office.name })); - await screen.findByRole('heading', { level: 2, name: office.name }); - expect(screen.getByText(/You can make/i)).toHaveTextContent(`You can make 5 booking per week`); - expect(screen.getByText(/daily capacity/i)).toHaveTextContent( - `The Office has a daily capacity of 100 and car park capacity of 100.` + await main.findByRole('heading', { level: 2, name: office.name }); + expect(main.getByText(/You can make/i)).toHaveTextContent( + `You can make ${user.quota} booking per week` + ); + expect(main.getByText(/daily capacity/i)).toHaveTextContent( + `The Office has a daily capacity of ${office.quota} and car park capacity of ${office.parkingQuota}.` ); + expect(main.getByText(/bookings remaining/i)).toHaveTextContent( + `${user.quota} bookings remaining` + ); + + await main.findAllByRole('button', { name: 'Book' }); }); diff --git a/client/test/data.ts b/client/test/data.ts index fa61ec2..92b3663 100644 --- a/client/test/data.ts +++ b/client/test/data.ts @@ -1,6 +1,7 @@ import { Config } from '../src/context/stores'; import { Booking, Office, OfficeWithSlots, User } from '../src/types/api'; -import { format } from 'date-fns'; +import { addDays, format } from 'date-fns'; +import { init } from 'array-fns'; export const createFakeConfig = (prototype?: Partial): Config => ({ advancedBookingDays: 14, @@ -18,12 +19,19 @@ export const createFakeOffice = (prototype?: Partial): Office => ({ }); export const createFakeOfficeWithSlots = ( + config: Config, prototype?: Partial -): OfficeWithSlots => ({ - ...createFakeOffice(prototype), - slots: [], - ...prototype, -}); +): OfficeWithSlots => { + return { + ...createFakeOffice(prototype), + slots: init(config.advancedBookingDays, (i) => ({ + date: format(addDays(new Date(), i), 'yyyy-MM-dd'), + booked: 0, + bookedParking: 0, + })), + ...prototype, + }; +}; export const createFakeUser = (prototype?: Partial): User => ({ email: 'mock.user@domain.test', diff --git a/client/yarn.lock b/client/yarn.lock index a5368aa..76ab5fe 100644 --- a/client/yarn.lock +++ b/client/yarn.lock @@ -3002,6 +3002,11 @@ array-flatten@^2.1.0: resolved "https://registry.yarnpkg.com/array-flatten/-/array-flatten-2.1.2.tgz#24ef80a28c1a893617e2149b0c6d0d788293b099" integrity sha512-hNfzcOV8W4NdualtqBFPyVO+54DSJuZGY9qT4pRroB6S9e3iiido2ISIC5h9R2sPJ8H3FHCIiEnsv1lPXO3KtQ== +array-fns@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/array-fns/-/array-fns-1.0.0.tgz#bce7b5cc9b95c28187328a320555fd175cb545de" + integrity sha512-mPqxfuDBey5FYj+f3wy4Jlf1u74eNBOhBuJ26od/UovTLTwOhusNoNgByQN3JI7yEr//CD1XuDtWVQYYfpJRLw== + array-includes@^3.1.1: version "3.1.1" resolved "https://registry.yarnpkg.com/array-includes/-/array-includes-3.1.1.tgz#cdd67e6852bdf9c1215460786732255ed2459348" diff --git a/server/app.ts b/server/app.ts index 2b1db85..472f6a7 100644 --- a/server/app.ts +++ b/server/app.ts @@ -47,7 +47,7 @@ export const configureApp = (config: Config) => { app.get('/api/config', (req, res, next) => { try { const clientConfig = { - version: '2.0.0', + version: '2.1.0', showTestBanner: config.showTestBanner, auth: config.authConfig.type === 'cognito' diff --git a/server/db/bookings.ts b/server/db/bookings.ts index 4b0940f..85a3353 100644 --- a/server/db/bookings.ts +++ b/server/db/bookings.ts @@ -2,7 +2,7 @@ import { Config } from '../app-config'; import DynamoDB from 'aws-sdk/clients/dynamodb'; import { attribute, table, hashKey, rangeKey } from '@aws/dynamodb-data-mapper-annotations'; import { DataMapper } from '@aws/dynamodb-data-mapper'; -import { format, subDays, addDays } from 'date-fns'; +import { format, subDays, addDays, getUnixTime } from 'date-fns'; import { AttributePath, FunctionExpression } from '@aws/dynamodb-expressions'; export interface CreateBookingModel { @@ -100,7 +100,7 @@ export const createBooking = async ( try { const created = await mapper.put( Object.assign(new BookingsModel(), booking, { - ttl: addDays(new Date(booking.date), config.dataRetentionDays).getTime(), + ttl: getUnixTime(addDays(new Date(booking.date), config.dataRetentionDays)), }), { condition: new FunctionExpression('attribute_not_exists', new AttributePath('id')), diff --git a/server/db/officeBookings.ts b/server/db/officeBookings.ts index ec148d1..7911991 100644 --- a/server/db/officeBookings.ts +++ b/server/db/officeBookings.ts @@ -12,7 +12,7 @@ import { UpdateExpression, FunctionExpression, } from '@aws/dynamodb-expressions'; -import { addDays } from 'date-fns'; +import { addDays, getUnixTime } from 'date-fns'; export interface OfficeBooking { officeId: string; @@ -57,7 +57,7 @@ export const incrementOfficeBookingCount = async ( date, bookingCount: 0, parkingCount: 0, - ttl: addDays(new Date(date), config.dataRetentionDays).getTime(), + ttl: getUnixTime(addDays(new Date(date), config.dataRetentionDays)), }), { condition: { diff --git a/server/db/userBookings.ts b/server/db/userBookings.ts index 85f3d75..3605b90 100644 --- a/server/db/userBookings.ts +++ b/server/db/userBookings.ts @@ -10,7 +10,7 @@ import { FunctionExpression, } from '@aws/dynamodb-expressions'; import { Arrays } from 'collection-fns'; -import { addDays } from 'date-fns'; +import { addDays, getUnixTime } from 'date-fns'; export interface UserBookings { email: string; @@ -69,7 +69,7 @@ export const incrementUserBookingCount = async ( email: userEmail, weekCommencing, bookingCount: 0, - ttl: addDays(new Date(weekCommencing), config.dataRetentionDays + 7).getTime(), + ttl: getUnixTime(addDays(new Date(weekCommencing), config.dataRetentionDays + 7)), }), { condition: { diff --git a/server/migrations/4-fix-ttl.ts b/server/migrations/4-fix-ttl.ts new file mode 100644 index 0000000..95b1933 --- /dev/null +++ b/server/migrations/4-fix-ttl.ts @@ -0,0 +1,41 @@ +import { DataMapper } from '@aws/dynamodb-data-mapper'; +import { DynamoDB } from 'aws-sdk'; +import { getUnixTime } from 'date-fns'; +import { Config } from '../app-config'; +import { BookingsModel } from '../db/bookings'; +import { OfficeBookingModel } from '../db/officeBookings'; +import { UserBookingsModel } from '../db/userBookings'; + +// Check if TTL is beyond the year 3000 +const timestampTooHigh = (ttl: number) => ttl > 32503680000; + +async function* getCorrectedTtls(mapper: DataMapper, model: { new (): any }) { + for await (const booking of mapper.scan(model)) { + if (timestampTooHigh(booking.ttl)) { + const ttl = new Date(booking.ttl); + booking.ttl = getUnixTime(ttl); + yield booking; + } + } +} + +const scanAndFix = async (mapper: DataMapper, model: { new (): any }) => { + let updated = 0; + for await (const _item of mapper.batchPut(getCorrectedTtls(mapper, model))) { + updated++; + if (updated % 100 === 0) { + console.log('Updated ', updated); + } + } + console.log('Updated ', updated); +}; + +export const fixTtl = async (config: Config) => { + const mapper = new DataMapper({ + client: new DynamoDB(config.dynamoDB), + tableNamePrefix: config.dynamoDBTablePrefix, + }); + await scanAndFix(mapper, BookingsModel); + await scanAndFix(mapper, OfficeBookingModel); + await scanAndFix(mapper, UserBookingsModel); +}; diff --git a/server/migrations/index.ts b/server/migrations/index.ts index 3b638a6..8efc157 100644 --- a/server/migrations/index.ts +++ b/server/migrations/index.ts @@ -1,6 +1,7 @@ import { options } from 'yargs'; import { SSM } from 'aws-sdk'; import { parseConfigFromEnv, Config } from '../app-config'; +import { fixTtl } from './4-fix-ttl'; /** Collection all migrations that should be applied to the system */ type Migrations = { @@ -30,6 +31,9 @@ const migrations: Migrations = { '3-move-bookings': { reasonToFailPreCheck: 'You must deploy version 1.2.0 first.', }, + '4-fix-ttl': { + execute: fixTtl, + }, }; /** diff --git a/server/tests/migrations.test.ts b/server/tests/migrations.test.ts index 9962833..a4aa061 100644 --- a/server/tests/migrations.test.ts +++ b/server/tests/migrations.test.ts @@ -1,7 +1,70 @@ -// const { app, resetDb, config } = configureServer('migrations', { -// users: [{ Username: cognitoUsername, UserCreateDate: cognitoUserCreated }], -// }); +import { configureServer } from './test-utils'; +import { DataMapper } from '@aws/dynamodb-data-mapper'; +import { DynamoDB } from 'aws-sdk'; +import { UserBookingsModel } from '../db/userBookings'; +import { fixTtl } from '../migrations/4-fix-ttl'; +import { BookingsModel } from '../db/bookings'; +import { OfficeBookingModel } from '../db/officeBookings'; -// beforeEach(resetDb); +const { resetDb, config } = configureServer('migrations'); -test('no-active-migration', () => {}); +beforeEach(resetDb); + +describe('4 - fix TTL', async () => { + it(`fixes bookings ttl`, async () => { + const expires = new Date('2020-03-01'); + const mapper = new DataMapper({ + client: new DynamoDB(config.dynamoDB), + tableNamePrefix: config.dynamoDBTablePrefix, + }); + + const userBookingKey = { + email: 'user.name@domain.test', + weekCommencing: '2020-01-01', + }; + const officeBookingKey = { + name: 'the-office', + date: '2020-01-01', + }; + const bookingKey = { + id: 'booking-id', + user: 'user.name@domain.test', + }; + await mapper.put( + Object.assign(new UserBookingsModel(), userBookingKey, { + bookingCount: 1, + ttl: expires.getTime(), + }) + ); + await mapper.put( + Object.assign(new OfficeBookingModel(), officeBookingKey, { + bookingCount: 1, + parkingCount: 0, + ttl: expires.getTime(), + }) + ); + await mapper.put( + Object.assign(new BookingsModel(), bookingKey, { + date: '2020-01-01', + officeId: 'the-office', + parking: false, + ttl: expires.getTime(), + }) + ); + + await fixTtl(config); + + const updatedUserBooking = await mapper.get( + Object.assign(new UserBookingsModel(), userBookingKey) + ); + expect(updatedUserBooking.ttl).toBe(Math.floor(expires.getTime() / 1000)); + + const updatedOfficeBooking = await mapper.get( + Object.assign(new OfficeBookingModel(), officeBookingKey) + ); + expect(updatedOfficeBooking.ttl).toBe(Math.floor(expires.getTime() / 1000)); + + const updatedBooking = await mapper.get(Object.assign(new BookingsModel(), bookingKey)); + expect(updatedBooking.ttl).toBe(Math.floor(expires.getTime() / 1000)); + }); +});