Merge pull request #123 from o2Labs/ttl-fixes

Fix TTL including milliseconds

Author: danielrbradley

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.

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",

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(
     <TestContext user={user}>
       <Home />
@@ -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' });
 });

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>): Config => ({
   advancedBookingDays: 14,
@@ -18,12 +19,19 @@ export const createFakeOffice = (prototype?: Partial<Office>): Office => ({
 });
 
 export const createFakeOfficeWithSlots = (
+  config: Config,
   prototype?: Partial<OfficeWithSlots>
-): 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>): User => ({
   email: '[email protected]',

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"

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'

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')),

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: {

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: {

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);
+};

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,
+  },
 };
 
 /**

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: '[email protected]',
+      weekCommencing: '2020-01-01',
+    };
+    const officeBookingKey = {
+      name: 'the-office',
+      date: '2020-01-01',
+    };
+    const bookingKey = {
+      id: 'booking-id',
+      user: '[email protected]',
+    };
+    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));
+  });
+});