[MNOE-978] Refactor Org tables into one code #230
Conversation
iseessel
changed the title
iseessel
force-pushed
the
feature/mnoe-978-refactor-table-pagination
branch
from
28b9229
to
c7b0a70
Compare
| scope.organizations.list = response.data | ||
| ).finally(-> scope.organizations.loading = false) | ||
| ) | ||
| scope.customizations.getOrganizations(limit, offset, sort) |
There was a problem hiding this comment.
The sub_tenant_id and account_manager_id params in the original call are just so that the organizations come back with the attributes belong_to_sub_tenant and belong_to_account_manager set correctly. I don't see where we are using those attributes, but I'm thinking they will be used in the future if they are not used already. So, I'm not sure it's a great idea to take them out.
Keeping that part of the code intact makes a refactor a bit easier and I'm not sure that it makes sense to extract it out. I feel like it should live in this component if possible since the function of it is to display a list of organizations. Maybe something like this would work:
# Fetch organisations
fetchOrganizations = (limit, offset, sort = 'created_at') ->
scope.organizations.loading = true
MnoeCurrentUser.getUser().then( ->
params = if MnoeAdminConfig.isAccountManagerEnabled()
{sub_tenant_id: MnoeCurrentUser.user.mnoe_sub_tenant_id, account_manager_id: MnoeCurrentUser.user.id}
else
{}
params = angular.extend({}, params, scope.customizations.searchParams)
return MnoeOrganizations.list(limit, offset, sort, params).then(
(response) ->
scope.organizations.totalItems = response.headers('x-total-count')
scope.organizations.list = response.data
).finally(-> scope.organizations.loading = false)
)
| @@ -4,7 +4,9 @@ | |||
| @App.directive('mnoeOrganizationsList', ($filter, $translate, MnoeOrganizations, MnoeAdminConfig, MnoeCurrentUser) -> | |||
| restrict: 'E' | |||
| scope: { | |||
| list: '=' | |||
| list: '=', | |||
| user: '<', | |||
There was a problem hiding this comment.
Is this necessary? Might not need to attach user to scope as it isn't used in this directive.
iseessel
force-pushed
the
feature/mnoe-978-refactor-table-pagination
branch
4 times, most recently
from
b992b02
to
a928d4e
Compare
| user.isUpdatingRole = true | ||
| # The role must be set on the user for #updateUserRole. | ||
| user.role = organization.role | ||
| MnoeUsers.updateUserRole(organization, user).then( |
There was a problem hiding this comment.
Remove unused dependencies from this file - MnoeUsers, UserRoles, toastr, $translate
Can also get rid of line 15 in this file => scope.userRoles = UserRoles since this is no longer used.
iseessel
force-pushed
the
feature/mnoe-978-refactor-table-pagination
branch
2 times, most recently
from
1176d78
to
71b029b
Compare
| return MnoeOrganizations.list(limit, offset, sort, params).then( | ||
| (response) -> | ||
| scope.organizations.totalItems = response.headers('x-total-count') | ||
| scope.organizations.list = response.data | ||
| ).finally(-> scope.organizations.loading = false) | ||
| ) | ||
|
|
||
|
|
There was a problem hiding this comment.
Remove extra newline here
| @@ -1,10 +1,13 @@ | |||
| # | |||
| # Mnoe Organizations List | |||
| # | |||
| # The organization-local-list is on the user's info page and shows the organizations attached to a user, while | |||
There was a problem hiding this comment.
Update this comment as the local-list is no longer on the user page.
| @@ -3,7 +3,7 @@ | |||
| # The organization-local-list is on the user's info page and shows the organizations attached to a user, while | |||
| # the organizations-list is on the homepage and shows all the users. | |||
|
|
|||
| @App.directive('mnoeOrganizationsLocalList', ($translate, $filter, $log, toastr, UserRoles, MnoeUsers) -> | |||
| @App.directive('mnoeOrganizationsLocalList', ($filter, $log) -> | |||
| restrict: 'E' | |||
| scope: { | |||
| list: '=', | |||
There was a problem hiding this comment.
remove unused user binding
iseessel
force-pushed
the
feature/mnoe-978-refactor-table-pagination
branch
5 times, most recently
from
b8efaa9
to
de4fd0e
Compare
iseessel
force-pushed
the
feature/mnoe-978-refactor-table-pagination
branch
from
de4fd0e
to
3f3aa42
Compare
|
@adamaziz15 @iseessel This one fell under the radar, could we get this rebased and tested? |
My approach:
Abstract out the API calls to be outside the directive (given to it through scope).
Allow additional fields to be added to the table through the given scope.
Change MNO-UI elements to be able to take "bindings," which expose a set of methods for a specific row. This is needed when a specific row has some sort of functionality attached to it.
We have two organizations-tables. The mnoe-organizations-list which deals with fetching the orgs in API calls and the mnoe-organizations-local-list which merely receives a list of organizations (some other components still use the mnoe-organizations-local-list, so I was unable to remove this).