Feature/740 fix mappings from prisma models to domain objects #775
Conversation
ognjenkurtic
requested review from
skarred14,
Therecanbeonlyone1969,
Kasshern,
Ybittan,
biscuitdey and
skosito
as code owners
| import { BpiSubjectRole } from '../src/bri/identity/bpiSubjects/models/bpiSubjectRole'; | ||
| import { IDomainObject } from '../src/shared/domainObject.interface'; | ||
|
|
||
| // We do mapping from prisma models to domain objects using simple Object.assing |
| import { BpiSubjectRole } from './bpiSubjectRole'; | ||
|
|
||
| export class BpiSubject { | ||
| export class BpiSubject implements IDomainObject { |
There was a problem hiding this comment.
why is it necessary to have the implemented interface IDomainObject and private activator. Just to ensure a common inherited value of id?
There was a problem hiding this comment.
The interface is not needed at all. I was following an article and thought that it might be a good idea in order to make sure only domain objects are accepted for activation but on second thought i think it is over engineering. Removed.
The activator is necessary to avoid instantiation of each domain object ctor by providing empty params. I was hoping for a fully generic solution (one method for any mapping pair) but couldn't make it work so stopped here for now.
|
@ognjenkurtic please clear conflicts. |
…> do on bpi subjects example
…n in prisma mapper
ognjenkurtic
force-pushed
the
feature/740-fix-mappings-from-prisma-models-to-domain-objects
branch
from
c3e8ee2
to
e0c9037
Compare
@Kasshern done and addressed all other comments, please rereview |
|
@ognjenkurtic e2e test failing |
|
e2e step is failing because of this #769 |
Gimme an approval please @Therecanbeonlyone1969 |
ognjenkurtic
deleted the
feature/740-fix-mappings-from-prisma-models-to-domain-objects
branch
Description
Implement an example of prisma model -> domain object mapping that does not rely on automapper as the automapper pojos strategy is bolierplate heavy and does not solve some edge cases.
New approach uses Object.assign and requires a single method per model - domain object pair.
If this approach looks good, i ll apply it across the codebase in a separate PR.
Related Issue
#740
Motivation and Context
Mapping was not done properly
How Has This Been Tested
Tested locally with postman, trying to fetch BpiSubject by id with associated roles and verified all mappings correctly performed.
Ran all unit tests
Screenshots (if appropriate)
Types of changes
Checklist