Conversation
Contributor
Does the PR have any schema changes?Found 1 breaking change: Resources
|
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes two issues with the RegisteredScript resource: the 404 error when attempting to update and version drift on every run. The changes correctly implement that all RegisteredScript property changes now trigger replacement (delete + recreate) instead of attempting an in-place update, since the Webflow API doesn't support PATCH for registered scripts.
Changes:
- Made
versionfield required (removing the optional workaround that caused drift) - Changed all property diffs in Diff method to use
UpdateReplaceinstead ofUpdate - Added safety net error in Update method explaining that updates are not supported
- Added deprecation comment to PatchRegisteredScript function
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| provider/registeredscript_resource.go | Changed Diff to mark all fields as UpdateReplace; simplified Update to return error; removed optional tag from version field |
| provider/registeredscript.go | Added deprecation comment to PatchRegisteredScript function |
| provider/registeredscript_resource_test.go | Added comprehensive tests for replacement behavior and Update error handling |
| provider/cmd/pulumi-resource-webflow/schema.json | Added version to required fields list |
| sdk/python/pulumi_webflow/registered_script.py | Made version required, reordered parameters, added validation check |
| sdk/nodejs/registeredScript.ts | Made version required, added validation check, updated type signature |
| sdk/go/webflow/registeredScript.go | Changed Version from StringPtrOutput to StringOutput, added validation |
| sdk/dotnet/Webflow/RegisteredScript.cs | Made version required, updated type from nullable to non-nullable |
| sdk/java/.../RegisteredScript.java | Made version required, updated return type and removed Optional wrapper |
| sdk/java/.../RegisteredScriptArgs.java | Made version required, updated builder and validation |
Comment on lines
+688
to
+715
| { | ||
| name: "hostedLocation change", | ||
| modifyFn: func(args *RegisteredScriptResourceArgs) { | ||
| args.HostedLocation = "https://cdn.example.com/script-v2.js" | ||
| }, | ||
| fieldName: "hostedLocation", | ||
| }, | ||
| { | ||
| name: "integrityHash change", | ||
| modifyFn: func(args *RegisteredScriptResourceArgs) { | ||
| args.IntegrityHash = "sha384-def456" | ||
| }, | ||
| fieldName: "integrityHash", | ||
| }, | ||
| { | ||
| name: "version change", | ||
| modifyFn: func(args *RegisteredScriptResourceArgs) { | ||
| args.Version = "2.0.0" | ||
| }, | ||
| fieldName: "version", | ||
| }, | ||
| { | ||
| name: "canCopy change", | ||
| modifyFn: func(args *RegisteredScriptResourceArgs) { | ||
| args.CanCopy = true | ||
| }, | ||
| fieldName: "canCopy", | ||
| }, |
There was a problem hiding this comment.
The test cases for replacement behavior should include test cases for siteId and displayName changes, since the Diff method also marks these fields as UpdateReplace. Adding these test cases would provide complete coverage of all fields that trigger replacement.
… update Webflow API does not support PATCH for registered scripts, which caused 404 errors when Pulumi tried to update them. This change: - Changes all property diffs to use UpdateReplace instead of Update - Makes version field required (removes optional workaround) - Updates Update method to return error as safety net - Adds deprecation comment to PatchRegisteredScript function - Adds tests for replacement behavior and Update error Fixes issues #1 and #4 from ISSUES-TO-FIX.md. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
JDetmar
force-pushed
the
fix/registeredscript-update-and-version-diff
branch
from
7101da5 to
3abb0aa
Compare
2 tasks
JDetmar
added a commit
that referenced
this pull request
Jan 13, 2026
All 6 issues have been fixed: 1. RegisteredScript Update Returns 404 - PR #51 2. SiteCustomCode Script ID Format - Resolved 3. getTokenInfo/getAuthorizedUser Invoke Crash - PR #54 4. RegisteredScript Version Diff - PR #51 5. Asset Variants Parsing Error - PR #53 6. CollectionItem Slug Uniqueness - PR #49 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
JDetmar
added a commit
that referenced
this pull request
Jan 13, 2026
#54) * fix(invoke): prevent crash in getTokenInfo/getAuthorizedUser functions The invoke functions were crashing with "grpc: the client connection is closing" because infer.GetConfig() panics (not returns nil) when provider config is not available in the context. This can happen for invoke functions called before Configure() completes. Changes: - Add safeGetConfigToken() helper with recover() to catch GetConfig panics - Refactor GetHTTPClient() to check env var first (safe), then config - Add 7 unit tests covering the fix and edge cases Fixes issue #3 in ISSUES-TO-FIX.md. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore: remove ISSUES-TO-FIX.md - all issues resolved All 6 issues have been fixed: 1. RegisteredScript Update Returns 404 - PR #51 2. SiteCustomCode Script ID Format - Resolved 3. getTokenInfo/getAuthorizedUser Invoke Crash - PR #54 4. RegisteredScript Version Diff - PR #51 5. Asset Variants Parsing Error - PR #53 6. CollectionItem Slug Uniqueness - PR #49 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(tests): address PR review feedback - Remove TestSafeGetConfigToken_NilContext (edge case without value) - Rename TestGetHTTPClient_EnvVarTakesPrecedence to TestGetHTTPClient_EnvVarWorksWithoutConfig (more accurate name) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UpdateReplacesince Webflow API has no PATCH endpointversionfield required (removes optional workaround that caused drift issues)Test plan
make test_provider)make codegen)🤖 Generated with Claude Code