Skip to content

Conversation

@gwprice
Copy link

@gwprice gwprice commented Nov 1, 2013

The motivation for this change is performance. The forums UI code gets
the list of users for each role and renders the staff label based on
those lists. The list for the staff role is expensive to compute because
there is no index on the is_staff attribute, and we cannot create one
because the User model is built into django.

Users with is_staff=True are still assigned the Moderator role upon
enrolling in a course, so this change will have no practical effect
except that a user who is granted staff privileges after enrolling in a
course will have to be made a Moderator in order for their posts to be
labeled.

Additionally, the UI did not use the list of users with the Student
role, so that list has been removed as well.

@jimabramson

The motivation for this change is performance. The forums UI code gets
the list of users for each role and renders the staff label based on
those lists. The list for the staff role is expensive to compute because
there is no index on the is_staff attribute, and we cannot create one
because the User model is built into django.

Users with is_staff=True are still assigned the Moderator role upon
enrolling in a course, so this change will have no practical effect
except that a user who is granted staff privileges after enrolling in a
course will have to be made a Moderator in order for their posts to be
labeled.

Additionally, the UI did not use the list of users with the Student
role, so that list has been removed as well.
@jimabramson
Copy link

does the addition of the Community TA role fix a bug, or is this an enhancement?

@gwprice
Copy link
Author

gwprice commented Nov 1, 2013

Neither; I just wanted the test to be more substantial than a single role with a single user. The fact that it now mirrors the actual roles we use is nice but not necessary.

@jimabramson
Copy link

got it, thanks. 👍

gwprice pushed a commit that referenced this pull request Nov 1, 2013
Remove label from forum posts by global staff
@gwprice gwprice merged commit 2123d5e into master Nov 1, 2013
@gwprice gwprice deleted the gprice/forum-staff-label branch November 1, 2013 20:10
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Dec 21, 2016
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