-
Notifications
You must be signed in to change notification settings - Fork 1
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
Regression Fix: W-1179558 -- Missing State value when State & Country Picklists are enabled #6962
Regression Fix: W-1179558 -- Missing State value when State & Country Picklists are enabled #6962
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.
added some initial comments
@@ -341,6 +341,15 @@ public inherited sharing class NPSP_Address implements IAddress { | |||
address.Geolocation__Latitude__s = (Decimal) getSObjectField(sobjSrc, fieldPrefixSrc, 'Latitude'); | |||
address.Geolocation__Longitude__s = (Decimal) getSObjectField(sobjSrc, fieldPrefixSrc, 'Longitude'); | |||
|
|||
if (orgConfig.isStateCountryPicklistsEnabled()) { | |||
if (address.MailingCountry__c == null && getSObjectField(sobjSrc, fieldPrefixSrc, 'CountryCode') != null) { |
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.
why do we perform a null check on MailingCountry__c here since we don't have it on MailingState__c below?
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.
I did this because we always want to use StateCode if it's available. This is no longer true 🙃
@@ -63,6 +63,10 @@ public inherited sharing class AddressService { | |||
sobjDst.put(strFieldPrefixDst + 'Country', sobjSrc.get(strFieldPrefixSrc + 'Country')); | |||
sobjDst.put(strFieldPrefixDst + 'Latitude', sobjSrc.get(strFieldPrefixSrc + 'Latitude')); | |||
sobjDst.put(strFieldPrefixDst + 'Longitude', sobjSrc.get(strFieldPrefixSrc + 'Longitude')); | |||
if (orgConfig.isStateCountryPicklistsEnabled()) { |
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.
since we are setting the codes here, we should update the method comments.
@@ -341,6 +341,15 @@ public inherited sharing class NPSP_Address implements IAddress { | |||
address.Geolocation__Latitude__s = (Decimal) getSObjectField(sobjSrc, fieldPrefixSrc, 'Latitude'); | |||
address.Geolocation__Longitude__s = (Decimal) getSObjectField(sobjSrc, fieldPrefixSrc, 'Longitude'); | |||
|
|||
if (orgConfig.isStateCountryPicklistsEnabled()) { |
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.
It seems a little strange that we allow the MailingState and MailingCountry on the address to potentially have a value copied to it in the above block, and then override that value with setting another value in this if
block. And with the country code null check below, that would mean the country code wouldn't be copied to the address if it already has a value, even if it's not the country code
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.
Want to take another look now that we've reverted this logic?
@@ -63,6 +63,10 @@ public inherited sharing class AddressService { | |||
sobjDst.put(strFieldPrefixDst + 'Country', sobjSrc.get(strFieldPrefixSrc + 'Country')); | |||
sobjDst.put(strFieldPrefixDst + 'Latitude', sobjSrc.get(strFieldPrefixSrc + 'Latitude')); | |||
sobjDst.put(strFieldPrefixDst + 'Longitude', sobjSrc.get(strFieldPrefixSrc + 'Longitude')); | |||
if (orgConfig.isStateCountryPicklistsEnabled()) { | |||
sobjDst.put(strFieldPrefixDst + 'StateCode', sobjSrc.get(strFieldPrefixSrc + 'StateCode')); |
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.
Do we still want to do this after the code reversion???
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.
Yes, this method is for Contact <-> Account and Vice Versa, and we need as much information as possible when we eventually create NPSP Addresses.
Work Item: W-1179558
Critical Changes
Changes
Issues Closed
Community Ideas Delivered
Features Intended for Future Release
Features for Elevate Customers
New Metadata
Deleted Metadata