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

Change UserService trait to use the GenericProfile trait as the user type, instead of locking in the BasicProfile case class #545

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pkaeding
Copy link
Contributor

Since it isn't possible to subclass a case class, and implementors may have a different structure for users than the BasicProfile, and in general, it is best to stick to the Robustness Principle (and I'm guessing this is why the GenericProfile trait exists in the first place?)

In my case, I needed this because I am migrating from an old version of SecureSocial, where the IdentityId was a thing. I didn't want to migrate all of my user records in mongo, or tack on a transformation step in my DB layer. I imagine others may have other reasons for wanting to have the freedom to implement GenericProfile differently.

Of course, implementors are still free to just use BasicProfile.

pkaeding added 2 commits April 7, 2015 22:24
…sing the GenericUser trait everywhere. This causes a handful of minor ripple changes in other bits of code.
@@ -121,7 +121,7 @@ trait BasePasswordReset[U] extends MailTokenBasedOperations[U] {
case Some(profile) =>
val hashed = env.currentHasher.hash(p._1)
for (
updated <- env.userService.save(profile.copy(passwordInfo = Some(hashed)), SaveMode.PasswordChange);
updated <- env.userService.save(BasicProfile.from(profile).copy(passwordInfo = Some(hashed)), SaveMode.PasswordChange);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the extent of real code changes. Since you can't enforce that a GenericProfile has a copy constructor, I'm using the BasicProfile to affect this change.

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

Successfully merging this pull request may close these issues.

1 participant