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

Added is_verified column to users with default value false #3885

Conversation

shubhscoder
Copy link
Contributor

Fixes #3849

  1. Added is_verified column to users to with default value false

Thanks!

@shubhscoder
Copy link
Contributor Author

@Radhikadua123 I am deleting the previous PR and as you said I have made this PR for #3849
Now I will start working on #3848
I am thinking of using the following method:

  1. Add a column to the users table to store the unique token that is generated for the user.
  2. Implement a function generate_token(user,expiry_time) that will generate a randomized token for the user. This function also stores the generated token in the database.
  3. Implement a function validate_token(user,token) that will validate the token and also clear it from the database as the link would be a one time link. This function will also update the is_verified field for the user.

What I am not getting is how are we using the expiry time?

@shubhscoder
Copy link
Contributor Author

Also, I need some help on finding out why did the test fail and what can be done to fix it. Thanking you in advance.

@Radhikadua123
Copy link
Contributor

Radhikadua123 commented Nov 3, 2018

@shubhscoder The approach which you are following, needs saving token into the db. But it would be better if we don't save token in db at all. To know how to implement that up, you can check out approach 2 on this comment #3522 (comment)

Do let me know in case you have any doubts regarding that approach.

About the failing test, @jywarren can you please help us out ? I'm not much aware of the environment, but shouldn't the travis CI delete db before running rake db:setup ? And then run rake db:migrate.

@shubhscoder
Copy link
Contributor Author

Okay so we can use the second approach. It won't require the addition of one more column. We will have to use some gem to get the encryption and decryption done.

@Radhikadua123
Copy link
Contributor

@shubhscoder Encryption/Decryption was added in #3801

@Radhikadua123
Copy link
Contributor

Deferred to #3907

@Radhikadua123
Copy link
Contributor

I saw that you removed these changes from #3907

That's a good practice to make atomic changes in the PR.

So, I'm reopening this PR.

@Radhikadua123 Radhikadua123 reopened this Nov 9, 2018
@ghost ghost assigned Radhikadua123 Nov 9, 2018
@ghost ghost added the in progress label Nov 9, 2018
@Radhikadua123
Copy link
Contributor

@jywarren Since I'm new to rails, I'm inexperienced with db changes. Though the column addition definition seems fine to me.

Can you please review this PR ?

Also will it cause down time during the time migration will be ran on the db ? Is that a concern for us ? Any idea ? Also may be @icarito knows about it.

@jywarren
Copy link
Member

jywarren commented Nov 9, 2018

Hi, no, it won't create significant downtime, no worries! So, now you have to update the /db/schema.rb.example file to reflect the new changes (just those related to your updates) and this should build properly.

Thanks!!!

@ghost ghost removed the in progress label Nov 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants