Skip to content

Conversation

@karlenDimla
Copy link
Contributor

@karlenDimla karlenDimla commented Dec 19, 2025

Task/Issue URL: https://app.asana.com/1/137249556945/project/488551667048375/task/1212426814527389?focus=true

Description

Added os_version and petal to the pixel.
Removed atb from the pixel.
Added entry for pixel registry.
https://app.asana.com/1/137249556945/project/69071770703008/task/1212452022245359?focus=true

Steps to test this PR

QA-optional


Note

Adds os_version and petal to m_privacy-pro_app_subscription_active and stops including ATB for it, with registry entry and sender wiring.

  • Pixels:
    • Active subscription (m_privacy-pro_app_subscription_active):
      • Add parameters os_version (from AppBuildConfig.sdkInt) and petal (hardcoded "true").
      • Stop including ATB; keep APP_VERSION only in includedParameters.
    • Register pixel in PixelDefinitions/pixels/subscription_pixels.json5 with new params.
  • Implementation:
    • Update SubscriptionPixelSender.reportSubscriptionActive to send os_version and petal.
    • Add SubscriptionPixelParameter.OS_VERSION and PETAL constants.

Written by Cursor Bugbot for commit 0257c72. This will update automatically on new commits. Configure here.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Contributor

@lmac012 lmac012 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a couple of comments.

}
]
},
"m_privacy-pro_app_subscription_active": {
Copy link
Contributor

@lmac012 lmac012 Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is always fired as a "daily" pixel, and as such, it has the _d suffix. This is not accounted for in this definition and will most likely cause validation errors. You could add "daily_count_short" to "suffixes", but this is never fired as a "count" type of pixel, so it's probably better to just include the type suffix in the name:

Suggested change
"m_privacy-pro_app_subscription_active": {
"m_privacy-pro_app_subscription_active_d": {

Comment on lines +29 to +33
{
"key": "petal",
"description": "Signals the pixel processing pipeline",
"type": "boolean"
}
Copy link
Contributor

@lmac012 lmac012 Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the best practices here, but shouldn't we add "enum": ["true"] to indicate that this is the only possible value for the petal param? cc @nshuba

Regardless, I think it would make sense to add this parameter to params_dictionary.json, since it's not specific to this pixel and will likely get reused on other pixels as well.

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.

2 participants