-
Notifications
You must be signed in to change notification settings - Fork 11
MEI-9862 return users main organizations in user and course api #332
base: master
Are you sure you want to change the base?
Conversation
| return [] | ||
|
|
||
| return user.organizations.all() | ||
| main_user_organization = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 1469-1502 seems duplicate code(used in serializer as well).
can we remove this duplication as both are related with each other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed
| user_organizations_set = user.organizations.all() | ||
| if main_user_organization: | ||
| user_organizations_set = user_organizations_set.exclude(id=main_user_organization.id) | ||
| user_organizations_set = [main_user_organization] + list(user_organizations_set) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are excluding main_user_organization from user_organizations_set and then re-adding it is there a reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
user.organizations.all().order_by('-user_organizations__is_main_company')
May be we can use some thing similar to this not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose of re-adding was to set the main company at first index. However, updated it with the suggested one.
| user_organizations_set = user.organizations.all() | ||
| if main_user_organization: | ||
| user_organizations_set = user_organizations_set.exclude(id=main_user_organization.id) | ||
| user_organizations_set = [main_user_organization] + list(user_organizations_set) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
user.organizations.all().order_by('-user_organizations__is_main_company')
May be we can use some thing similar to this not sure.
This PR implements the code base changes to return user main(or actual) organization in user and course api.