growthbook icon indicating copy to clipboard operation
growthbook copied to clipboard

[WIP] Data Model Classes

Open jdorn opened this issue 2 years ago • 3 comments

Overview

New data model classes to enforce best practices and reduce boilerplate code required to create new models. For an initial test, this PR converts the FactModel to use the new class.

Example

Create the data model class based on an Interface:

export class FooDataModel extends BaseModel<FooInterface> {
  protected config: ModelConfig<FooInterface> = {
    collectionName: "foos",
    idPrefix: "foo__",
    writePermission: "createFoo",
    projectScoping: "multiple"
  };
}

Add it to the Request Context:

this.foo = new FooDataModel(this);

And then, you can access it anywhere you have context and use the built-in methods:

const foo = await context.foo.getById("foo_123");

await context.foo.update(foo, {name: "New Name"});

Features

Built-in Public Methods

The following public methods are created automatically:

  • getAll()
  • getAllByProject(project)
  • getById(id)
  • create(data)
  • update(existing, changes)
  • updateById(id, changes)
  • delete(existing)

You can add more tailored data fetching methods as needed by using the protected _findOne and _find methods. There are similar protected methods for write operations, although those are rarely needed.

Write Hooks

The following hooks are available, letting you add additional validation or perform trigger-like behavior. All of these are async.

  • beforeCreate(data)
  • afterCreate(newObj)
  • beforeUpdate(existing, changes)
  • afterUpdate(existing, changes, newObj)
  • beforeDelete(existing)
  • afterDelete(existing)
  • validate(obj) (called for both update/create flows)

Built-In Best Practices

Includes the following best practices:

  • [x] Everything is strongly typed
  • [ ] Mongo indexes for common access patterns (org + id)
  • [ ] Automatically check permissions on both read and write
  • [x] Multi-tenant safe (auto-adds organization to every query)
  • [ ] Creates audit log entries for all write actions
  • [x] Automatically runs JIT migrations
  • [x] Keep dateUpdated and dateCreated up-to-date
  • [x] Registers any new tags that are being used
  • [ ] Makes sure common foreign key references are valid (e.g. projectIds exist)
  • [ ] Within-request caching

jdorn avatar Feb 01 '24 15:02 jdorn

High-level but is there a restriction or constraint that requires us to have models defined within req.context? It would be cleaner IMO to have something like req.models. It would also avoid confusion / name-clobbering in certain cases (req.context.organizations vs. req.context.organization)

bttf avatar Feb 01 '24 22:02 bttf

High-level but is there a restriction or constraint that requires us to have models defined within req.context? It would be cleaner IMO to have something like req.models. It would also avoid confusion / name-clobbering in certain cases (req.context.organizations vs. req.context.organization)

Nice thing about it being in context is that we can use it from anywhere, not just within request handlers. I originally put them under req.context.models, but it did get pretty verbose. Open to ideas

jdorn avatar Feb 02 '24 18:02 jdorn

Nice thing about it being in context is that we can use it from anywhere, not just within request handlers.

Yeah, that's pretty important.

I originally put them under req.context.models, but it did get pretty verbose. Open to ideas

I actually don't think that's verbose at all. We will likely destructure the context object like so:

const someBusinessLogicFn = ({ org, models }: ReqContext, exp: ExperimentInterface) => {
  // ...
}

Or

const someBusinessLogicFn = (context: ReqContext, exp: ExperimentInterface) => {
  const { models: { factMetrics }, org } = context;
  const factMetric = await factMetrics.findById(...);
  // ...
}

bttf avatar Feb 07 '24 18:02 bttf

Your preview environment pr-2092-bttf has been deployed.

Preview environment endpoints are available at:

github-actions[bot] avatar Mar 05 '24 02:03 github-actions[bot]