Canvas Course Manager Next: A redesign of the existing CCM application. It extends Canvas features, makes cumbersome features easier to use, and adds new features.
8
stars
9
forks
source link
Consider extracting user-related API operations from `AdminApiHandler` #380
I'm not entirely sure this is necessary, but there is some awkwardness with the current implementation that I wanted to document.
AdminApiHandler has some constructor parameters and instance variables that are not used by all methods. For instance, userLoginId is not used by AdminApiHandler.createExternalUsers, but is by AdminApiHandler.getParentAccounts. This leads to a situation where we have to provide defaults even when they are not used. This is likely best resolved by extracting some of the methods into a separate class, although there shouldn't be an issue with the current implementation so long as methods are used properly (i.e. defaults aren't relied upon when they should be set). Alternatively, constructor parameters could become method parameters.
I originally envisioned the handler organization as being one class per Canvas API root (i.e. courses, sections, accounts), but the inclusion of AdminApiHandler.getUserInfo somewhat violates that (users root). However, createExternalUser does use accounts, and I'd probably try to keep createExternalUser with getUserInfo since they are related conceptually. I could see moving both to a new UserApiHandler class, but I'm open to other ideas.
I'm not entirely sure this is necessary, but there is some awkwardness with the current implementation that I wanted to document.
AdminApiHandlerhas some constructor parameters and instance variables that are not used by all methods. For instance,userLoginIdis not used byAdminApiHandler.createExternalUsers, but is byAdminApiHandler.getParentAccounts. This leads to a situation where we have to provide defaults even when they are not used. This is likely best resolved by extracting some of the methods into a separate class, although there shouldn't be an issue with the current implementation so long as methods are used properly (i.e. defaults aren't relied upon when they should be set). Alternatively, constructor parameters could become method parameters.I originally envisioned the handler organization as being one class per Canvas API root (i.e.
courses,sections,accounts), but the inclusion ofAdminApiHandler.getUserInfosomewhat violates that (usersroot). However,createExternalUserdoes useaccounts, and I'd probably try to keepcreateExternalUserwithgetUserInfosince they are related conceptually. I could see moving both to a newUserApiHandlerclass, but I'm open to other ideas.