refactor(cdp): rename emailAssets to messageAssets - #75930
Closed
dmarchuk wants to merge 1 commit into
Closed
Conversation
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
Contributor
Author
|
Duplicate with #75801 |
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.
Problem
The field that carries captured message assets on an invocation result is called
emailAssets, but since #74555 it also carries push notification rows. The name now says the wrong thing at every call site that touches push.This is the rename from #75801, which was reviewed and approved there. That PR was stacked on #75800 and merged into its base branch rather than into master, so the rename never reached master. #75800 is being closed because its own change turned out to be unnecessary — a test push send records no metrics, logs or assets, since the test endpoint returns them inline and discards the result. This PR carries the rename on its own.
Changes
emailAssets→messageAssetsacrossnodejs/src. Field name only, no behavior change.15 of the 17 files are byte-identical to the version reviewed in #75801. The two that differ are the push notification service and its test, and they differ only by the
isTestguard that belonged to #75800 and is deliberately not included here.How did you test this code?
emailAssetsremain anywhere in the repo (not justnodejs/src); 60 ofmessageAssets.tsc --noEmitreports no errors beyond the pre-existing@posthog/replay-anonymizermodule-resolution ones from an unbuilt local package.message_assets, allsent, with correct recipients, subjects and HTML. So the collector and the producers still agree on the field after the rename.No test added: a rename with no behavior change has no regression to catch that the type checker doesn't.
Automatic notifications
None.
Docs update
Not needed. Internal field name, no user-facing surface.
🤖 Agent context
Written with Claude Code. Reconstructed by applying the rename directly to master rather than cherry-picking #75801's commit, because that commit's diff was computed against #75800's guard and would not apply cleanly. The result was then diffed against the reviewed version file by file to confirm equivalence.