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

Add support for AdminRequest Sessions #25

Merged
merged 4 commits into from
Nov 15, 2023
Merged

Conversation

amcalgates
Copy link
Contributor

Summary

Adds support for AdminRequest Begin/End session as seen in Create a session documentation.

All added classes/additions mirror the naming found in that documentation, however I had to introduce a new class which I named SessionContainer to combine the Session and [Operation] objects for JSON serialization, which can then be base64 encoded to a string and used for AdminRequest.ServiceIdentification

Please let me know if you'd prefer a different name for SessionContainer - I couldn't find a similar concept in adyen-terminal-api-ios to base the naming off of.

Tested scenarios

I've tested this with both Begin and End session calls using an AMS1 device.

@kaphacius
Copy link
Contributor

Hi @amcalgates,

Thank you so much for taking the time to contribute to our repository.
We will check your request and get back to you soon.

Regards,
Yurii
Adyen

Sources/TerminalAPIKit/Models/Operation.swift Outdated Show resolved Hide resolved
Sources/TerminalAPIKit/Models/Operation.swift Outdated Show resolved Hide resolved
Sources/TerminalAPIKit/Models/Operation.swift Outdated Show resolved Hide resolved
Sources/TerminalAPIKit/Models/Operation.swift Outdated Show resolved Hide resolved
Sources/TerminalAPIKit/Models/Operation.swift Outdated Show resolved Hide resolved
Sources/TerminalAPIKit/Models/Session.swift Outdated Show resolved Hide resolved
Sources/TerminalAPIKit/Models/SessionContainer.swift Outdated Show resolved Hide resolved
- Add `OperationVariant` enum
- Add comment stubs for inits to match rest of code base
- Fix spacing in variable declarations
- Capitalize first word in comments
@amcalgates
Copy link
Contributor Author

I've made all requested changes, except for one comment I was unsure on (see my inline replies).

Copy link
Contributor

@kaphacius kaphacius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the PR. I made a few small comments.
Could you also make sure the headers in every file have the following format

//
//  {FILENAME}.swift
//  TerminalAPIKit
//
//  Copyright (c) 2023 Adyen N.V.
//

Sources/TerminalAPIKit/Models/Session.swift Outdated Show resolved Hide resolved
Sources/TerminalAPIKit/Models/Operation.swift Outdated Show resolved Hide resolved
Sources/TerminalAPIKit/Models/Operation.swift Show resolved Hide resolved
Sources/TerminalAPIKit/Models/Operation.swift Outdated Show resolved Hide resolved
@amcalgates
Copy link
Contributor Author

I've made most of the requested changes, and left comments in areas I need more clarification on.

@amcalgates
Copy link
Contributor Author

@kaphacius - any follow-up on my comments?

@kaphacius
Copy link
Contributor

hey @amcalgates
sorry for not getting back on this, things were kind of busy.
i'll approve and merge your MR. thanks for contributing!

@kaphacius kaphacius merged commit 022131c into Adyen:develop Nov 15, 2023
1 check passed
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.

2 participants