Add short-circuit for currency detection

[jefersonchaves]

Uses isMultiCurrencyOrganization before attempting to check the object.
Removes code duplication with multiple checks for UserInfo.isMultiCurrencyOrganization().
It also can help to make the debugging easier by using variables for the fieldsMap.

---

This change is 

[stohn777]

Thank you for your submission, @jefersonchaves !!

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @jefersonchaves)

---

sfdx-source/apex-common/main/classes/fflib_SObjectSelector.cls line 37 at r1 (raw file):

     * enabled. 
     **/
    private Boolean CURRENCY_ISO_CODE_ENABLED {

Although indent characters are inconsistent throughout the framework, please sync your indenting within the scope of the Getter, at least.

Otherwise reviewing further.

[jefersonchaves]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @stohn777)

---

sfdx-source/apex-common/main/classes/fflib_SObjectSelector.cls line 37 at r1 (raw file):

Previously, stohn777 (John Storey) wrote…

Although indent characters are inconsistent throughout the framework, please sync your indenting within the scope of the Getter, at least.

Otherwise reviewing further.

Hey @stohn777 - Thanks a lot, sure thing. What is the desired indent? I was under the impression it was 4 tab spaces for this file, and I'm happy to fix it at least in this section.

[stohn777]

It also looks like a related test should be updated -- fflib_SObjectSelector.testPolymorphicSelectWithRelatedType(). With the changes, the field list not contains Lead.CurrencyIsoCode and Lead.Owner.CurrencyIsoCode, which I believe are proper. Lead.Owner.CurrencyIsoCode is appropriate because the test is creating a User Selector which should contain the multi-currency field. Might I offer the following a food for thought.

	@isTest
	static void testPolymorphicSelectWithRelatedType() {
		//Given		

		Testfflib_CampaignMemberSelector cmSelector = new Testfflib_CampaignMemberSelector();
		fflib_QueryFactory qf = cmSelector.newQueryFactory();
		new Testfflib_LeadSelector().configureQueryFactoryFields(qf, 'Lead');
		new Testfflib_UserSelector().configureQueryFactoryFields(qf, 'Lead.Owner');


		Set<String> expectedSelectFields = new Set<String>{
				'Id', 'Status', 'Lead.Id', 'Lead.OwnerId', 'Lead.Owner.Id', 'Lead.Owner.UserRoleId'
		};
		if (UserInfo.isMultiCurrencyOrganization()) {
			expectedSelectFields.add('CurrencyIsoCode');
			expectedSelectFields.add('Lead.CurrencyIsoCode');
			expectedSelectFields.add('Lead.Owner.CurrencyIsoCode');  // Because the Selector is for User; Group would not have
		}

		//When
		String soql = qf.toSOQL();

		//Then
		Pattern soqlPattern = Pattern.compile('SELECT (.*) FROM CampaignMember ORDER BY CreatedDate ASC NULLS FIRST ');
		Matcher soqlMatcher = soqlPattern.matcher(soql);
		soqlMatcher.matches();

		List<String> actualSelectFields = soqlMatcher.group(1).deleteWhiteSpace().split(',');
		System.assertEquals(expectedSelectFields, new Set<String>(actualSelectFields));
	}

	@isTest
	static void testPolymorphicSelectWithRelatedTypeWherePolymorphicNotImpactedByMulticurrency() {
		//Given

		Testfflib_CampaignMemberSelector cmSelector = new Testfflib_CampaignMemberSelector();
		fflib_QueryFactory qf = cmSelector.newQueryFactory();
		new Testfflib_LeadSelector().configureQueryFactoryFields(qf, 'Lead');
		new Testfflib_GroupSelector().configureQueryFactoryFields(qf, 'Lead.Owner');


		Set<String> expectedSelectFields = new Set<String>{
				'Id', 'Status', 'Lead.Id', 'Lead.OwnerId', 'Lead.Owner.Id'
		};
		if (UserInfo.isMultiCurrencyOrganization()) {
			expectedSelectFields.add('CurrencyIsoCode');
			expectedSelectFields.add('Lead.CurrencyIsoCode');
			// expectedSelectFields.add('Lead.Owner.CurrencyIsoCode');  // Because Group does NOT have CurrencyIsoCode
		}

		//When
		String soql = qf.toSOQL();

		//Then
		Pattern soqlPattern = Pattern.compile('SELECT (.*) FROM CampaignMember ORDER BY CreatedDate ASC NULLS FIRST ');
		Matcher soqlMatcher = soqlPattern.matcher(soql);
		soqlMatcher.matches();

		List<String> actualSelectFields = soqlMatcher.group(1).deleteWhiteSpace().split(',');
		System.assertEquals(expectedSelectFields, new Set<String>(actualSelectFields));
	}

Along with the following mock in the same file.

	private class Testfflib_GroupSelector extends fflib_SObjectSelector {
		public Testfflib_GroupSelector() {
			super();
		}

		public List<Schema.SObjectField> getSObjectFieldList() {
			return new List<Schema.SObjectField>{
					Group.Id
			};
		}

		public Schema.SObjectType getSObjectType() {
			return Group.sObjectType;
		}
	}

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @jefersonchaves)

---

sfdx-source/apex-common/main/classes/fflib_SObjectSelector.cls line 37 at r1 (raw file):

Previously, jefersonchaves (Jéferson Chaves) wrote…

Hey @stohn777 - Thanks a lot, sure thing. What is the desired indent? I was under the impression it was 4 tab spaces for this file, and I'm happy to fix it at least in this section.

It should be Tabs and only the section you worked on. Don't worry about any more than that, unless you're feeling ambitious.

[jefersonchaves]

Hello @stohn777 - I have made the changes, including a new change on the unit test to validate the currency iso code not available for groups adding to what you initially mentioned. How does it look?

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @stohn777)

[jefersonchaves]

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @stohn777)

---

sfdx-source/apex-common/main/classes/fflib_SObjectSelector.cls line 37 at r1 (raw file):

Previously, stohn777 (John Storey) wrote…

It should be Tabs and only the section you worked on. Don't worry about any more than that, unless you're feeling ambitious.

Done.

[stohn777]

@jefersonchaves In my efforts last week, I overlooked an experimental change which impacted this PR. Please review my change to your PR. Thanks!!

[stohn777]

Reviewed 1 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jefersonchaves)

[jefersonchaves]

That sounds fair enough. If I understood the changes correctly, these are formatting changes + calling addFields with the SobjectType to avoid ambiguity.
Is that right?

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jefersonchaves)

[jefersonchaves]

Reviewed 1 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jefersonchaves)

[jefersonchaves]

Hello @stohn777: thanks for the review. It seems that a 2nd review is required and I'm not authorized to merge, so I leave this up to you peeps.

[stohn777]

@jefersonchaves The original polymorphic test has been failing for a bit. With your PR, motivation was reinvigorated to deal with it too.

You're correct that after further successful review by other maintainers, the PR will be merged.

[jefersonchaves]

Hello @stohn777 - Is there anything else I'm missing, or can I help to move forward?

[daveespo]

A separate PR was opened to address the unit test failure with Mulicurrency orgs -- the scope of this change on this PR was too broad #437