8000 [DoctrineBridge] Fix deprecation message/documentation of implementing UserProviderInterface using the entity provider by chalasr · Pull Request #20540 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[DoctrineBridge] Fix deprecation message/documentation of implementing UserProviderInterface using the entity provider #20540

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

Merged

Conversation

chalasr
Copy link
Member
@chalasr chalasr commented Nov 16, 2016
Q A
Branch? 2.8
Bug fix? yes (but the behavior doesn't change)
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #20514
License MIT
Doc PR symfony/symfony-docs#5993 (already merged)

@@ -55,7 +55,7 @@ public function loadUserByUsername($username)
throw new \InvalidArgumentException(sprintf('You must either make the "%s" entity Doctrine Repository ("%s") implement "Symfony\Bridge\Doctrine\Security\User\UserLoaderInterface" or set the "property" option in the corresponding entity provider configuration.', $this->classOrAlias, get_class($repository)));
}

@trigger_error('Implementing loadUserByUsername from Symfony\Component\Security\Core\User\UserProviderInterface is deprecated since version 2.8 and will be removed in 3.0. Implement the Symfony\Bridge\Doctrine\Security\User\UserLoaderInterface instead.', E_USER_DEPRECATED);
@trigger_error('Having a Doctrine repository implementing Symfony\Component\Security\Core\User\UserProviderInterface for loading users is deprecated since version 2.8 and will not work in 3.0. Make the repository implement Symfony\Bridge\Doctrine\Security\User\UserLoaderInterface instead.', E_USER_DEPRECATED);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT of:

- Having a Doctrine repository implementing Symfony\Component\Security\Core\User\UserProviderInterface for loading users is deprecated since version 2.8 and will not work in 3.0. Make the repository implement Symfony\Bridge\Doctrine\Security\User\UserLoaderInterface instead.
+ Implementing "Symfony\Component\Security\Core\User\UserProviderInterface" in a Doctrine repository is deprecated since version 2.8 and will not work in 3.0. Implement "Symfony\Bridge\Doctrine\Security\User\UserLoaderInterface" instead.

?

Copy link
Member Author
@chalasr chalasr Nov 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about implementing in, looks indeed better after reading it :).

About the part you removed, just implementing UserProviderInterface in a doctrine repository is still ok (no matter the why) so I think it would be clearer to mention that the deprecation is only about when using the entity provider. From what you given, what do you think about

Implementing Symfony\Component\Security\Core\User\UserProviderInterface in a Doctrine repository for loading users through an entity provider is deprecated since version 2.8 and will not work in 3.0. Make the repository implement Symfony\Bridge\Doctrine\Security\User\UserLoaderInterface instead.

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The load users could be removed, keeping just that it's only when using the entity provider, but I fail at obtaining the right sentence ^^

Copy link
Contributor
@ogizanagi ogizanagi Nov 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no matter the why

Indeed I've removed this part because it's unlikely to be implemented in another context 😅
But if you consider it's better to keep it, it's fine to me :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- Implementing Symfony\Component\Security\Core\User\UserProviderInterface in a Doctrine repository for loading users through an entity provider is deprecated since version 2.8 and will not work in 3.0. Make the repository implement Symfony\Bridge\Doctrine\Security\User\UserLoaderInterface instead.
+ Implementing Symfony\Component\Security\Core\User\UserProviderInterface in a Doctrine repository used as an entity provider is deprecated since version 2.8 and will not work in 3.0. Make the repository implement Symfony\Bridge\Doctrine\Security\User\UserLoaderInterface instead.

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @ogizanagi :)

@chalasr chalasr force-pushed the fix/doctrinebridge/misleading_deprecation branch 2 times, most recently from 7f4d4f0 to ca2a136 Compare November 16, 2016 21:33
…g UserProviderInterface using the entity provider
@chalasr chalasr force-pushed the fix/doctrinebridge/misleading_deprecation branch from ca2a136 to 127af57 Compare November 16, 2016 23:18
@stof
Copy link
Member
stof commented Nov 18, 2016

👍

@fabpot
Copy link
Member
fabpot commented Nov 18, 2016

Thank you @chalasr.

@fabpot fabpot merged commit 127af57 into symfony:2.8 Nov 18, 2016
fabpot added a commit that referenced this pull request Nov 18, 2016
…f implementing UserProviderInterface using the entity provider (chalasr)

This PR was merged into the 2.8 branch.

Discussion
----------

[DoctrineBridge] Fix deprecation message/documentation of implementing UserProviderInterface using the entity provider

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes (but the behavior doesn't change)
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #20514
| License       | MIT
| Doc PR        | symfony/symfony-docs#5993 (already merged)

Commits
-------

127af57 [DoctrineBridge] Fix deprecation message/documentation of implementing UserProviderInterface using the entity provider
@chalasr chalasr deleted the fix/doctrinebridge/misleading_deprecation branch November 18, 2016 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0