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

[BE]Signup validations #70

Merged
merged 10 commits into from
Jan 13, 2022
Merged

[BE]Signup validations #70

merged 10 commits into from
Jan 13, 2022

Conversation

apoorv-mishra
Copy link
Contributor

apoorv-mishra and others added 5 commits January 11, 2022 17:10
* adding the org setup

* redirecting after org setup to the root page

* adding the org setup

* redirecting after org setup to the root page

* minor fix

* minor fix

* did the functionality

* added company_id

* added controller

* styled the org setup page without the two tabs

* added the new schema with country and timezone in the model

* added company factory

* removed comments from factory

* text field for business phone

* deleted unwanted migrations

* used up and down instead of change

* model constraint added to companies migration

* Company model validation added

* schema and validation updated

* company attribute and validation updated

* remove logo_url coloumn from companies table

* seed added and typo fixed

* recreate gemfile.lock

* unit test for company model added

* removed test coverage

* added linux arch in gemlock file

* company routes added

* role and user-company migration added

* typo fixed in AddCompanyIdToUser

* restored previous migration

* removed null: false constraint from AddCompanyIdToUser

* company form updated

* replaced city-state gem with countries gem

* conflict fix

* Add minor changes

* Dependent destroy added

Co-authored-by: Akhil G Krishnan <akhilgkrishnan4u@gmail.com>
Co-authored-by: Abinash Panda <abinashp@protonmail.ch>
* removed role attribute from user

* removed roles from user

* added rolify

* fixed schema

* code review changes
* Rails upgraded to 7.0.1 and Ruby upgraded to 3.1.0

* bundle lock fix
@vipulnsward vipulnsward temporarily deployed to miru-review-pr-70 January 12, 2022 07:22 Inactive
@akhilgkrishnan
Copy link
Member

@apoorv-mishra can you rebase your branch with develop. Also please fix the confict.

@vipulnsward vipulnsward temporarily deployed to miru-review-pr-70 January 12, 2022 07:51 Inactive
@github-actions
Copy link

github-actions bot commented Jan 12, 2022

Current Code Coverage Percent of this PR:

78.7 %

Files having coverage below 100%

Impacted Files Coverage
/config/application.rb 92.31 %
/config/routes.rb 87.5 %
/app/controllers/root_controller.rb 33.33 %
/app/controllers/application_controller.rb 66.67 %
/app/helpers/users/registrations_helper.rb 41.67 %
/app/controllers/company_controller.rb 42.11 %
/app/controllers/users/sessions_controller.rb 25.0 %
/app/controllers/users/registrations_controller.rb 80.0 %

validates :first_name, :last_name, :email, :encrypted_password, presence: true
validates :first_name, :last_name,
presence: true,
format: { with: /\A[a-zA-Z]+\z/, message: "First and last name must only contain letters" }
Copy link
Member

Choose a reason for hiding this comment

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

Can you move the message to en.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


# Include default devise modules. Others available are:
# :confirmable, :lockable, :timeoutable, :trackable and :omniauthable
devise :database_authenticatable, :registerable,
:recoverable, :rememberable, :validatable,
:trackable, :confirmable

validates :email, format: { with: /^([^\s]+)((?:[-a-z0-9]+\.)+[a-z]{2,})$/i, multiline: true }
Copy link
Member

Choose a reason for hiding this comment

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

can you move all the validates in one place

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


# Include default devise modules. Others available are:
# :confirmable, :lockable, :timeoutable, :trackable and :omniauthable
devise :database_authenticatable, :registerable,
:recoverable, :rememberable, :validatable,
:trackable, :confirmable

validates :email, format: { with: /^([^\s]+)((?:[-a-z0-9]+\.)+[a-z]{2,})$/i, multiline: true }
Copy link
Contributor

@keshavbiswa keshavbiswa Jan 13, 2022

Choose a reason for hiding this comment

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

We can replace the regex with devise's default regex inside devise.rb config.email_regexp. Either replace the default value with this or use the default value from there itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks for the tip @keshavbiswa . I was actually looking to override devise's default email validation since it's weaker and validates strings such as foo@bar.

@vipulnsward vipulnsward temporarily deployed to miru-review-pr-70 January 13, 2022 06:38 Inactive
Copy link
Member

@akhilgkrishnan akhilgkrishnan left a comment

Choose a reason for hiding this comment

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

LGTM 🎉 .

Copy link
Contributor

@keshavbiswa keshavbiswa left a comment

Choose a reason for hiding this comment

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

LGTM! 👍🏻

@apoorv-mishra apoorv-mishra merged commit bf8bcc7 into develop Jan 13, 2022
@akhilgkrishnan akhilgkrishnan deleted the signup-validations branch March 8, 2022 09:00
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.

Fix the issues on Signup/login page
5 participants