-
Notifications
You must be signed in to change notification settings - Fork 2
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
Id mainline updates #16
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Mike. I only really had queries about a few little things. I skimmed through the unit tests, but I assume if they're passing OK then it's most likely all good!
@@ -10,11 +10,11 @@ def nationbuilder_id | |||
end | |||
|
|||
def phone | |||
strip_country_code(@object.landline) | |||
strip_country_code(@object.phone_numbers.mobile.first) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oopsies. This looks to be the wrong field, but I think you picked it up in a later commit.
end | ||
|
||
def mobile | ||
strip_country_code(@object.mobile) | ||
strip_country_code(@object.phone_numbers.landline.first) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
@@ -34,7 +34,7 @@ def self.push_in_batches(sync_id, members, external_system_params) | |||
if sync_type === 'tag' | |||
member_ids.each do |member_id| | |||
member = Member.find(member_id[:identity_id]) | |||
member.update_external_id(SYSTEM_NAME, member_id[:nationbuilder_id], {sync_id: sync_id}) if member | |||
member.update_external_id(SYSTEM_NAME, member_id[:nationbuilder_id]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't we still need an "if member" here? Or member&.update_external_id(...)?
5652204
to
5a54104
Compare
Assume a default id setting exists (this should be set via a corresponding `NATION_BUILDER_DEFAULT_EVENT_CAMPAIGN_ID` setting) and set that on new events if needed.
5a54104
to
0ae3d28
Compare
0ae3d28
to
abc6f8c
Compare
Bring the engine up-to-date with current Identity mainline