Deprecate AssertPermissionTrait

[askvortsov1]

**Fixes #1320 **

Changes proposed in this pull request:
Deprecate the AssertPermissionTrait. We should merge this in the next release to give extension authors time to adjust prior to stable.

Confirmed

• Backend changes: tests are green (run composer test).

[clarkwinkelmann]

What were the motivations behind this change ? Why get rid of AssertPermissionTrait ? This puts even more code into the User model 🤔

[franzliedke]

I believe the idea is encapsulation. These methods all operate on the user/actor objects, so it makes sense that they are owned by the User class.

Yes, this adds more code to User, but as I argue above, it kinda belongs there - to the part of it that is concerned with "actor" stuff. The class inherits from the Eloquent model god class, so this is merely a drop in the bucket. 😆

On a more serious note. I could see us reducing or at least separating the responsibilities by introducing an interface for actor-related things. That would help in e.g. at least decoupling Guest from all the Eloquent stuff. That might help simplify certain tests.

[franzliedke]

Looks good, I'd be happy with this change! (Want to wait for others' responses.)

I found two small copy-paste mistakes (?), though...

[luceos]

Yes, this adds more code to User, but as I argue above, it kinda belongs there

@franzliedke can't we move this out of the User class into a Concerns/trait please? It's what I proposed in the user preferences as well and it would help keep Users clean.

[franzliedke]

@luceos We could. Personally, I rarely see the benefit in extracting a trait if it's not for the purpose of sharing code. In this case, we would just try to conceal the fact that the class is too big / doing too much. On the downside, I tend to be surprised by traits, because I often forget to consider them (or simply overlook them) when searching for a method's implementation.

Not too strong an opinion, curious to hear what others think.

[askvortsov1]

I also see traits as a mechanism for sharing functionality among unrelated classes moreso than a way to split up big classes 👍

[franzliedke]

Please squash and rebase, then I'll gladly merge.

[franzliedke]

@askvortsov1 That did not go well. 😱

[askvortsov1]

@franzliedke here we go :D 🙈

[tankerkiller125]

Looks good, I don't see any more issues and it appears that Franz problem was resolved.

[franzliedke]

Any volunteers for applying this change to all bundled extensions? 😁

[tankerkiller125]

@franzliedke I'll take care of it this weekend once I get done meeting the dog I might be adopting.