Skip to content

fix: allow string type for tables.id parameter with safe conversion#61

Draft
ZdenekSrotyr wants to merge 2 commits intomasterfrom
devin/1767373017-fix-id-type-validation
Draft

fix: allow string type for tables.id parameter with safe conversion#61
ZdenekSrotyr wants to merge 2 commits intomasterfrom
devin/1767373017-fix-id-type-validation

Conversation

@ZdenekSrotyr
Copy link
Copy Markdown

@ZdenekSrotyr ZdenekSrotyr commented Jan 2, 2026

Summary

Fixes configuration validation error where parameters.tables[].id was strictly typed as integerNode, causing failures when the UI/API sends the value as a string (e.g., "0" instead of 0).

The id field now uses scalarNode with custom validation that:

  • Accepts integer values directly
  • Converts numeric strings to integers (e.g., "123"123)
  • Throws a clear error for non-numeric values

This is consistent with how sheetId is already handled in the same configuration file.

Triggered by: Datadog alert for keboola.wr-google-sheets showing Invalid type for path "parameters.tables.0.id". Expected int, but got string.

CI Status

  • build: Passes (lint, phpcs, phpstan)
  • tests-in-kbc: Passes
  • ⚠️ tests: Fails due to expired Google OAuth credentials (unrelated to this change - 401 Unauthorized on token refresh)

Review & Testing Checklist for Human

  • IMPORTANT: The min(0) constraint was removed. Verify if negative IDs should be rejected (integer -1 would now be accepted; string "-1" is rejected by ctype_digit)
  • Verify edge cases: empty string "", float string "1.5", negative string "-1" - all should throw errors
  • Test with the actual failing configuration from project 16370 to confirm the fix resolves the issue
  • Consider refreshing Google OAuth credentials in CI secrets to fix the unrelated test failure

Recommended Test Plan

  1. Deploy to a test environment using the branch tag
  2. Run a configuration with "id": "0" (string) - should now work
  3. Run a configuration with "id": 0 (integer) - should still work
  4. Run a configuration with "id": "abc" - should fail with clear error

Notes

The id field in parameters.tables was defined as integerNode which strictly
required an integer type. However, the UI or API may send this value as a
string (e.g., "0" instead of 0), causing a validation error.

This change converts the id field to use scalarNode with validation that:
- Accepts integer values directly
- Converts numeric strings to integers (e.g., "123" -> 123)
- Throws an error for non-numeric strings to prevent silent failures

This is consistent with how sheetId is already handled in the same config.

Co-Authored-By: zdenek.srotyr@keboola.com <zdenek.srotyr@keboola.com>
@devin-ai-integration
Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Co-Authored-By: zdenek.srotyr@keboola.com <zdenek.srotyr@keboola.com>
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.

1 participant