Feature#37 add private number #44
Conversation
Co-athored-by: danielasotomayor99 <[email protected]>
Co-authored-by: danielasotomayor99 <[email protected]>
Co-authored-by: danielasotomayor99 <[email protected]>
Co-authored-by: danielasotomayor99 <[email protected]>
| @@ -11,6 +11,10 @@ def show | |||
|
|
|||
| def new | |||
| @employee = Employee.new | |||
| @private_number = rand(111111..999999) | |||
| if Employee.exists?(private_number: @private_number) | |||
| @private_number = rand(111111..999999) | |||
There was a problem hiding this comment.
What if the new private number generated has also been already assigned to another Employee?
There was a problem hiding this comment.
We have validation for that in the model using uniqueness: true
| @private_number = rand(111111..999999) | ||
| if Employee.exists?(private_number: @private_number) | ||
| @private_number = rand(111111..999999) | ||
| end |
There was a problem hiding this comment.
You are removing changes that you just added in a previous commit. I suggest making all the private number changes in the same commit.
| @@ -21,7 +34,7 @@ | |||
|
|
|||
| <div class="form-group"> | |||
| <%= label_tag 'private_number', 'Private Number:'%> | |||
| <%= form.text_field :private_number, class:'form-control', value: @private_number, disabled: true%> | |||
| <%= form.text_field :private_number, class:'form-control'%> | |||
There was a problem hiding this comment.
I don't think manually setting the employee number is good. Why not automatically assign an employee number using a generator?
There was a problem hiding this comment.
We thought about that and we had a lot of trouble and because of time decided to implement it manually.
| @@ -14,7 +14,11 @@ | |||
| *= require_self | |||
| */ | |||
|
|
|||
| .alert-notice{ | |||
| .alert-notice { | |||
| display: none; | |||
There was a problem hiding this comment.
I think in CSS standard is to use two spaces and you are using 4 here.
db/schema.rb
Outdated
| @@ -33,10 +33,14 @@ | |||
| t.string "name" | |||
| t.string "email" | |||
| t.string "position" | |||
| <<<<<<< HEAD | |||
There was a problem hiding this comment.
This shouldn't be here. These are Git anotation from an earlier rebase.
| admin.password_confirmation = "12345" | ||
| admin.save | ||
|
|
||
| Admin.create(user: 'admin', password_digest: 'admin') |
There was a problem hiding this comment.
Did you verify this seeds to be creating valid users?
There was a problem hiding this comment.
Yes, we had mentorship for that
There was a problem hiding this comment.
Thank you. Next time you get a suggestion, try to apply it in the same commit if possible. In this scenario if for some reason you remove the last commit, then your migrations are going to stop working because the comment is in the previous comment.
You can try git commit --amend or an interactive rebase with git rebase -i
Description
Testing
None.
Screenshots