Skip to content

Commit c0cf667

Browse files
fix(google-sheets): upgrade to SDK 2.0.0, fix Lambda crash on error paths (#289)
* fix(google-sheets): upgrade to SDK 2.0.0, fix Lambda crash on error paths, add unit tests Converts all error paths from ActionResult(data={error, result: False}) to ActionError(message=...) to fix output schema validation crashes in SDK 2.0.0. Removes error/result fields from all output schemas. Adds 51 pytest unit tests. * fix(google-sheets): fix auth to use .get(), add integration test, update .env.example - Fix build_credentials to use context.auth.get("credentials", {}).get("access_token", "") - Add test_google_sheets_integration.py with GOOGLE_SHEETS_ACCESS_TOKEN skip guard - Add GOOGLE_SHEETS_ACCESS_TOKEN and GOOGLE_SHEETS_TEST_SPREADSHEET_ID to .env.example - Apply ruff format to all google-sheets files * fix(google-sheets): fix optional param access and ruff formatting * fix(google-sheets): add full integration tests for all 10 actions * chore(google-sheets): remove legacy test file replaced by test_google_sheets_integration.py * fix(google-sheets): use 'is not None' for pageSize/pageToken truthy check A pageSize of 0 (or pageToken of empty string) was previously dropped by the truthy check. Use 'is not None' so explicit falsy values still flow through to the Drive API request. * refactor(google-sheets): drop importlib boilerplate from unit tests - conftest.py now adds the integration root to sys.path (was a no-op inserting tests/ which exposed nothing useful). - Each unit test file replaces the spec_from_file_location dance with a plain 'from google_sheets import google_sheets'. - @patch targets switch from per-file unique mod names back to 'google_sheets.build'. - Delete stale tests/context.py (pre-SDK-2.0 testbed shim, no longer imported anywhere). * fix(google-sheets): assert ResultType.ACTION in destructive integration tests 'result.result is not None' is always true (it's ActionResultData on success or ActionErrorData on failure), so the destructive tests for sheets_format_range, sheets_freeze, and sheets_batch_update would pass even when the action returned an ActionError. Assert on result.type instead so the tests fail when the action errors. * chore(google-sheets): remove dead get_first_spreadsheet_id helper The helper was never called — every test inlined the sheets_list_spreadsheets call directly. It also used the deprecated asyncio.get_event_loop().run_until_complete() pattern, which would clash with pytest-asyncio's loop if it ever were invoked. * refactor(google-sheets): drop importlib boilerplate from integration tests Mirrors the unit test refactor in 3a33344 — replace the spec_from_file_location dance with 'from google_sheets import google_sheets' and remove the now-redundant inline ResultType import in TestDuplicateSpreadsheet. * fix(google-sheets): escape single quotes in owner email q-string The 'owner' input is interpolated unescaped into the Drive q-string, which would break (and could in principle inject into) the query for emails containing apostrophes. Mirror the existing escape applied to 'name_contains'. * chore(google-sheets): remove unreachable validation in sheets_batch_update The handler's 'requests must be a list of dicts' check is unreachable: the SDK validates the input schema before the handler runs and returns a VALIDATION_ERROR for non-list / non-dict-item inputs. The two existing unit tests confirm this — both still pass and assert ResultType.VALIDATION_ERROR coming from the SDK layer. * test(google-sheets): cover pageSize/pageToken, build_credentials, rich format style - TestListSpreadsheets: assert pageSize and pageToken reach drive.files().list(**params) when supplied. Closes the 1% coverage gap and locks in the 'is not None' fix from 5e9f80a. - TestBuildCredentials: direct tests for the empty-string fallback paths added to build_credentials so a regression to bracket indexing on missing 'credentials' or 'access_token' is caught. - TestFormatRange: richer style payload (background + textFormat + alignment) flows through to userEnteredFormat unchanged, and the request range merges sheetId with the gridRange. --------- Co-authored-by: Shubhank <72601061+Sagsgit@users.noreply.github.com> Co-authored-by: Kai Koenig <kai@ventego-creative.co.nz>
1 parent 99129aa commit c0cf667

11 files changed

Lines changed: 1428 additions & 510 deletions

.env.example

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@
6464
# -- Coda --
6565
# CODA_API_KEY=
6666

67+
# -- Google Sheets --
68+
# GOOGLE_SHEETS_ACCESS_TOKEN=
69+
# GOOGLE_SHEETS_TEST_SPREADSHEET_ID=
70+
6771
# -- Gong --
6872
# GONG_ACCESS_TOKEN=
6973
# GONG_API_BASE_URL=

google-sheets/config.json

Lines changed: 21 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "Google Sheets",
33
"display_name": "Google Sheets",
4-
"version": "0.1.1",
4+
"version": "2.0.0",
55
"entry_point": "google_sheets.py",
66
"description": "Read and write Google Sheets with safe, deterministic operations and basic formatting.",
77
"auth": {
@@ -36,11 +36,9 @@
3636
"type": "object",
3737
"properties": {
3838
"files": { "type": "array", "items": { "type": "object" } },
39-
"nextPageToken": { "type": "string" },
40-
"result": { "type": "boolean" },
41-
"error": { "type": "string" }
39+
"nextPageToken": { "type": "string" }
4240
},
43-
"required": ["files", "result"]
41+
"required": ["files"]
4442
}
4543
},
4644
"sheets_get_spreadsheet": {
@@ -60,11 +58,9 @@
6058
"output_schema": {
6159
"type": "object",
6260
"properties": {
63-
"spreadsheet": { "type": "object" },
64-
"result": { "type": "boolean" },
65-
"error": { "type": "string" }
61+
"spreadsheet": { "type": "object" }
6662
},
67-
"required": ["spreadsheet", "result"]
63+
"required": ["spreadsheet"]
6864
}
6965
},
7066
"sheets_list_sheets": {
@@ -80,11 +76,9 @@
8076
"output_schema": {
8177
"type": "object",
8278
"properties": {
83-
"sheets": { "type": "array", "items": { "type": "object" } },
84-
"result": { "type": "boolean" },
85-
"error": { "type": "string" }
79+
"sheets": { "type": "array", "items": { "type": "object" } }
8680
},
87-
"required": ["sheets", "result"]
81+
"required": ["sheets"]
8882
}
8983
},
9084
"sheets_read_range": {
@@ -119,11 +113,9 @@
119113
"type": "array",
120114
"items": { "type": ["string", "number", "boolean", "null"] }
121115
}
122-
},
123-
"result": { "type": "boolean" },
124-
"error": { "type": "string" }
116+
}
125117
},
126-
"required": ["range", "values", "result"]
118+
"required": ["range", "values"]
127119
}
128120
},
129121
"sheets_write_range": {
@@ -153,11 +145,8 @@
153145
"updatedRows": { "type": "integer" },
154146
"updatedColumns": { "type": "integer" },
155147
"updatedCells": { "type": "integer" },
156-
"dryRun": { "type": "boolean" },
157-
"result": { "type": "boolean" },
158-
"error": { "type": "string" }
159-
},
160-
"required": ["result"]
148+
"dryRun": { "type": "boolean" }
149+
}
161150
}
162151
},
163152
"sheets_append_rows": {
@@ -185,11 +174,8 @@
185174
"output_schema": {
186175
"type": "object",
187176
"properties": {
188-
"updates": { "type": "object" },
189-
"result": { "type": "boolean" },
190-
"error": { "type": "string" }
191-
},
192-
"required": ["result"]
177+
"updates": { "type": "object" }
178+
}
193179
}
194180
},
195181
"sheets_format_range": {
@@ -220,11 +206,8 @@
220206
"output_schema": {
221207
"type": "object",
222208
"properties": {
223-
"replies": { "type": "array", "items": { "type": "object" } },
224-
"result": { "type": "boolean" },
225-
"error": { "type": "string" }
226-
},
227-
"required": ["result"]
209+
"replies": { "type": "array", "items": { "type": "object" } }
210+
}
228211
}
229212
},
230213
"sheets_freeze": {
@@ -243,11 +226,8 @@
243226
"output_schema": {
244227
"type": "object",
245228
"properties": {
246-
"replies": { "type": "array", "items": { "type": "object" } },
247-
"result": { "type": "boolean" },
248-
"error": { "type": "string" }
249-
},
250-
"required": ["result"]
229+
"replies": { "type": "array", "items": { "type": "object" } }
230+
}
251231
}
252232
},
253233
"sheets_batch_update": {
@@ -266,11 +246,8 @@
266246
"type": "object",
267247
"properties": {
268248
"replies": { "type": "array", "items": { "type": "object" } },
269-
"dryRun": { "type": "boolean" },
270-
"result": { "type": "boolean" },
271-
"error": { "type": "string" }
272-
},
273-
"required": ["result"]
249+
"dryRun": { "type": "boolean" }
250+
}
274251
}
275252
},
276253
"sheets_duplicate_spreadsheet": {
@@ -306,11 +283,9 @@
306283
"parents": { "type": "array", "items": { "type": "string" } },
307284
"webViewLink": { "type": "string" }
308285
}
309-
},
310-
"result": { "type": "boolean" },
311-
"error": { "type": "string" }
286+
}
312287
},
313-
"required": ["file_metadata", "result"]
288+
"required": ["file_metadata"]
314289
}
315290
}
316291
}

0 commit comments

Comments
 (0)