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

Create device for User - EXTRA: set boolean for Admin in User schema #11

Merged
merged 5 commits into from
Mar 28, 2017

Conversation

Winkeltje84
Copy link
Owner

Created devise for user with extra: Admin boolean - standard to false

@@ -0,0 +1,5 @@
FactoryGirl.define do
Copy link
Contributor

Choose a reason for hiding this comment

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

If you're not using it, remove it!

Copy link
Owner Author

Choose a reason for hiding this comment

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

using it now and added factory for user

@@ -0,0 +1,5 @@
require 'rails_helper'
Copy link
Contributor

Choose a reason for hiding this comment

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

idem

t.string :email, null: false, default: ""
t.string :encrypted_password, null: false, default: ""
# created extra boolean whether user is admin or not
t.boolean :admin, default: false
Copy link
Contributor

Choose a reason for hiding this comment

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

Please create test to expose/ensure that a user won't be an admin by default (as it is important business logic). Also, add a test to show your colleagues how to add an admin user to the system.

Copy link
Owner Author

Choose a reason for hiding this comment

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

added specs

@Winkeltje84 Winkeltje84 removed the LGTM1 label Mar 27, 2017
@Winkeltje84
Copy link
Owner Author

@arnoFleming , it should be better now.

@Winkeltje84
Copy link
Owner Author

please label me @leefreemanxyz !

expect(guest_user.admin).to eq(false)
end

it "the 'Admin' status is false" do
it "if 'Admin' status is set to true by another Admin,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we could describe this test as : 'it "creates an admin user when admin: true is included in the create function" '

Copy link
Owner Author

Choose a reason for hiding this comment

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

@lee, changed it according to your suggestion

Copy link
Collaborator

@leefreemanxyz leefreemanxyz left a comment

Choose a reason for hiding this comment

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

Goed.

Copy link
Contributor

@arnoFleming arnoFleming left a comment

Choose a reason for hiding this comment

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

🚢 it

@Winkeltje84 Winkeltje84 merged commit 87157a4 into master Mar 28, 2017
@leefreemanxyz leefreemanxyz deleted the device branch March 28, 2017 15:52
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.

None yet

3 participants