Merge pull request #21 from Lan2u/training_register_form

Register training sheet for equipment

.gitignore

@@ -1,4 +1,5 @@
 /node_modules/
 /build/
 /.cache/
-.env
+*.ignore
+.env

Makefile

@@ -4,7 +4,6 @@ check: test lint typecheck unused-exports
 
 node_modules: package.json bun.lockb
 	bun install --frozen-lockfile
-	touch node_modules
 
 .env:
 	cp .env.example .env

README.md

@@ -73,4 +73,12 @@ curl -X POST -H 'Authorization: Bearer secret' -H 'Content-Type: application/jso
 ```
 curl -X POST -H 'Authorization: Bearer secret' -H 'Content-Type: application/json' \
 --data '{"name": "Woodspace"}' http://localhost:8080/api/create-area
-```
+
+# Testing
+When writing tests conceptionally the code can be split into 2 sections. The code responsible for writing events
+(commands) and the code for reading events (read models). When testing generally it should be split so that
+any testing that involves reading is done by providing the events to a read-model and any testing that involves
+writing should be done by asserting the resulting events. In theory this would mean that we never call a
+command.process from within the read-models part of the tests and we never call a read-model from the commands
+part of the tests. In reality we do still call command.process within the read-model tests just so that we can use
+it to generate the required events we need without having to do that manually.

src/commands/equipment/index.ts

@@ -1,9 +1,13 @@
 import {add} from './add';
 import {addForm} from './add-form';
+import {registerTrainingSheet} from './register-training-sheet';
 
 export const equipment = {
   add: {
     ...add,
     ...addForm,
   },
+  training_sheet: {
+    ...registerTrainingSheet,
+  },
 };

src/commands/equipment/register-training-sheet.ts

@@ -0,0 +1,42 @@
+import {DomainEvent, constructEvent} from '../../types';
+import * as RA from 'fp-ts/ReadonlyArray';
+import * as t from 'io-ts';
+import * as tt from 'io-ts-types';
+import * as O from 'fp-ts/Option';
+import {pipe} from 'fp-ts/lib/function';
+import {Command} from '../command';
+import {isAdminOrSuperUser} from '../is-admin-or-super-user';
+
+const codec = t.strict({
+  equipmentId: tt.UUID,
+  trainingSheetId: t.string,
+});
+
+type RegisterTrainingSheet = t.TypeOf<typeof codec>;
+
+const process = (input: {
+  command: RegisterTrainingSheet;
+  events: ReadonlyArray<DomainEvent>;
+}): O.Option<DomainEvent> =>
+  pipe(
+    input.events,
+    RA.match(
+      () =>
+        O.some(
+          constructEvent('EquipmentTrainingSheetRegistered')(input.command)
+        ),
+      () => O.none
+    )
+  );
+
+const resource = (command: RegisterTrainingSheet) => ({
+  type: 'Equipment',
+  id: command.equipmentId,
+});
+
+export const registerTrainingSheet: Command<RegisterTrainingSheet> = {
+  process,
+  resource,
+  decode: codec.decode,
+  isAuthorized: isAdminOrSuperUser,
+};

src/http/api-post.ts

@@ -44,6 +44,7 @@ const getActorFrom = (authorization: unknown, conf: Config) =>
     TE.fromEither
   );
 
+// See formPost for a more indepth discussion about the design decisions around why this is how it is.
 export const apiPost =
   <T>(deps: Dependencies, conf: Config, command: Command<T>) =>
   async (req: Request, res: Response) => {

src/http/form-get.ts

@@ -18,6 +18,10 @@ const getUser = (req: Request, deps: Dependencies) =>
     )
   );
 
+// See formPost for a more indepth discussion about the design decisions around why this is how it is.
+// formGet is like formPost but rather than processing a command formGet handles calling a read model to
+// get a view of the current state of a resource. This should be completely pure because its read-only and
+// is where conflict resolution etc. is handled as described in form-post.
 export const formGet =
   <T>(deps: Dependencies, form: Form<T>) =>
   async (req: Request, res: Response) => {

src/http/form-post.ts

@@ -16,6 +16,10 @@ import {persistOrNoOp} from '../commands/persist-or-no-op';
 const getCommandFrom = <T>(body: unknown, command: Command<T>) =>
   pipe(
     body,
+    // Stateless command validation rules go in decode.
+    // Giving user feedback here is easy but must be stateless.
+    // Direct quick feedback loop between the user -> code for validation.
+    // No stateful buisness rules should be enforced here. See the notes on formPost for more details.
     command.decode,
     E.mapLeft(formatValidationErrors),
     E.mapLeft(
@@ -37,11 +41,21 @@ const getActorFrom = (session: unknown, deps: Dependencies) =>
 export const formPost =
   <T>(deps: Dependencies, command: Command<T>, successTarget: string) =>
   async (req: Request, res: Response) => {
+    // Look at comments to see the core ideas of this pipe / how this works.
     await pipe(
       {
         actor: getActorFrom(req.session, deps),
+        // First we use getCommandFrom to statelessly check the input is sensical and in a sense this can be thought of
+        // as a fast path to minimise processing for garbage.
         formPayload: getCommandFrom(req.body, command),
-        events: deps.getAllEvents(),
+        // Second we get all the events. This is because we need all the events to get the authorisation of the command
+        // since there are addOwner events we need to read.
+        // There are 2 optimisations here we could take if required (but we haven't yet because they aren't needed):
+        // 1. Only get the events if the first step (stateslessly validating the command input) actually succeeds. This
+        //    would minimise the DDOS potential of a malicious or malformed component spamming the service with crap.
+        // 2. We actually only need some events and if we only got those events there would be significantly less data
+        //    to process.
+        events: deps.getAllEvents(), 
       },
       sequenceS(TE.ApplySeq),
       TE.filterOrElse(command.isAuthorized, () =>
@@ -63,6 +77,51 @@ export const formPost =
           sequenceS(TE.ApplyPar)
         )
       ),
+      // Command.process has 2 jobs:
+      //  1. To actually create the event
+      //  2. To check if the event should be created.
+      //
+      // Throughout this text 'user' refers to the consumer of this service. It doesn't necessarily mean a human.
+      //
+      // The idea of this code base is to use the CQRS pattern and following this we want command.process to do as little checking of
+      // business rules / other context as possible. Essentially all checking of buisness rules such as 'does this piece of equipment
+      // I'm trying to edit exist' should be done on the read side (the read models). This means that practically command.process
+      // should never fail and should always emit an event which is then processed when reading in a way of our choosing (such as
+      // by displaying there was a problem).
+      //
+      // Idempotency:
+      // Handling duplicate events so the system has idempotency should be primarily done on the readmodel side however it can also
+      // optionally be done on the write side in a cheap 'best effort' approach. This is an example of when command.process might not
+      // actually return an event but this can be thought of as it still being 'successful' because no-event is actually correct.
+      //
+      // Command.process failures 
+      // There are certain cases however where this level of always-succeed 'purity' isn't practical or could cause issues for example
+      // during user setup we don't want 2 users to be created with same user id and this is important enough that we want to strictly
+      // enforce this on the write side aswell. In this case command.process can actually 'fail' but we stil do this by emitting an
+      // event however instead of a regular event we emit one that has been created specifically for the failure case. This can then
+      // be handled by the read models etc. to display it to the user. This prevents us needing to add extra failure handling on the
+      // write side because its all deferrred to the read model.
+      //
+      // Write side locking:
+      // This is where persistOrNoOp comes in. persistOrNoOp uses non-blocking concurrency to ensure that events for a given resource
+      // are only written by 1 command at a time. In the normal case it is expected that this isn't required because the read model
+      // will handle any conflicts however in cases such as the duplicate user id problem above we want a stricter guarantee. In
+      // combination with command.process failures the write side locking can be used to effectively enforce exactly what is written
+      // to the events with a race condition handled by making the command a no-op.
+      // In future we can expand the handling of a detected write conflict by re-attempting the command again with the new state until
+      // the conflict is resolved. This would open up the ability for the command to raise a proper Failure 'event' as described above
+      // if the conflict can't be automatically resolved.
+      //
+      // Optimisations / notes:
+      // - While we actually only require write-side locking in rare cases for now we use it for all commands. The reason is that the
+      //    performance overhead is expected to be sufficiently low that we might aswell just use it for everything to minimse the
+      //    need to somehow specific which commands require it.
+      // - There is currently no mechanism to immediately use a Failure type event and use this to affect the response sent back to
+      //    the user. For the time being this intentional as the point of this architecture is to avoid complicated handling on the
+      //    write side. If something does happen the user will get a generic error back and then the read-model side is responsible
+      //    for displaying the new state based on the failure events that have been stored. It is forseeable that we may decide to
+      //    relax this slightly in the future to allow certain failure events to trigger an immediate specific response back to the
+      //    user.
       TE.chainW(input =>
         persistOrNoOp(
           deps.commitEvent,

src/index.ts

@@ -49,6 +49,15 @@ startMagicLinkEmailPubSub(deps, conf);
 const server = http.createServer(app);
 createTerminus(server);
 
+// Background processing can be kicked off here.
+// Background processes should write events with their results.
+// Background processes can call commands as needed.
+// Readmodels are used to get the current status of the background tasks via the
+// events that have been written.
+// There is no 'direct' communication between front-end and background tasks except
+// via the events. This makes things much easier to test and allows changes to happen
+// to the front/backend without having to update both.
+
 void (async () => {
   await pipe(
     ensureEventTableExists(dbClient),

src/read-models/equipment/get-all.ts

@@ -4,11 +4,12 @@ import {DomainEvent, isEventOfType} from '../../types';
 import * as RA from 'fp-ts/ReadonlyArray';
 import {readModels} from '..';
 
-type Equipment = {
+export type Equipment = {
   name: string;
   id: string;
   areaId: string;
   areaName: string;
+  trainingSheetId: O.Option<string>;
 };
 
 export const getAll = (
@@ -23,6 +24,12 @@ export const getAll = (
         O.map(area => ({
           ...equipment,
           areaName: area.name,
+        })),
+        O.map(equipmentData => ({
+          ...equipmentData,
+          trainingSheetId: readModels.equipment.getTrainingSheetId(events)(
+            equipmentData.id
+          ),
         }))
       )
     ),

src/read-models/equipment/get-training-sheet-id.ts

@@ -0,0 +1,18 @@
+import {pipe} from 'fp-ts/lib/function';
+import * as RA from 'fp-ts/ReadonlyArray';
+import * as O from 'fp-ts/Option';
+import {DomainEvent, isEventOfType} from '../../types';
+
+export const getTrainingSheetId =
+  (events: ReadonlyArray<DomainEvent>) =>
+  (equipmentId: string): O.Option<string> =>
+    pipe(
+      events,
+      RA.filter(isEventOfType('EquipmentTrainingSheetRegistered')),
+      RA.filter(event => event.equipmentId === equipmentId),
+      RA.last,
+      O.match(
+        () => O.none,
+        mostRecentSheet => O.some(mostRecentSheet.trainingSheetId)
+      )
+    );

src/read-models/equipment/index.ts

@@ -1,9 +1,11 @@
 import {get} from './get';
 import {getAll} from './get-all';
 import {getForArea} from './get-for-area';
+import {getTrainingSheetId} from './get-training-sheet-id';
 
 export const equipment = {
   get,
   getAll,
   getForArea,
+  getTrainingSheetId,
 };

src/router.ts

@@ -0,0 +1,91 @@
+import express, {Router} from 'express';
+import path from 'path';
+import {oopsPage} from './templates';
+import {Dependencies} from './dependencies';
+import {configureAuthRoutes} from './authentication';
+import {Config} from './configuration';
+import {StatusCodes} from 'http-status-codes';
+import {commands} from './commands';
+import {queries} from './queries';
+import {http} from './http';
+
+export const createRouter = (deps: Dependencies, conf: Config): Router => {
+  const router = Router();
+
+  router.get('/', queries.landing(deps));
+
+  router.get('/areas', queries.areas(deps));
+  router.get('/areas/create', http.formGet(deps, commands.area.create));
+  router.post(
+    '/areas/create',
+    http.formPost(deps, commands.area.create, '/areas')
+  );
+  router.get('/areas/add-owner', http.formGet(deps, commands.area.addOwner));
+  router.post(
+    '/areas/add-owner',
+    http.formPost(deps, commands.area.addOwner, '/areas')
+  );
+  router.get('/areas/:area', queries.area(deps));
+  router.post(
+    '/api/create-area',
+    http.apiPost(deps, conf, commands.area.create)
+  );
+
+  router.get(
+    '/areas/:area/add-equipment',
+    http.formGet(deps, commands.equipment.add)
+  );
+  router.get('/equipment', queries.allEquipment(deps));
+  router.post(
+    '/equipment/add',
+    http.formPost(deps, commands.equipment.add, '/equipment')
+  );
+  router.post(
+    '/equipment/add-training-sheet',
+    http.apiPost(deps, conf, commands.equipment.training_sheet)
+  );
+
+  router.get('/super-users', queries.superUsers(deps));
+  router.get(
+    '/super-users/declare',
+    http.formGet(deps, commands.superUser.declare)
+  );
+  router.post(
+    '/super-users/declare',
+    http.formPost(deps, commands.superUser.declare, '/super-users')
+  );
+  router.post(
+    '/api/declare-super-user',
+    http.apiPost(deps, conf, commands.superUser.declare)
+  );
+  router.get(
+    '/super-users/revoke',
+    http.formGet(deps, commands.superUser.revoke)
+  );
+  router.post(
+    '/super-users/revoke',
+    http.formPost(deps, commands.superUser.revoke, '/super-users')
+  );
+  router.post(
+    '/api/revoke-super-user',
+    http.apiPost(deps, conf, commands.superUser.revoke)
+  );
+
+  router.post(
+    '/api/link-number-to-email',
+    http.apiPost(deps, conf, commands.memberNumbers.linkNumberToEmail)
+  );
+
+  configureAuthRoutes(router);
+
+  router.get('/ping', (req, res) => res.status(StatusCodes.OK).send('pong\n'));
+
+  router.use('/static', express.static(path.resolve(__dirname, './static')));
+
+  router.use((req, res) => {
+    res
+      .status(StatusCodes.NOT_FOUND)
+      .send(oopsPage('The page you have requested does not exist.'));
+  });
+  return router;
+};

src/types/domain-event.ts

@@ -48,6 +48,11 @@ export const DomainEvent = t.union([
     memberNumber: t.number,
     email: EmailAddressCodec,
   }),
+  t.strict({
+    type: t.literal('EquipmentTrainingSheetRegistered'),
+    equipmentId: tt.UUID,
+    trainingSheetId: t.string,
+  }),
 ]);
 
 export type DomainEvent = t.TypeOf<typeof DomainEvent>;
@@ -82,6 +87,12 @@ type EventSpecificFields<T extends EventName> = Omit<
   'type' | 'actor' | 'recordedAt'
 >;
 
+// You must use this for constructing events because it means that if ever completely
+// remove an event its easy to find where it needs to be deleted from within the code.
+// 
+// We might remove an event if its not longer being produced and doesn't appear in the database
+// anymore but generally we wouldn't delete an event immediately after we stop producing it
+// so that read models can still use it for historical context.
 export const constructEvent =
   <T extends EventName, A extends EventSpecificFields<T> & {actor?: Actor}>(
     type: T