Skip to content

Document disposable email domain analytics parameter#9912

Merged
aduth merged 1 commit intomainfrom
aduth-document-disposable-event-param
Jan 22, 2024
Merged

Document disposable email domain analytics parameter#9912
aduth merged 1 commit intomainfrom
aduth-document-disposable-event-param

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jan 11, 2024

🛠 Summary of changes

Updates user_registration_complete method signature to include the disposable_email_domain argument, which may be passed as a result of this code added in #9747:

if page_occurence.present? && DisposableEmailDomain.disposable?(email_domain)
attributes[:disposable_email_domain] = email_domain
end

Why?

  • So that Analytics Events documentation accurately reflects the available properties
  • To enable discoverability based on searches performed within the analytics_events.rb (e.g. this was motivated by my own pursuit to jog my memory about which event this was logged on)

📜 Testing Plan

After merged, observe the parameter is visible in Analytics Events documentation

Build should pass

@aduth aduth requested a review from a team January 11, 2024 20:51
kevinsmaster5
kevinsmaster5 approved these changes Jan 12, 2024
@kevinsmaster5 kevinsmaster5 self-requested a review January 12, 2024 14:41
Copy link
Contributor

@kevinsmaster5 kevinsmaster5 left a comment

Choose a reason for hiding this comment

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

Looks like the spec needs to be revised to include added value.
I see, the spec will be done with #9747:

changelog: Internal, Documentation, Add missing parameter to user registration analytics event
@aduth aduth force-pushed the aduth-document-disposable-event-param branch from 1ded309 to d1c00e0 Compare January 22, 2024 15:23
@aduth aduth merged commit f28dc59 into main Jan 22, 2024
@aduth aduth deleted the aduth-document-disposable-event-param branch January 22, 2024 15:52
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.

3 participants