Fix crud-generator position group for ref-fields #2613
Conversation
Ben-Ho
force-pushed
the
fix-crud-generator-position
branch
from
1240f92
to
cb9b121
Compare
| .map( | ||
| (prop) => | ||
| `${prop.name}${prop.nullable ? `?` : ``}: ${ | ||
| [ReferenceType.EMBEDDED, ReferenceType.SCALAR].includes(prop.reference) ? prop.type : "string" |
There was a problem hiding this comment.
@johnnyomair @nsams I think we don't want to support OneToMany and ManyToMany fields in groupByFields. Should I throw an error?
|
@johnnyomair I added an example in demo. I also found another bug when a dedicatedResolverArg is used for position-group (which are probably one of the most used fields) |
| @InjectRepository(ProductVariant) private readonly repository: EntityRepository<ProductVariant>, | ||
| ) {} | ||
|
|
||
| async incrementPositions(group: { product: string }, lowestPosition: number, highestPosition?: number) { |
There was a problem hiding this comment.
Wouldn't it be better if the product entity was passed here, and not the ID?
There was a problem hiding this comment.
this would be much more complicated. in this case grouping is done by references to other entities. they can be configured differently. most of the time it will probably be something like this:
@ManyToOne(() => MyEntity, { ref: true })
myReferencedEntity?: Ref<MyEntity>;
but it could also be a eager reference. They require different code. id is always present.
CRUD-Generator position-feature generates wrong code if groupByFields contains Ref field.
Ref field requires to generate
baseEntity.refEntity.id(orbaseEntity.refEntity?.idif optional) to call position-functions from service, but was alwaysbaseEntity.refEntity.Type needs to be string for Ref and prop.type for Scalars and Embedded. OneToMany and ManyToMany shouldn't be possible at all.