Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parent email to new contact rollups process #34541

Merged
merged 9 commits into from May 5, 2020

Conversation

bencodeorg
Copy link
Contributor

PLC-815

Extracts parent_email from the users table and adds it to the contact rollups pipeline. Note that there are no additional attributes associated with the email being extracted here, just the parent's email address itself.

Required a little of restructuring to deal with allowing a subquery to be used. Namely, I've added an optional argument for a source_name when the data isn't coming from a single dashboard table -- @hacodeorg proposed this should follow the structure schema.table.column_name.

Testing story

Ran bin/cron/build_contact_rollups_v2 successfully and manually reviewed results in each step of contact rollups process (raw, processed, final). Successful integration test and unit tests for raw and processed steps. Added some new unit testing as well.

Reviewer Checklist:

  • Tests provide adequate coverage
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@bencodeorg bencodeorg changed the title Contact rollups parent email Add parent email to new contact rollups process Apr 30, 2020
@bencodeorg bencodeorg requested review from hacodeorg and a team April 30, 2020 23:37
dashboard/app/models/contact_rollups_raw.rb Outdated Show resolved Hide resolved
dashboard/app/models/contact_rollups_raw.rb Outdated Show resolved Hide resolved
dashboard/app/models/contact_rollups_raw.rb Outdated Show resolved Hide resolved
dashboard/app/models/contact_rollups_raw.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@hacodeorg hacodeorg left a comment

Choose a reason for hiding this comment

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

Thank you Ben!

@@ -23,25 +23,45 @@ def self.truncate_table
end

def self.extract_email_preferences
query = extract_from_source_query('email_preferences', ['opt_in'], 'email')
query = get_extraction_query("#{CDO.dashboard_db_name}.email_preferences", false, ['opt_in'], 'email')
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking: what if we change the parameter order by moving source_is_query to the end (closer to source_name) and set its default value to false? This line will be shorter and read more smoothly: get extraction query for <a table> for <several columns>.

Suggested change
query = get_extraction_query("#{CDO.dashboard_db_name}.email_preferences", false, ['opt_in'], 'email')
query = get_extraction_query("#{CDO.dashboard_db_name}.email_preferences", 'email', ['opt_in'])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, much better!

@bencodeorg bencodeorg merged commit 29c6a93 into staging May 5, 2020
@bencodeorg bencodeorg deleted the contact-rollups-parent-email branch May 5, 2020 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants