Verification link in welcome mail #3992

shubhscoder · 2018-11-16T16:21:54Z

The welcome email would now contain, link for verification of email
Upon successful verification, the is_verified gets updated

plotsbot · 2018-11-16T16:43:20Z

	1 Message
📖	@shubhscoder Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.

Generated by 🚫 Danger

tsparksh · 2018-11-18T04:23:02Z

if decrypted_user_id != 0
    User.find(decrypted_user_id).update_column(:is_verified, true)
    action_msg = "Successfully verified email"
end

If I'm right, this code will update the is_verified field even when the user has already activated an account. Can you change it?

tsparksh

Thank you!

jywarren · 2018-11-20T22:43:33Z

app/views/welcome_mailer/notify_newcomer.html.erb

@@ -18,6 +18,13 @@

    Looking forward to hearing from you soon,

+    Please click on the link below to verify your email.
+
+    <% verifaction_link="https://publiclab.org/verify/#{@user.generate_token}" %>


Here we could replace publiclab.org with <%= ActionMailer::Base.default_url_options[:host] %> so it's not host-specific!

Yeah I fixed that. Thank you so much!

jywarren

Hello, would you be able to add a functional test for this ? Thank you!!!

shubhscoder · 2018-11-30T11:42:52Z

Yes I ll do that asap. I am kindoff stuck with semester exams ! Will do that in a day or two . Thank you so much !

…

On Fri, Nov 30, 2018, 9:57 AM Jeffrey Warren ***@***.***> wrote: ***@***.**** requested changes on this pull request. Hello, would you be able to add a functional test for this ? Thank you!!! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3992 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ARofxUU8FIKiF5gcv0wofIUeBEK4aAVuks5u0LO5gaJpZM4Ymj0I> .

jywarren · 2018-11-30T18:45:13Z

good luck with your exams! On Fri, Nov 30, 2018 at 6:43 AM Shubham Sangamnerkar < notifications@github.com> wrote:

…

Yes I ll do that asap. I am kindoff stuck with semester exams ! Will do that in a day or two . Thank you so much ! On Fri, Nov 30, 2018, 9:57 AM Jeffrey Warren ***@***.***> wrote: > ***@***.**** requested changes on this pull request. > > Hello, would you be able to add a functional test for this ? Thank you!!! > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > < #3992 (review) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/ARofxUU8FIKiF5gcv0wofIUeBEK4aAVuks5u0LO5gaJpZM4Ymj0I > > . > — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3992 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ0ogXE2uVgijALJn6zAlzvA9zYulks5u0Rm9gaJpZM4Ymj0I> .

shubhscoder · 2018-12-17T09:13:00Z

I tried writing the tests and encountered this error "undefined method []' for true:TrueClass from lib/authlogic_openid/authlogic_openid/acts_as_authentic.rb:74:in save'
" on line " user_obj.update(:is_verified => true)" in users_controller. Tried debugging it but to no avail. @jywarren do you know why this is happening? Thanking you in advance!

jywarren · 2018-12-17T16:04:57Z

Is it in the Travis tests or on your local? Try rerunning them, I think there is an intermittent issue with Travis... Hope that helps!

…

On Mon, Dec 17, 2018, 4:13 AM Shubham Sangamnerkar ***@***.*** wrote: I tried writing the tests and encountered this error "undefined method []' for true:TrueClass from lib/authlogic_openid/authlogic_openid/acts_as_authentic.rb:74:in save' " on line " user_obj.update(:is_verified => true)" in users_controller. Tried debugging it but to no avail. @jywarren <https://github.com/jywarren> do you know why this is happening? Thanking you in advance! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3992 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ_9JimkWW4eGCgxyGyewDkY3mIhFks5u52AdgaJpZM4Ymj0I> .

shubhscoder · 2018-12-17T16:21:07Z

Its not passing even on my local machine , would definitely fail on Travis ! On Mon, Dec 17, 2018, 9:35 PM Jeffrey Warren <notifications@github.com> wrote:

…

Is it in the Travis tests or on your local? Try rerunning them, I think there is an intermittent issue with Travis... Hope that helps! On Mon, Dec 17, 2018, 4:13 AM Shubham Sangamnerkar < ***@***.*** wrote: > I tried writing the tests and encountered this error "undefined method []' > for true:TrueClass from > lib/authlogic_openid/authlogic_openid/acts_as_authentic.rb:74:in save' > " on line " user_obj.update(:is_verified => true)" in users_controller. > Tried debugging it but to no avail. @jywarren > <https://github.com/jywarren> do you know why this is happening? Thanking > you in advance! > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#3992 (comment)>, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AABfJ_9JimkWW4eGCgxyGyewDkY3mIhFks5u52AdgaJpZM4Ymj0I > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3992 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ARofxaKlJJ_3OAPQmtrcLR1pCpVDWBukks5u58CrgaJpZM4Ymj0I> .

jywarren · 2018-12-18T16:42:39Z

Hmm, @SidharthBansal do you have any ideas on this?

SidharthBansal · 2018-12-19T08:02:30Z

Can you @shubhscoder please push in the failing tests so that we can help you?

shubhscoder · 2018-12-22T09:30:33Z

@SidharthBansal I have pushed the failing tests. Sorry for the delay!

shubhscoder · 2018-12-30T15:32:53Z

@SidharthBansal @jywarren I realised why the tests are failing. The users_controller_test is using the fixtures to get its object, where as the object gets updated in the users_controller, which updates it in the sql database, that is why the is_verified of the fixtures object never gets updated. I don't think there is a way to write tests for such a scenario. I have tested everything on the localhost I can make a video of the same to make sure that everything is working. Any help/ suggestion is welcome. Thank you so much!

Minor changes Redirection of routes Added verification link in the welcome email Update is_verified only if user is not already verified Tests added fixed tests

shubhscoder · 2019-01-08T17:42:11Z

@SidharthBansal check if this looks good to you, or we need more changes. Thank you so much!

SidharthBansal · 2019-01-13T17:38:13Z

@jywarren changes looks good to me. It is important PR so your careful review is must.

shubhscoder · 2019-02-06T17:09:26Z

@jywarren I think this is ready! You can check this? I can make any further changes if required.

jywarren · 2019-02-06T20:51:29Z

This is really nicely implemented, thanks! Just a few questions:

what currently happens when unverified?
relatedly, could we roll this out and then measure a bit whether people are successfully verifying even if we aren't yet requiring it? That'd give us a chance to build confidence in this workflow
what about people who join by OAuth, do we exempt them?

Thanks!

shubhscoder · 2019-02-07T04:44:01Z

@jywarren

Currently if the user is not verified then nothing happens (He is allowed to access everything) but after we merge addition of flash to verify email #4493 the user will start getting a prompt on the dashboard page to verify his email( Again even after addition of flash to verify email #4493 he is not restricted from doing anything.)
Yeah, that sounds a good idea, but in this PR the current users wont know that we have introduced a new email verification process. We would be able to check with only the people who join public labs after we merge this.
For the people joining with Oauth we have not made any provision yet, but yes we could exempt them if required
Thank you!

jywarren · 2019-02-07T22:33:23Z

This is great! Thank you for the clear response.

current users wont know that we have introduced a new email verification process. We would be able to check with only the people who join public labs after we merge this.

Great, this is fine. Let's make these follow-up steps in an issue and this can be merged!

jywarren · 2019-02-07T22:36:04Z

And... THANK YOU!!! 👍 👍 👍

grvsachdeva · 2019-02-08T21:24:35Z

app/views/welcome_mailer/notify_newcomer.html.erb

+    <%= link_to "#{verifaction_link}", verifaction_link%>
+
+    In case you are not able to click the link, try copy pasting the link in the browser.
+


Hi @jywarren make sure too copy this change to the welcome-email-body feature whenever you push this PR to production as the changes made here are just on the fallback email body and the production would send the email using the welcome-email-body feature's content(if it's present).

Thanks!

Ah, then this is an issue - we can't execute code in the features system, so this will need to be moved outside the <% end %> on this line: https://github.com/publiclab/plots2/pull/3992/files/b74bbcbd3d85ff70b18bc7eae14bf79105270976#diff-a3214821bc5b12012e6fbb54ad82f05aR29

@jywarren . Sorry but I am not getting what we are supposed to do?

Oh I think I got it. We need to move the verification link and the corresponding instructions regarding the mail outside the if else block right? Because if body is present then the verification link wont appear , so we need to move it outside the block. I will make a new PR for that. Thank you so much for the help!

jywarren · 2019-02-08T22:44:34Z

@shubhscoder do you see our comments above? I think this needs one more change, sorry!

grvsachdeva · 2019-02-10T11:22:30Z

Yes, Shubham!

…

On Sun, Feb 10, 2019 at 12:38 PM Shubham Sangamnerkar < ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In app/views/welcome_mailer/notify_newcomer.html.erb <#3992 (comment)>: > @@ -18,6 +18,13 @@ Looking forward to hearing from you soon, + Please click on the link below to verify your email. + + <% ***@***.***_token}" %> + <%= link_to "#{verifaction_link}", verifaction_link%> + + In case you are not able to click the link, try copy pasting the link in the browser. + Oh I think I got it. We need to move the verification link and the corresponding instructions regarding the mail outside the if else block right? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3992 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AT6S9uvXm5KJVxZuC_NITFVhBt6zc8ABks5vL8WHgaJpZM4Ymj0I> .

Minor changes Redirection of routes Added verification link in the welcome email Update is_verified only if user is not already verified Tests added fixed tests

tsparksh approved these changes Nov 18, 2018

View reviewed changes

jywarren reviewed Nov 20, 2018

View reviewed changes

jywarren requested changes Nov 30, 2018

View reviewed changes

SidharthBansal added this to the OAuth milestone Dec 18, 2018

shubhscoder force-pushed the verification_link_in_welcome_mail branch from b0c50d2 to f5b97ce Compare December 22, 2018 09:29

SidharthBansal removed this from the OAuth milestone Dec 24, 2018

shubhscoder mentioned this pull request Jan 1, 2019

addition of flash to verify email #4493

Closed

5 tasks

shubhscoder closed this Jan 8, 2019

shubhscoder reopened this Jan 8, 2019

Added functionality to verify email from token

b74bbcb

Minor changes Redirection of routes Added verification link in the welcome email Update is_verified only if user is not already verified Tests added fixed tests

shubhscoder force-pushed the verification_link_in_welcome_mail branch from d20d40f to b74bbcb Compare January 8, 2019 16:39

SidharthBansal approved these changes Jan 13, 2019

View reviewed changes

jywarren merged commit e731d60 into publiclab:master Feb 7, 2019

shubhscoder mentioned this pull request Feb 8, 2019

Failure in sending welcome email #4777

Closed

grvsachdeva reviewed Feb 8, 2019

View reviewed changes

shubhscoder mentioned this pull request Feb 10, 2019

Move verification link #4786

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Verification link in welcome mail #3992

Verification link in welcome mail #3992

shubhscoder commented Nov 16, 2018

plotsbot commented Nov 16, 2018 •

edited

Loading

tsparksh commented Nov 18, 2018

tsparksh left a comment

jywarren Nov 20, 2018

shubhscoder Nov 22, 2018 •

edited

Loading

jywarren left a comment

shubhscoder commented Nov 30, 2018 via email

jywarren commented Nov 30, 2018 via email

shubhscoder commented Dec 17, 2018

jywarren commented Dec 17, 2018 via email

shubhscoder commented Dec 17, 2018 via email

jywarren commented Dec 18, 2018

SidharthBansal commented Dec 19, 2018

shubhscoder commented Dec 22, 2018

shubhscoder commented Dec 30, 2018 •

edited

Loading

shubhscoder commented Jan 8, 2019

SidharthBansal commented Jan 13, 2019

shubhscoder commented Feb 6, 2019

jywarren commented Feb 6, 2019

shubhscoder commented Feb 7, 2019

jywarren commented Feb 7, 2019

jywarren commented Feb 7, 2019

grvsachdeva Feb 8, 2019

jywarren Feb 8, 2019

shubhscoder Feb 10, 2019

shubhscoder Feb 10, 2019 •

edited

Loading

jywarren commented Feb 8, 2019

grvsachdeva commented Feb 10, 2019 via email

		<%= link_to "#{verifaction_link}", verifaction_link%>

		In case you are not able to click the link, try copy pasting the link in the browser.

Verification link in welcome mail #3992

Verification link in welcome mail #3992

Conversation

shubhscoder commented Nov 16, 2018

plotsbot commented Nov 16, 2018 • edited Loading

tsparksh commented Nov 18, 2018

tsparksh left a comment

Choose a reason for hiding this comment

jywarren Nov 20, 2018

Choose a reason for hiding this comment

shubhscoder Nov 22, 2018 • edited Loading

Choose a reason for hiding this comment

jywarren left a comment

Choose a reason for hiding this comment

shubhscoder commented Nov 30, 2018 via email

jywarren commented Nov 30, 2018 via email

shubhscoder commented Dec 17, 2018

jywarren commented Dec 17, 2018 via email

shubhscoder commented Dec 17, 2018 via email

jywarren commented Dec 18, 2018

SidharthBansal commented Dec 19, 2018

shubhscoder commented Dec 22, 2018

shubhscoder commented Dec 30, 2018 • edited Loading

shubhscoder commented Jan 8, 2019

SidharthBansal commented Jan 13, 2019

shubhscoder commented Feb 6, 2019

jywarren commented Feb 6, 2019

shubhscoder commented Feb 7, 2019

jywarren commented Feb 7, 2019

jywarren commented Feb 7, 2019

grvsachdeva Feb 8, 2019

Choose a reason for hiding this comment

jywarren Feb 8, 2019

Choose a reason for hiding this comment

shubhscoder Feb 10, 2019

Choose a reason for hiding this comment

shubhscoder Feb 10, 2019 • edited Loading

Choose a reason for hiding this comment

jywarren commented Feb 8, 2019

grvsachdeva commented Feb 10, 2019 via email

plotsbot commented Nov 16, 2018 •

edited

Loading

shubhscoder Nov 22, 2018 •

edited

Loading

shubhscoder commented Dec 30, 2018 •

edited

Loading

shubhscoder Feb 10, 2019 •

edited

Loading