Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Admin users require role #858

Merged
merged 2 commits into from
Aug 30, 2015
Merged

Conversation

mleanos
Copy link
Member

@mleanos mleanos commented Aug 29, 2015

These changes make the role field required in the User model. Changes to
the Admin user edit view were added to provide validation for the role
field.

As an added enhancement, the user's roles are displayed in the Admin
user list view.

Added model tests to accommodate these changes.

These changes make the role field required in the User model. Changes to
the Admin user edit view were added to provide validation for the role
field.

As an added enhancement, the user's roles are displayed in the Admin
user list view.
Added tests for the User model's roles field.

Should be able to update existing user with valid roles
Should NOT be able to update existing user WITHOUT a role
Should NOT be able to update existing user with INVALID role
@codydaig
Copy link
Member

LGTM

@lirantal
Copy link
Member

Looks good. I want to merge my additions to the user model tests for and then you can probably rebase again for the conflicts

@lirantal lirantal added this to the 0.4.x milestone Aug 29, 2015
@lirantal lirantal self-assigned this Aug 29, 2015
@mleanos
Copy link
Member Author

mleanos commented Aug 29, 2015

@lirantal Sounds like a plan. I figured I would just adjust the tests according to your changes. Once you merge, I'll rebase and update these new tests. Thanks.

Will rebase & squash after #840 is merged.

@lirantal
Copy link
Member

Looks like we're good here to merge.

lirantal added a commit that referenced this pull request Aug 30, 2015
@lirantal lirantal merged commit 8335aa7 into meanjs:master Aug 30, 2015
mleanos added a commit to mleanos/mean that referenced this pull request Aug 30, 2015
PR meanjs#840 changed the global var `user` to `user1`. This was merged and
then meanjs#858 was merged, which was still referencing the global var as
`user` in the new *roles* tests. This was causing jshint failures from
the new

This change updates the new *roles* tests to use `user1`
@mleanos mleanos deleted the admin-users-require-role branch October 1, 2015 03:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants