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

Feature/pt bdi support checkbox fields #4105

Merged
merged 28 commits into from
Mar 18, 2019

Conversation

ptewson-sfdo
Copy link
Contributor

@ptewson-sfdo ptewson-sfdo commented Mar 7, 2019

Bring more responsible support for mapping to checkbox fields to BDI/BGE. Mapped field on
Data Import must be picklist with None/True/False or text field for full support, but current user processes that use checkbox-to-checkbox mappings will be unaffected by this change.

Critical Changes

Changes

Batch Gift Entry Enhancements

  • Batch Gift Entry now supports improved mapping to standard and custom checkbox fields. In order to allow your users more flexibility around checkbox fields in Batch Gift Entry and Data Importer, the corresponding field should be a picklist. This picklist translates what you see in the Opportunity or Payment checkbox field (checked or unchecked) to their boolean equivalents (True for checked and False for unchecked). Through Batch Gift Entry, your users will be able to uncheck a previously checked checkbox field or even leave a checkbox field completely unchanged by leaving that field blank for any given donation line item. Read more about this in the Add Custom Fields to Your Batch documentation.

Issues Closed

New Metadata

Deleted Metadata

// This allows for mapping checkboxes to a tri-state picklist - None, True, False - or a text with either
// blank, "True", or "False"
String destinationSObjectname = destinationRecord.getSObjectType().getDescribe().getName();
if ((UTIL_Describe.getFieldDisplayType(destinationSObjectname, destinationField) ==
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Field name must be LOWERCASE. Also namespace support.

Copy link
Contributor

@klewis-sfdc klewis-sfdc left a comment

Choose a reason for hiding this comment

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

A few stylistic comments is all from me! Looks great.

// This allows for mapping checkboxes to a tri-state picklist - None, True, False - or a text with either
// blank, "True", or "False"
String destinationSObjectname = destinationRecord.getSObjectType().getDescribe().getName();
if (!(value instanceof Boolean) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

I know were nesting a few ifs here, but I might consider adding one more and putting the destinationSObjectname var inside the first value instanceof Boolean if block, since we don't need to get the SObject name to inspect this unless the field is Boolean.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option could be to extract the comparison to a little utility method, for readability - something like if(booleanMappingIsValid(value, destinationSObjectname). This is good - I'm just trying to think through it if it could be a little more readable what with multiple calls to those UTIL classes here.

@@ -878,6 +878,18 @@ global with sharing class BDI_DataImportService {
return false;
}

// If the destination field is a Boolean and the DI field intermediate requires interpretation to Boolean
// This allows for mapping checkboxes to a tri-state picklist - None, True, False - or a text with either
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this allow for mapping from picklists with any values? if so we might not want to specifify "None" here, since admins can configure whatever PL values they want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

None just means null or no value ... it's important for the behavior but I've been struggling with how to type that to make it clear that it's not an actual picklist option with text "none" but that there IS a valid option in the picklist that is neither true or false and evaluates to null

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I should just say "null" ... 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And if the PL values aren't parse-able to a boolean value with Boolean.valueOf() it will fail

Copy link
Contributor

Choose a reason for hiding this comment

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

I see! Ok, yeah, very cool.

Copy link
Contributor

@klewis-sfdc klewis-sfdc left a comment

Choose a reason for hiding this comment

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

One more little idea, but all up to you of course - this is super cool and exciting to add into bdi!

@@ -878,6 +878,18 @@ global with sharing class BDI_DataImportService {
return false;
}

// If the destination field is a Boolean and the DI field intermediate requires interpretation to Boolean
// This allows for mapping checkboxes to a tri-state picklist - None, True, False - or a text with either
Copy link
Contributor

Choose a reason for hiding this comment

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

I see! Ok, yeah, very cool.

// This allows for mapping checkboxes to a tri-state picklist - None, True, False - or a text with either
// blank, "True", or "False"
String destinationSObjectname = destinationRecord.getSObjectType().getDescribe().getName();
if (!(value instanceof Boolean) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Another option could be to extract the comparison to a little utility method, for readability - something like if(booleanMappingIsValid(value, destinationSObjectname). This is good - I'm just trying to think through it if it could be a little more readable what with multiple calls to those UTIL classes here.

@cpose
Copy link
Contributor

cpose commented Mar 15, 2019

**lurch: attach W-031084

Patrick Tewson added 2 commits March 15, 2019 13:02
…alesforceFoundation/Cumulus into feature/pt-bdi-support-checkbox-fields
// null, "True", or "False"
if (!(value instanceof Boolean) &&
fieldIsBooleanDisplayType(destinationRecord, destinationField)) {

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 much easier to read!!

@ptewson-sfdo ptewson-sfdo merged commit ed66093 into master Mar 18, 2019
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.

4 participants