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

CRv2: Process state attribute #35517

Merged
merged 8 commits into from Jun 30, 2020
Merged

CRv2: Process state attribute #35517

merged 8 commits into from Jun 30, 2020

Conversation

hacodeorg
Copy link
Contributor

@hacodeorg hacodeorg commented Jun 26, 2020

PLC-918: Use contact's state to set Pardot prospect's db_State.

  • Contact's state value is calculated from dashboard.schools#state, dashboard.users#state and pegasus.form_geos#state.
  • Prospect's db_State is a text attribute and should show a single value.

Links

Speadsheet with all attributes to process.

Test story

See #35508.

@hacodeorg hacodeorg changed the base branch from staging to ha/cr-process-roles June 26, 2020 19:38
@hacodeorg hacodeorg marked this pull request as ready for review June 26, 2020 22:46
def self.extract_state(contact_data)
# The priority is: school state > user state > form geo state
# US state in schools table is in abbreviation, must convert it back to state name.
school_state = extract_field_latest_value contact_data, 'dashboard.schools', 'state'
Copy link
Contributor

Choose a reason for hiding this comment

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

A user will only have one row here...why use latest value here and not in other processing steps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm trying to not use too many implicit assumptions here. We join users to school_infos then to schools to map an email (not user_id) to a school. There is a chance that an email is mapped to more than 1 school.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This query returns 59 users with more than 1 school. It's a small number, but it means our data can sometime surprise us.

SELECT t.email, count(*) as hit, count(distinct s.id) as distinct_school
FROM schools AS s
INNER JOIN school_infos AS si ON s.id = si.school_id
INNER JOIN users AS t ON si.id = t.school_info_id
WHERE email > ''
GROUP BY email
HAVING distinct_school > 1
ORDER BY distinct_school desc

@hacodeorg hacodeorg requested a review from a team June 29, 2020 17:45
Base automatically changed from ha/cr-process-roles to staging June 30, 2020 16:05
return {state: state_name}
end

user_state = extract_field_latest_value contact_data, 'dashboard.users', 'state'
Copy link
Contributor

Choose a reason for hiding this comment

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

This confused me for a while, because our User model doesn't have a column or method named state. I finally noticed that ContactRollupsRaw includes user_geos when it populates the dashboard.users source.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We think of user_geos table as an extension of users table. So instead of doing 2 separate extraction, reading user_id from users table, and reading user location from user_geos (still has to join with users to get email address), we just do one join and get both user_id and user location.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Sounds good!

@hacodeorg hacodeorg merged commit fb8d49f into staging Jun 30, 2020
@hacodeorg hacodeorg deleted the ha/cr-process-state branch June 30, 2020 16:46
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

3 participants