Add ExecuteScript feature to launch Google APIs #26
Conversation
darrarski
left a comment
darrarski
left a comment
There was a problem hiding this comment.
Looks good, thanks for your contribution! I've added some comments that should be addressed before I merge it.
I'm not familiar with Apps Script API, but I see that Google provides documentation for the response body. Please consider adding a Swift struct, like ExecutionResponse and use it as a result of ExecuteScript.Run (instead of returning Data). As an example, take a look at GetFile interface that returns File struct.
I'm afraid the current way of how this is implemented is not entirely correct, as Google API can return error info in the response body. This should be handled by the ExecuteScript and an error should thrown in such a case.
I see you followed my approach to designing dependencies without a problem. The only thing that's missing is a test case for the life implementation of ExecuteScript.
There are test cases for other endpoints already in the repository. For example, take a look at these files:
Tests/GoogleDriveClientTests/GetFileTests.swiftTests/GoogleDriveClientTests/GetAboutTests.swift- and others
I will be happy to review the tests for ExecuteScript.live once you add it. If you have any questions - feel free to ask.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new feature to execute Google Apps Script via the Google Drive API. The changes include adding a new file (ExecuteScript.swift) that defines the ExecuteScript struct and updating the Client.swift file to integrate and expose the new executeScript functionality.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Sources/GoogleDriveClient/ExecuteScript.swift | New feature implementation for executing Apps Script. |
| Sources/GoogleDriveClient/Client.swift | Updated client to integrate and expose executeScript. |
… MR darrarski#26 - removed print statements, - added unit tests, - raised exception for invalid parameters.
|
I updated my fork with a set of unit tests and tested with my swatDocuments app the modified error management (now performed partially in the ExecuteScript API). Hopefully you will receive the update in the MR. Let me know if you wan additional changes. Best regards. Alban |
2 participants
Hello Dariusz,
I used your framework for my application swatDocuments and I needed to execute a Google Apps Script in the same Google session as the Google Drive. I made a fork to GoogleDriveClient and added a new feature to execute scripts and integrated it in my fork. If you want to add it, I would be more than happy. The are some aspects of your developed solution with "dependencies" that I'm not expert in, and this is the reason why I did not add tests for it.
Have a nice day. Let me know if you have questions.
Alban