feat(user-management): add invitation accept endpoint#448
feat(user-management): add invitation accept endpoint#448gjtorikian merged 1 commit intoworkos:mainfrom
Conversation
c842f05 to
438502a
Compare
Greptile SummaryThis PR adds an Implementation ( Tests ( No security, logic, or syntax concerns with the implementation itself. Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant UserManagement as WorkOS::UserManagement
participant API as WorkOS API
Caller->>UserManagement: accept_invitation(id:)
UserManagement->>UserManagement: post_request(path: /user_management/invitations/{id}/accept, auth: true)
UserManagement->>API: POST /user_management/invitations/{id}/accept
API-->>UserManagement: 200 OK (Invitation JSON)
UserManagement->>UserManagement: WorkOS::Invitation.new(response.body)
UserManagement-->>Caller: WorkOS::Invitation
Last reviewed commit: 438502a |
| describe '.accept_invitation' do | ||
| context 'with a valid id' do | ||
| it 'accepts invitation' do | ||
| expect(described_class).to receive(:post_request) do |options| | ||
| expect(options[:path]).to eq('/user_management/invitations/invitation_123/accept') | ||
| expect(options[:auth]).to be true | ||
|
|
||
| double('request') | ||
| end.and_return(double('request')) | ||
|
|
||
| response_body = { | ||
| id: 'invitation_123', | ||
| email: 'test@workos.com', | ||
| state: 'accepted', | ||
| }.to_json | ||
|
|
||
| expect(described_class).to receive(:execute_request).and_return( | ||
| double('response', body: response_body), | ||
| ) | ||
|
|
||
| invitation = described_class.accept_invitation( | ||
| id: 'invitation_123', | ||
| ) | ||
|
|
||
| expect(invitation.id).to eq('invitation_123') | ||
| expect(invitation.email).to eq('test@workos.com') | ||
| expect(invitation.state).to eq('accepted') | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Unlike the comparable revoke_invitation and resend_invitation specs, there is no test context for failure scenarios (e.g. invitation not found, invitation already accepted). Both of those methods include error-case contexts ('with an invalid payload' for revoke_invitation at line 1776, and 'with an invalid id' / 'when invitation has expired' / etc. for resend_invitation starting at line 1806) that exercise the error path via VCR cassette.
Even if the VCR cassette is blocked for now, a context 'with an invalid id' block using the same mock-based approach would be straightforward to add and improve coverage consistency.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
gjtorikian
left a comment
There was a problem hiding this comment.
As noted by Greptile, the error-case test coverage is missing for accept_invitation. Here's a suggestion to add tests for invalid IDs and already-accepted invitations, matching the pattern used by revoke_invitation and resend_invitation.
| end | ||
| end |
There was a problem hiding this comment.
| end | |
| end | |
| end | |
| context 'with an invalid id' do | |
| it 'returns an error' do | |
| expect(described_class).to receive(:post_request).and_return(double('request')) | |
| expect(described_class).to receive(:execute_request).and_raise( | |
| WorkOS::NotFoundError.new(message: 'Invitation not found'), | |
| ) | |
| expect do | |
| described_class.accept_invitation(id: 'invalid_id') | |
| end.to raise_error( | |
| WorkOS::NotFoundError, | |
| /Invitation not found/, | |
| ) | |
| end | |
| end | |
| context 'when invitation has already been accepted' do | |
| it 'returns an error' do | |
| expect(described_class).to receive(:post_request).and_return(double('request')) | |
| expect(described_class).to receive(:execute_request).and_raise( | |
| WorkOS::InvalidRequestError.new(message: 'Invite has already been accepted'), | |
| ) | |
| expect do | |
| described_class.accept_invitation(id: 'invitation_123') | |
| end.to raise_error( | |
| WorkOS::InvalidRequestError, | |
| /Invite has already been accepted/, | |
| ) | |
| end | |
| end | |
| end |
438502a to
63c0f76
Compare
63c0f76 to
94be40b
Compare
|
Thanks for the feedback @gjtorikian. I've added those specs. |
|
thanks! |
Summary
WorkOS::UserManagement.accept_invitation(id:)forPOST /user_management/invitations/:id/acceptWorkOS::Invitation, consistent with existing invitation helpers