Skip to content
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

FullAuditedAggregateRootWithUser generic type wrong constraint #2624

Closed
RayMMond opened this issue Jan 13, 2020 · 7 comments · Fixed by #2900
Closed

FullAuditedAggregateRootWithUser generic type wrong constraint #2624

RayMMond opened this issue Jan 13, 2020 · 7 comments · Fixed by #2900

Comments

@RayMMond
Copy link
Contributor

RayMMond commented Jan 13, 2020

Anyone please tell me why FullAuditedAggregateRootWithUser need TUser to inherited from IEntity<long> while IUser interface using IEntity<Guid>?

public abstract class FullAuditedAggregateRootWithUser<TUser> : FullAuditedAggregateRoot, IFullAuditedObject<TUser>
where TUser : IEntity<long>

public abstract class FullAuditedAggregateRootWithUser<TKey, TUser> : FullAuditedAggregateRoot<TKey>, IFullAuditedObject<TUser>
where TUser : IEntity<long>

public interface IUser : IAggregateRoot<Guid>, IMultiTenant

@maliming
Copy link
Member

I am also confused about this.

@hikalkan can you explain it. Thanks.

@acjh
Copy link
Contributor

acjh commented Jan 13, 2020

I suppose that is because FullAuditedAggregateRootWithUser was committed on 14 Mar 2018 (2339fab#diff-88bc7789a4b0b95478b3bb69f9f1fc29), before @hikalkan decided to go with Guid for TKey of IUser on 20 Jun 2018 (745d891#diff-99e4bec2857b169bc53dbd6d309560c4).

FullAuditedAggregateRootWithUser may have been overlooked because it is not used anywhere (https://github.com/abpframework/abp/search?q=FullAuditedAggregateRootWithUser) and the two classes are in different projects — FullAuditedAggregateRootWithUser in the framework and IUser in the users module.

I believe we should update it in FullAuditedAggregateRootWithUser.

@hikalkan
Copy link
Member

@acjh you are right. FullAuditedAggregateRootWithUser classes have never been used and forgotten :) I will update them.

@hikalkan hikalkan changed the title FullAuditedAggregateRootWithUser generic type constraint question FullAuditedAggregateRootWithUser generic type wrong constraint Jan 13, 2020
@hikalkan
Copy link
Member

BTW, we don't suggest to use this base class since it breaks one of the Aggregate design rules: "Don't add navigation properties between aggregates, use Id instead".

But you know not everyone has to perfectly design DDD based apps. If you are using EF Core, it is a common habit to add such navigation properties.

@RayMMond
Copy link
Contributor Author

Thanks for the extra explanation about DDD :) @hikalkan

@hikalkan
Copy link
Member

You're welcome :)

@xkaede
Copy link

xkaede commented Feb 24, 2020

hi @hikalkan
AuditedAggregateRootWithUser and FullAuditedEntityWithUser are wrong too.


by the way, I am using CQRS pattern. and I find use *EntityWithUser in Query model will be convenient.
so I still need these base classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants