Skip to content

Moonshots XXI allow for use in declarative sfdc #20

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tmoserLD
Copy link

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

N/A

Describe the solution you've provided

Extending SDK functionality to declaritive SFDC via Flows. Also adjusting SOQL queries and DML statements to limit their occurance to minimize impact on SFDC Governor Limit breaches.

Describe alternatives you've considered

No other way to get Apex SDK into SFDC Flows. Adjusted SOQL queries / DML statements because every call to get a flag variation was consuming 2 SOQL queries. Most SFDC orgs are close to those limits so even consuming 2 SOQL queries in a transaction can be problematic.

Additional context

This was done for Moonshots XXI.

SFDC Flows are the main declarative automation for SFDC. It can only reference Apex code methods annotated with @InvocableMethod. However, only 1 @InvocableMethod can be included in an Apex class. As such, I only implemented the LDClientInvocable.flagVariation method that retrieves a single flag variation. In order to access other LDClient methods such as allFlags, the LDClientInvocable.flagVariation method would have to be augmented or another class/method would have to be created. That being said, SFDC Flows do not handle Map structures very well. So, a method such as allFlags is unlikely to provide more benefit than just retrieving a number of flags individually via the LDClientInvocable.flagVariation method (and passing in theldClient_Flow attribute to conserve SOQL queries).

It should be noted that I also have built a base Apex class file that showcases how to access the LD Apex SDK via SFDC's Omnistudio as an Apex Remote Action. I have omitted that class here since Omnistudio is a paid add-on and it can only be included in the package if Omnistudio exists in the SFDC org. See https://docs.google.com/document/d/1xO-JjPDTmHmulZXEpXXQUzFtVCJetnKUpiNYR--lUDI/edit?usp=sharing.

SFDC Governor Limits reference: https://developer.salesforce.com/docs/atlas.en-us.salesforce_app_limits_cheatsheet.meta/salesforce_app_limits_cheatsheet/salesforce_app_limits_platform_apexgov.htm

tmoserLD added 3 commits June 24, 2024 17:17
Added ability to get LD Flag variations declaratively from an SFDC Flow. Gives access to SFDC Admins to use LD in SFDC via Flows and not just SFDC Developers via Apex.
Test class coverage for LDClientInvocableTest and LDFlowMapKey
@tmoserLD tmoserLD requested a review from a team June 26, 2024 22:36
@cwaldren-ld
Copy link
Contributor

Hey @tmoserLD , thanks for this! I'll need to spend a chunk of time understanding this so can't get to it at the moment. But when I do, I'll reach out to you to get more context.

Copy link

@jbarazoto-launchdarkly jbarazoto-launchdarkly left a comment

Choose a reason for hiding this comment

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

Approved.

@jbarazoto-launchdarkly
Copy link

@cwaldren-ld happy to go over it with you as well (with Tristan leaving) if clarification is needed.


if (count >= this.maxEvents) {
// Preserve SOQL queries (each transaction only gets 100) by getting count once and comparing to updated events count
if (existingEventsCount == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: These instances all shared EventData__c (?). If so, it's not safe to query the existing events count once because we could have a race condition where multiple instances think it's safe to add new events, but it is not.

@@ -59,9 +63,15 @@ public class DataStore implements DataStoreInterface {

public void putAll(Map<String, Object> kinds) {
Copy link
Contributor

Choose a reason for hiding this comment

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

putAll should have a comment saying that it's just used for the side-effect of updating the underlying data tables, and it's not going to have any effect on the Data Store instances used by SDK clients

@cwaldren-ld
Copy link
Contributor

Open question:

Do people need to get different flag values from their SDK instance over time? That is, if you are toggling a flag while an SDK is being used, will the SDK need to reflect this change?

Depends on how people use the SDK.

public String stringVariation;

public InvocableResponse(InvocableRequest request) {
this.booleanVariation = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

q: why is this being set?

}
}

@InvocableMethod(label='Get LaunchDarkly Variation' description='Returns the flag variation for a given of Flag Key')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@InvocableMethod(label='Get LaunchDarkly Variation' description='Returns the flag variation for a given of Flag Key')
@InvocableMethod(label='Get LaunchDarkly Variation' description='Returns the flag variation for a given Flag Key')

@InvocableVariable(label='LaunchDarkly Client' description='Instance of LDClient used to retrieve the variation' required=false)
public LDFlowClient ldClient_Flow;
@InvocableVariable(label='Variation (Boolean)' description='Variation for Boolean Flag type' required=false)
public Boolean booleanVariation;
Copy link
Contributor

Choose a reason for hiding this comment

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

can this actually be null?

private static LDClient determineClient(List<InvocableRequest> requests) {
LDClient client;
for (InvocableRequest request : requests) {
if (client != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we just return on line 81?

valueObjectMap.put(userAttribute.key, LDValue.of(userAttribute.stringValue));
}
}
when else {
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

.setCustom(LDValueObject.fromMap(valueObjectMap))
.build();

if (request.userToBeIdentified == true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (request.userToBeIdentified == true) {
if (request.userToBeIdentified) {

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.

3 participants