Skip to content

Conversation

@dmitchell
Copy link
Contributor

Ensure user admin screen gets the union of all possibly matching group names.
Smarter default group naming.
STUD-1003

@singingwolfboy @cahrens One of you please review
@zubair-arbi @adampalay Also, one or both of you.

I have no way to really reproduce the bug, but I believe test_permissions does.

@zubair-arbi
Copy link
Contributor

All functions properly working and documented, modified functions not breaking any other code. 👍

@dmitchell
Copy link
Contributor Author

@chrisndodge Can you review this. My timing seems to have sucked and only zubair's looked at it.

Copy link
Contributor

Choose a reason for hiding this comment

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

How does this new group name formatting map to the roles in the LMS? If it uses the new 'staff_myu.mycourse.myrun' format then are we making similar edits in the auth in the LMS, for example is a Studio course author treated as a Course Staff member in the LMS?

Maybe @cpennington or @ormsbee has some insight.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked the lms code and its auth is completely separate from cms's. I also tried it locally to ensure it works.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to not have those roles separate, given that we expect them to actually be the same (i.e. staff in LMS should also be staff in Studio and vice versa). I recently added code to the LMS side of things to separate the notion of what the roles are (CourseStaff, CourseInstructor) from how they are implemented (the specifics of what the group names are, and which one takes priority when adding a new user, and so on).

Would it be possible to switch Studio over to the new roles code, and then make this change there?

Copy link

Choose a reason for hiding this comment

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

A story was added to the Studio backlog for this. https://edx-wiki.atlassian.net/browse/STUD-1006

@singingwolfboy
Copy link
Contributor

👍

Ensure user admin screen gets the union of all possibly matching group names.
Smarter default group naming.
STUD-1003
dmitchell added a commit that referenced this pull request Dec 2, 2013
Improve auth handling of Locators
@dmitchell dmitchell merged commit 7fa7641 into master Dec 2, 2013
@jzoldak jzoldak deleted the dhm/bug-1003 branch May 5, 2014 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.

7 participants