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

Users #44

Closed
wants to merge 8 commits into from
Closed

Users #44

wants to merge 8 commits into from

Conversation

scottnath
Copy link
Contributor

@Snugug not an official PR. Please offer feedback on this code so I can make changes on tuesday.

@Snugug Snugug mentioned this pull request Jun 1, 2016
id: 'admin',
allows: [
{
resources: '/content/services',
Copy link
Member

Choose a reason for hiding this comment

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

Will we need to do this for every single content type?

Copy link
Member

Choose a reason for hiding this comment

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

Can we abstract this for ease of our users? Instead of exposing ACL directly to them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Snugug There are other ways to do this, but was getting very heavy for this PR. Ok if I make a story for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@scottnath
Copy link
Contributor Author

@Snugug all PR changes are in. I've very close on the tests, but it just won't recognize the cookies.

@@ -44,18 +45,58 @@ module.exports = {
path: 'content',
label: 'Content',
},
{
path: 'users',
Copy link
Member

Choose a reason for hiding this comment

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

Please follow the pattern in content

const userRoles = config.roles.config;

// memoryBackend starts with a lowercase from node-acl
const acl = new NodeAcl(new NodeAcl.memoryBackend()); // eslint-disable-line new-cap
Copy link
Member

Choose a reason for hiding this comment

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

So, found your problem with everyone being unloaded on server crash. Using memory backend. Should use knex backend

@Snugug Snugug mentioned this pull request Jun 2, 2016
@scottnath scottnath closed this in #48 Jun 6, 2016
@scottnath scottnath deleted the users branch June 6, 2016 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants