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

Features/find user by reset token interactor #57

Merged
merged 4 commits into from
Jan 16, 2016

Conversation

npauzenga
Copy link
Owner

Hey @enriikke what do you think of these specs? Also, I'm right in my thinking that the User is stored with the hashed version of the password reset token but we pass the raw version in the link in the email, right?

Adapted from your class, token[0] is hashed, token[1] is raw.

@enriikke
Copy link
Collaborator

That's correct! You send the user the URL safe unencrypted token but internally that token is hashed when storing it. Awesome work @npauzenga!! :neckbeard: 🍶 🌵

context "when successful" do
before do
@user = create(:confirmed_user, reset_digest: token[0])
end
Copy link
Owner Author

Choose a reason for hiding this comment

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

@enriikke rubocop doesn't like this but I need the user to be instantiated before the subject is called. Using an instance variable seemed like the cleaned way to make this work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh that darn rubocop! Always nagging 👮

Best practice says you tend to avoid using instance variables, and that's probably why rubocop is complaining. You can approach this in a couple different ways.

One is to not use a variable at all and simply call create in the before block. You only have a single test that is using the variable to compare to the interactor.user. You could argue that you don't really need to validate that though. You can test that you are getting a user back but are not concerned that it was the user you created. You leave that to AR's find_by method.

You could also say: "but I really want to know that I'm getting the right guy". In that case you can make use or rspec's let! (let bang). Our known friend let works by lazy evaluating its block, in contrast let! evaluates before the example is executed.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Hmm I see what you mean about not concerning ourselves and allocating responsibility to find_by.

Even though ActiveRecord is responsible here, I still have to test the line anyway and I tend to think being more explicit in my expectation is better? Though maybe it's better to really be extra sure not to test things that are already thoroughly tested on their own. That way your tests cover very strictly only your contributions. I'm conflicted.

@npauzenga
Copy link
Owner Author

@enriikke, Boom! Green specs.

npauzenga added a commit that referenced this pull request Jan 16, 2016
…en-interactor

Features/find user by reset token interactor
@npauzenga npauzenga merged commit c4f86dc into staging Jan 16, 2016
@npauzenga npauzenga deleted the features/find-user-by-reset-token-interactor branch January 16, 2016 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants