Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for cancellation of organization invitations #2072

Merged
merged 1 commit into from Oct 13, 2021

Conversation

jsimpso
Copy link
Contributor

@jsimpso jsimpso commented Oct 11, 2021

This pull request will implement a new function cancel_invitation for github.Organization objects, allowing for cancellation of organization invitations per https://docs.github.com/en/rest/reference/orgs#cancel-an-organization-invitation

>>> pending_invitations = github_org.invitations()
>>> any([i for i in pending_invitations if i.email == 'user@example.com'])
False
>>> github_org.invite_user(email='james@snowterminal.com')
>>> pending_invitations = github_org.invitations()
>>> any([i for i in pending_invitations if i.email == 'user@example.com'])
True
>>> pending_invitation = [i for i in pending_invitations if i.email == 'user@example.com'][0]
>>> github_org.cancel_invitation(pending_invitation)
>>> pending_invitations = github_org.invitations()
>>> any([i for i in pending_invitations if i.email == 'user@example.com'])
False

@s-t-e-v-e-n-k
Copy link
Collaborator

This requires tests, and type checking -- also, maybe having it return a boolean.

@jsimpso
Copy link
Contributor Author

jsimpso commented Oct 11, 2021

This requires tests, and type checking -- also, maybe having it return a boolean.

Thanks for the feedback. I've adjusted it to return a bool in the same way as other methods in the Organization class.

Apologies I'm not 100% sure that the tests are what you're looking for, I've adapted the way that a previous PR did Organization testing. If the way I've done that isn't right let me know and I'll take another crack at it.

RE: type checking are you able to link to an example of where that's being done elsewhere? Happy to put it in but not sure what you mean other than the "assert isinstance" line already there :)

@s-t-e-v-e-n-k
Copy link
Collaborator

The test failures in AuthenticatedUser are my partially fault -- I pushed a fix a few minutes ago, please rebase.

WRT type checking, I actually mean type hints, and picking on the method before the you added, like this: https://github.com/PyGithub/PyGithub/blob/master/github/Organization.pyi#L164

As as aside, you can just assertTrue() with calling the method in the test case, no need for a result variable.

@jsimpso
Copy link
Contributor Author

jsimpso commented Oct 11, 2021

The test failures in AuthenticatedUser are my partially fault -- I pushed a fix a few minutes ago, please rebase.

WRT type checking, I actually mean type hints, and picking on the method before the you added, like this: https://github.com/PyGithub/PyGithub/blob/master/github/Organization.pyi#L164

As as aside, you can just assertTrue() with calling the method in the test case, no need for a result variable.

Thanks - have rebased and updated with the suggested changes

@s-t-e-v-e-n-k
Copy link
Collaborator

You've been caught by a sharp edge here -- your replay data has jwt, but we expect password, compare the sent headers in another replay data text file.

@jsimpso
Copy link
Contributor Author

jsimpso commented Oct 12, 2021

You've been caught by a sharp edge here -- your replay data has jwt, but we expect password, compare the sent headers in another replay data text file.

Thanks for the info, re-recorded tests under auth_with_token and replaced token private_token_removed with Basic login_and_password_removed per the contributing guide - hopefully that works!

@s-t-e-v-e-n-k
Copy link
Collaborator

More issues caused by master, sorry, rebase, and we should be good to merge.

@jsimpso
Copy link
Contributor Author

jsimpso commented Oct 13, 2021

More issues caused by master, sorry, rebase, and we should be good to merge.

Done, fingers crossed :)

@codecov-commenter
Copy link

codecov-commenter commented Oct 13, 2021

Codecov Report

Merging #2072 (f98e29b) into master (aa694f8) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2072   +/-   ##
=======================================
  Coverage   98.86%   98.87%           
=======================================
  Files         108      108           
  Lines       11086    11090    +4     
=======================================
+ Hits        10960    10965    +5     
+ Misses        126      125    -1     
Impacted Files Coverage Δ
github/Organization.py 98.79% <100.00%> (+0.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa694f8...f98e29b. Read the comment docs.

@s-t-e-v-e-n-k s-t-e-v-e-n-k merged commit 53fb498 into PyGithub:master Oct 13, 2021
@MaximKorobovAtTinkoff
Copy link

@s-t-e-v-e-n-k, when do you plan to release 1.56 with this update?
Seems like 1.55 was in April 26, 2021 and current PR was merged later. So no new functionality available when installed pip package.

@MaximKorobovAtTinkoff
Copy link

Temporary patch that works for those who need that functionality immediately:

def cancel_invitation(self, invitee):
    """
    :calls: `DELETE /orgs/{org}/invitations/{invitation_id} <https://docs.github.com/en/rest/reference/orgs#cancel-an-organization-invitation>`_
    :param invitee: :class:`github.NamedUser.NamedUser`
    :rtype: None
    """
    assert isinstance(invitee, NamedUser.NamedUser), invitee
    status, headers, data = self._requester.requestJson(
        "DELETE", f"{self.url}/invitations/{invitee.id}"
    )
    return status == 204

...

    org.cancel_invitation = types.MethodType(cancel_invitation, org)

    org.cancel_invitation(invite)

This manual was used: https://bobbyhadz.com/blog/python-add-method-to-existing-object

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.

None yet

4 participants