User mailer

[hcarreras]

This is a solution for the issue 32: #32 email updates.
Is my first contribution to a open source project so please tell me any advice.
Thanks

[coveralls]

Coverage remained the same when pulling 0af45e1 on hcarreras:user_mailer into 6df3474 on hacketyhack:master.

[PragTob]

Hi there,

and welcome to the open source world! :-)
Thanks a lot for your contribution!

As for this pull request there are a few things:

• most importantly the tests are failing - at least one of them seems to be a simple typo though (reder instead of render)
• at a first glance I don't see why you implemented the index action for User - what is it used for (it is cool to have, just want to understand :-) )
• there are still a bunch of auto generated asset files and specs (with only pending specs) - it would be nice if you could remove then otherwise I/someone else will do so when merging

Thanks again and have a great weekend! :-D
Tobi

[steveklabnik]

We only need one blank space here!

[steveklabnik]

First of all, thank you so much! We really need this feature. I'm really happy you're contributing it.

Second, I left a bunch of little comments. Lots of these are style kinds of questions, so they're not wrong, per se, just trying to keep them in line with the rest of the project. :)

Tobi's feedback is also good, please get rid of the files that do nothing and make sure the tests work!

❤️ 😄

[PragTob]

Thanks Steve for your feedback! =)

So in case you are wondering how to get the changes in, you can just make them locally (on the branch from which you sent this pull request), commit them, and then push again. They will be automatically added to this pull request.

Cheers + thanks for all the work,
Tobi

[hcarreras]

Thanks both of you and thanks for the corrections!!
I'm really happy of contribute with an open source project and I hope to learn all I can.
Tobi, I don't understand you with:
"at a first glance I don't see you implemented the index action for User - what is it used to (it is cool to have, just want to understand :-) )"
What that means? A link with user/index?
I will do the necessary changes and I will commit again!

[PragTob]

@hcarreras argh stupid writing, small but important word why missing there. sorry. Correct version should be: "at a first glance I don't see why you implemented the index action for User - what is it used for (it is cool to have, just want to understand :-) )" (corrected it in my comment above). I was just asking because I saw that you implemented that action but couldn't see the direct relation to the general purpose of this pull request.

[hcarreras]

@PragTob Oh! I understand now!
Well you need an way to know who is going to receive the email, so if you go to users/index you will have a list of all users and a button to "email him"
Next step would be to have a button to "Email everyone" or something like that.
Also it would be good an user searcher.
That's how I see it, I don't know if there is a better way... what do you think?

[hcarreras]

The test give an error here, but I don't know why...
In message_mailer model I have:
default from: "steve@hackety.com"
Any idea why?

[coveralls]

Coverage remained the same when pulling 665a7f0 on hcarreras:user_mailer into 6df3474 on hacketyhack:master.

[coveralls]

Coverage decreased (-0%) when pulling 3a5de0e on hcarreras:user_mailer into 6df3474 on hacketyhack:master.

[PragTob]

Oh I've just seen this.

The next time you make corrections please comment another time, then I get a notification that something happened. Don't get that for pushes. But thanks a lot looks good now, will just remove the 2 files with only pending specs :-)

[PragTob]

So thanks a lot again for your work! It's merged now :-) ❤️