Bacon / BaconUser

BaconUser provides simple user management
BSD 2-Clause "Simplified" License
14 stars 9 forks source link

User reference in password reset entity #21

Closed Ocramius closed 11 years ago

Ocramius commented 11 years ago

As discussed on IRC, it makes no sense to directly store the email address of a user in a password reset request. Instead, it makes sense to have a user object being reference, and therefore the password requests being cascade-deleted.

This PR fixes that by introducing the user repository interface, a user repository implementation via composition and modifying the password repository implementation.

Ocramius commented 11 years ago

To be noted that I'd probably prefer the password reset repository to handle the password reset instantiation, which reduces coupling, complexity and also assumes that there can always be a password request for an existing user.

While this moves some instantiation logic to the repository, I think it is a valid use case, since it abstracts the idea "for an existing email, I can always retrieve a password reset token"

coveralls commented 11 years ago

Coverage Status

Coverage remained the same when pulling f0fe9cd70a64cea183e0a604a9662d81ff3de68e on Ocramius:feature/user-id-in-password-reset into 9445a5df1136aadcfa3114c5839110a5f3c85776 on Bacon:master.

Ocramius commented 11 years ago

@DASPRiD applied required CS fixes - mergeable IMO

coveralls commented 11 years ago

Coverage Status

Coverage remained the same when pulling 15e8608261eef595315309a06ceafe3e4963e554 on Ocramius:feature/user-id-in-password-reset into 9445a5df1136aadcfa3114c5839110a5f3c85776 on Bacon:master.

coveralls commented 11 years ago

Coverage Status

Coverage remained the same when pulling 4ec7b8dbe8da496ed8c00ac9ab4d188897c7024f on Ocramius:feature/user-id-in-password-reset into 9445a5df1136aadcfa3114c5839110a5f3c85776 on Bacon:master.

Ocramius commented 11 years ago

@DASPRiD sad green merge button is sad :(