-
Notifications
You must be signed in to change notification settings - Fork 57
Allow using auth filters on creation #321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import { CreateOneOptions } from './create-one-options.interface' | ||
|
||
export type CreateManyOptions<DTO> = CreateOneOptions<DTO> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import { Filter } from './filter.interface' | ||
|
||
export interface CreateOneOptions<DTO> { | ||
/** | ||
* Additional filter applied to the input dto before creation. This could be used to apply an additional filter to ensure | ||
* that the entity being created belongs to a particular user. | ||
*/ | ||
filter?: Filter<DTO> | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,8 @@ import { NotFoundException } from '@nestjs/common' | |
import { | ||
AggregateQuery, | ||
AggregateResponse, | ||
applyFilter, | ||
CreateOneOptions, | ||
DeepPartial, | ||
DeleteManyResponse, | ||
DeleteOneOptions, | ||
|
@@ -132,9 +134,13 @@ export class MongooseQueryService<Entity extends Document> | |
* const todoItem = await this.service.createOne({title: 'Todo Item', completed: false }); | ||
* ``` | ||
* @param record - The entity to create. | ||
* @param opts - Additional options. | ||
*/ | ||
async createOne(record: DeepPartial<Entity>): Promise<Entity> { | ||
async createOne(record: DeepPartial<Entity>, opts?: CreateOneOptions<Entity>): Promise<Entity> { | ||
this.ensureIdIsNotPresent(record) | ||
if (opts?.filter && !applyFilter(record as Entity, opts.filter)) { | ||
throw new Error('Entity does not meet creation constraints') | ||
} | ||
Comment on lines
+141
to
+143
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just doing some type casting here to make this happy but I think raises a good question. What should be the behaviour for a filter like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something we could do, but that would make it all a lot more complex is by doing inside a TypeORM query runner, doing the insert, getting the record, apply filter (can even then be done with the normal builder), if failed revert and throw error. Otherwise I think this would also need to be documented as a limitation. (Preferred) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's my preference too. |
||
return this.Model.create(record) | ||
} | ||
|
||
|
@@ -149,10 +155,11 @@ export class MongooseQueryService<Entity extends Document> | |
* ]); | ||
* ``` | ||
* @param records - The entities to create. | ||
* @param opts - Additional options. | ||
*/ | ||
public async createMany(records: DeepPartial<Entity>[]): Promise<Entity[]> { | ||
public async createMany(records: DeepPartial<Entity>[], opts?: CreateOneOptions<Entity>): Promise<Entity[]> { | ||
records.forEach((r) => this.ensureIdIsNotPresent(r)) | ||
return this.Model.create(records) | ||
return this.Model.create(opts?.filter ? applyFilter(records as Entity[], opts.filter) : records) | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,10 @@ import { | |
AggregateOptions, | ||
AggregateQuery, | ||
AggregateResponse, | ||
applyFilter, | ||
Class, | ||
CountOptions, | ||
CreateOneOptions, | ||
DeepPartial, | ||
DeleteManyOptions, | ||
DeleteManyResponse, | ||
|
@@ -190,12 +192,15 @@ export class TypeOrmQueryService<Entity> | |
* const todoItem = await this.service.createOne({title: 'Todo Item', completed: false }); | ||
* ``` | ||
* @param record - The entity to create. | ||
* @param opts - Additional options. | ||
*/ | ||
public async createOne(record: DeepPartial<Entity>): Promise<Entity> { | ||
public async createOne(record: DeepPartial<Entity>, opts?: CreateOneOptions<Entity>): Promise<Entity> { | ||
const entity = await this.ensureIsEntityAndDoesNotExist(record) | ||
|
||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore | ||
if (opts?.filter && !applyFilter(entity, opts.filter)) { | ||
throw new Error('Entity does not meet creation constraints') | ||
} | ||
Comment on lines
+200
to
+202
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally I'm thinking we should remove this check and only pass it down to the service, so that, when users want to they can use it there, main reason being: I would not want the service even beign called when the user does not have the right access (that's why on our API's we are using the before create). This would also make it a breaking change so also not really in favor of that. Would it work for you if we do pass it down (as that is a good idea to do!) but do not check the filter/throw a error? Especially as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be ignored as it will become opt-in, would be good to document on the flag that it could/would not work with relation filters though. |
||
|
||
return this.repo.save(entity) | ||
} | ||
|
||
|
@@ -210,12 +215,11 @@ export class TypeOrmQueryService<Entity> | |
* ]); | ||
* ``` | ||
* @param records - The entities to create. | ||
* @param opts - Additional options. | ||
*/ | ||
public async createMany(records: DeepPartial<Entity>[]): Promise<Entity[]> { | ||
public async createMany(records: DeepPartial<Entity>[], opts?: CreateOneOptions<Entity>): Promise<Entity[]> { | ||
const entities = await Promise.all(records.map((r) => this.ensureIsEntityAndDoesNotExist(r))) | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore | ||
return this.repo.save(entities) | ||
return this.repo.save(opts?.filter ? applyFilter(entities, opts.filter) : entities) | ||
Smtih marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/** | ||
|
Uh oh!
There was an error while loading. Please reload this page.