Changed private to protected for the ResetPasswordRequestTrait

[DavidBilodeau1]

The private access modifier could prevent classes from having a level of inheritance. Changing this modifier to protected will allow doctrine to pick up the properties in inherited classes.

[bocharsky-bw]

Do you mean, if you use Doctrine entity inheritance - Doctrine does not see those private fields from mapped superclass?

[DavidBilodeau1]

Precisely, if a mapped superclass uses the ResetPasswordRequestTrait, the child of this mapped superclass will not be able to pick up the properties

[bocharsky-bw]

Hm, changes make sense probably. But to clarify, you can just use that ResetPasswordRequestTrait in both child classes, right? This should be the valid workaround IMO

[DavidBilodeau1]

Yes but it requires unnecessary changes when you have a multi-level inheritance group for users. Changing private to protected makes sure all users will have necessary properties for the reset to work.

[bocharsky-bw]

OK, this sounds Like a valid case for me, but let's see what other things

Thank you for sharing some thoughts on it.

[jrushlow]

Thanks for the contribution @DavidBilodeau1 - I'm a bit hesitant to approve this simply because once we change the visibility from private -> protected; it would take a major release to reduce the visibility again for any unforeseen circumstances.

Having said that, I definitely understand your use case for the change and I would most likely submit a PR if I was in your shoes. But, I'm also thinking about the majority of consumers that, I suspect, do not have multiple user entities that need reset functionality. Going along those lines, as @bocharsky-bw said previously, adding use ResetPasswordRequestTrait to request entities that implement the ResetPasswordRequestInterface is still doable without this change.

@weaverryan Thoughts?

[weaverryan]

Because this code is in a trait, even private methods are already accessible to the end-user. So we're already in a different situation than in a normal class (where private truly hides things, and protected starts opening them up for extension).

Also, as an example, TimestampableTrait uses protected: https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/src/Timestampable/Traits/Timestampable.php

So 👍 from me

[jrushlow]

Good point Ryan. Thanks again @DavidBilodeau1

[NOUIY]

on:
  push:
    branches: ['main']
  pull_request:
  schedule:
    - cron: '0 */12 * * *'

jobs:
  static-analysis:
    name: Static Analysis