Repository navigation
fix(API): make clearable string fields on update endpoints nullable #STRINGS-3652 - #1326
Open
Mikko Karkee (Varpuspaavi) wants to merge 2 commits into
Open
Mikko Karkee (Varpuspaavi) wants to merge 2 commits into
Mikko Karkee (Varpuspaavi) wants to merge 2 commits into
Conversation
…STRINGS-3652 Plain optional strings generate Go fields with omitempty, so sending an empty string through the generated clients/CLI is dropped before the request is sent. Mark free-text update fields nullable so the Go client generates *NullableString and preserves an empty string on the wire. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Contributor
API changelog (oasdiff)Doc-only edits (descriptions, examples) do not appear here. |
… #STRINGS-3652 webhook secret (ignored when blank), authorization note and job/job locale annotation value (presence validated), variable value (unconfirmed body wrapping) and release_triggers app versions (omitting already clears). Document that an empty string or null clears the remaining fields. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
This branch has not been deployed
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
1 participant
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.
Problem
Follow-up to #1325 (STRINGS-3652). Optional
type: stringfields on update endpoints generate Go struct fields withjson:",omitempty". Sending""through the generated Go client or the CLI (--data) is dropped before the request is sent, so clearing a value silently no-ops.Change
Add
nullable: true(so the Go client generates*NullableString) to free-text fields that the API lets callers clear, and document that an empty string ornullclears the value, omitting leaves it unchanged, and the CLI dropsnull(use an empty string there).key/updatename_plural,original_file,localized_format_string,localized_format_keyscreenshot/update,glossary_term/update,custom_metadata_property/update,webhook/updatedescriptionjob/updatebriefing,ticket_urljob_template/update,organization_job_template/updatebriefingrelease/updatedescription,app_min_version,app_max_versionproject/updatepoint_of_contactlocale/updatelanguage_ai_profile,fallback_locale_idstyleguide/updateaudience,business,company_branding,formatting,glossary_terms,grammar_consistency,literal_translation,overall_tone,sampleskey/updatedescriptionand translationcontentare handled in #1325.Deliberately not changed
An earlier revision of this PR also marked the fields below. Checking the API code showed
nullableis wrong or pointless for them:webhook/updatesecret:nulland""are both ignored on update (the existing secret is kept), so it can't be cleared this way. The "default project secret will be used" text applies at create only (not changed here).authorization/updatenote:nulland""both return 422 (presence validation).job_annotation/updateandjob_locale_annotation/updatevalue:nulland""both return 422 (presence validation, NOT NULL column).variable/updatevalue: the API would storenull/"", but the controller readsparams.require(:variable). Whether a flat JSON body is auto-wrapped is unconfirmed, so left out until checked.release_triggers/updateapp_min_version,app_max_version: the update rebuilds the template, so omitting these already clears them; there is noomitemptyproblem to fix.API behavior (from reading the API code, not from running requests)
For the fields in the table,
nullstoresNULLand""stores an empty string, with no validation failure. Exceptions that normalize:release/updateapp_min_version/app_max_version:""is stripped toNULL; non-blank values are semver-checked (422 if invalid).locale/updatefallback_locale_idandproject/updatepoint_of_contact: bothnulland""clear the value.Not confirmed: whether any code treats
""specially for keyoptionsvalues (original_file,localized_format_*) orlanguage_ai_profile.Verification
make lintpasses. Other language generators and the Go client were not regenerated for this revision.Client impact (source-breaking for typed clients)
Same as #1325, on 27 fields across 12 endpoints: in
phrase-gothese change fromstringto*NullableString, so direct users must wrap values withphrase.NewNullableString(&s), andNewNullableString(nil), a zero-value&NullableString{}orUnset()now marshal as JSONnull, which clears the field. Other typed clients get nullable field types too (not inspected). phrase-cli built, vetted and passed tests against the client generated for #1325 because it unmarshals--datainto the struct; not re-checked for this PR.The PR title is
fix(API), which release-please treats as a patch bump; see the release-note question on #1325 and decide both together.Known limitation
nullis still dropped by the CLI/Go client when unmarshalling--data(encoding/jsonleaves the pointernil); same as #1325. Use an empty string there.Merge note
Possible small conflict with #1325 in
paths/keys/update.yaml(adjacent lines).🤖 Generated with Claude Code