277 update group feature #279
Conversation
There was a problem hiding this comment.
Looks pretty good -- can you update the tests as well? A mocked PUT like we do for other updates should be OK.
We'll want @d45 or @aaroncarey to take a look at the doc updates, I think we need more text explaining that for a 'local' group this is a traditional update, but for an AD group it triggers a synchronization job.
| @@ -55,6 +55,16 @@ def delete(self, group_id): | |||
| self.delete_request(url) | |||
| logger.info('Deleted single group (ID: {0})'.format(group_id)) | |||
|
|
|||
| @api(version="2.0") | |||
| def update(self, group_item, default_site_role=UserItem.Roles.Unlicensed): | |||
There was a problem hiding this comment.
Pull UserItem.Roles.Unlicensed into a constant UNLICENSED_USER or something a little shorter, it helps this read a little easier for me
|
There's a linter error in the tests, but it looks good otherwise, once it's passing checks I'll approve. I'll ping the team to have another set of eyes take a look as well. |
|
@t8y8, what do you think about merge this PR? |
|
I will merge this PR, but we realized that docs changes need to go to the We'll be removing the docs from this branch to make it less confusing in the future :) |
Adding support for update method for groups.