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

Convert member role to learner role. Add default phase to new users who are learners. #1083

Merged

Conversation

sdweber422
Copy link
Contributor

@sdweber422 sdweber422 commented Aug 18, 2017

Fixes #1079.
Fixes #1080.

Overview

Converted roles where 'member' existed and replaced it with 'learner'. Added default phase when creating a new user who has the learner role in userCreated worker.

Data Model / DB Schema Changes

User model roles changed from 'member' to 'learner'.

Environment / Configuration Changes

None.

Notes

Copy link
Collaborator

@heyheyjp heyheyjp left a comment

Choose a reason for hiding this comment

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

Please see comments; minor changes requested.


expect(member).to.exist
expect(member.chapterId).to.eq(this.chapter.id, 'member should have chapter ID for invite code')
// TODO: fix - should assert github service function called w/ correct args
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please do add at least one assertion about a github service function being called.

if (idmUser.roles.includes(LEARNER)) {
const defaultPhase = (await Phase.getAll(DEFAULT_PHASE_NUMBER, {index: 'number'}))[0]
if (!defaultPhase) {
throw new Error('Phase not found for default numberL', DEFAULT_PHASE_NUMBER)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo: defaut numberL

@sdweber422 sdweber422 force-pushed the 1079-convert-member-to-learner branch from 235bf6a to cf7f541 Compare August 28, 2017 20:41
Copy link
Collaborator

@heyheyjp heyheyjp left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@heyheyjp heyheyjp merged commit 5fdab51 into LearnersGuild:master Aug 30, 2017
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.

3 participants