Skip to content

Comments

feat: Init Cal.com Salesforce package#26330

Merged
volnei merged 106 commits intomainfrom
init-salesforce-package
Jan 13, 2026
Merged

feat: Init Cal.com Salesforce package#26330
volnei merged 106 commits intomainfrom
init-salesforce-package

Conversation

@joeauyeung
Copy link
Contributor

@joeauyeung joeauyeung commented Dec 31, 2025

Stacked on #26007

What does this PR do?

This PR creates the Cal.com Salesforce package, to send user update data to Cal.com as a part of the attribute sync feature.

  • Creates the Salesforce package using Apex
  • Creates scripts to create test orgs for development and pushing packages to the test org
  • Updates README

Updates since last revision

Addressed Cubic AI review feedback (confidence >= 9/10):

  • Removed sensitive fields (email, changedFields) from logging in user-sync.ts to avoid exposing PII
  • Renamed scratch-org:open to scratch-org:start in package.json to match README documentation

Items flagged but not addressed (confidence < 9/10 or requires design decision):

  • Named Credential authentication: Currently uses principalType="Anonymous" - authentication will be handled in a stacked PR via instanceUrl + orgId validation against stored credentials (see TODO in user-sync.ts)
  • Change detection for nullified fields: UserUpdateHandler.cls may miss fields cleared to null
  • Sequential HTTP callouts: Performance consideration for bulk updates

Visual Demo (For contributors especially)

https://www.loom.com/share/ec50726997a04027ab0f6dc2f3b1c562

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • In the Salesforce folder
    • Run yarn scratch-org:create to create a test org
    • Run yarn sfdc:deploy to deploy the current SF package to the scratch org
    • Run yarn scratch-org:start to open the scratch org
  • Update a user in the scratch org
    • The updated data should be sent to your cal.com instance

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my changes generate no new warnings

Link to Devin run: https://app.devin.ai/sessions/03d81257bb5445389176c29823afac1d
Requested by: unknown ()

devin-ai-integration bot and others added 2 commits January 12, 2026 19:38
- RuleBuilder.tsx: Use unique IDs for React keys instead of array index to prevent reconciliation bugs when removing conditions
- updateAttributeSync.handler.ts: Add early organization check before service calls for consistency
- createAttributeSync.handler.ts: Add CredentialNotFoundError class for type-safe error handling instead of string matching
- IntegrationAttributeSyncService.test.ts: Replace expect.fail() with proper Vitest rejects.toSatisfy() pattern

Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
@joeauyeung joeauyeung marked this pull request as ready for review January 12, 2026 22:43
@joeauyeung joeauyeung requested a review from a team as a code owner January 12, 2026 22:43
@graphite-app graphite-app bot added the enterprise area: enterprise, audit log, organisation, SAML, SSO label Jan 12, 2026
@graphite-app graphite-app bot requested a review from a team January 12, 2026 22:44
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

5 issues found across 15 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/app-store/salesforce/api/user-sync.ts">

<violation number="1" location="packages/app-store/salesforce/api/user-sync.ts:14">
P1: Rule violated: **Avoid Logging Sensitive Information**

`log.info` writes the raw request payload (including `email` and potentially sensitive `changedFields`) to application logs, exposing PII in violation of the logging policy. Remove or mask sensitive fields before logging.</violation>
</file>

<file name="packages/app-store/salesforce/sfdc-package/force-app/main/default/classes/CalComCalloutQueueable.cls">

<violation number="1" location="packages/app-store/salesforce/sfdc-package/force-app/main/default/classes/CalComCalloutQueueable.cls:14">
P2: Sequential HTTP callouts in a loop may cause performance issues or timeout errors with multiple payloads. Consider checking Limits.getCallouts() before each callout and potentially chaining to a new Queueable job if the limit is approached, or batch payloads into a single API request if the endpoint supports it.</violation>
</file>

<file name="packages/app-store/salesforce/sfdc-package/force-app/main/default/namedCredentials/CalCom_Production.namedCredential-meta.xml">

<violation number="1" location="packages/app-store/salesforce/sfdc-package/force-app/main/default/namedCredentials/CalCom_Production.namedCredential-meta.xml:6">
P0: Critical security vulnerability: Named Credential sends sensitive user data without authentication. The configuration uses principalType="Anonymous" and generateAuthorizationHeader="false", and the callout code doesn't add any Authorization header or API key. This allows anyone who discovers the endpoint to send unauthorized data to Cal.com. Add authentication using either Named Credential's per-user or per-org authentication, or implement API key authentication in the HTTP request headers.</violation>
</file>

<file name="packages/app-store/salesforce/package.json">

<violation number="1" location="packages/app-store/salesforce/package.json:12">
P2: The new script is named `scratch-org:open` while our README and workflow instructions expect `scratch-org:start`, so the documented `yarn scratch-org:start` command still fails. Rename or alias the script so the documented command works.</violation>
</file>

<file name="packages/app-store/salesforce/sfdc-package/force-app/main/default/classes/UserUpdateHandler.cls">

<violation number="1" location="packages/app-store/salesforce/sfdc-package/force-app/main/default/classes/UserUpdateHandler.cls:35">
P1: Change detection misses cleared/nullified fields. When a User field is set to null, it won't appear in `newFields.keySet()` (since `getPopulatedFieldsAsMap()` only returns populated fields), so the change won't be detected. This causes field deletions to not sync to Cal.com.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

<fullName>CalCom_Production</fullName>
<label>Cal.com Production</label>
<endpoint>https://app.cal.com</endpoint>
<principalType>Anonymous</principalType>
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 12, 2026

Choose a reason for hiding this comment

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

P0: Critical security vulnerability: Named Credential sends sensitive user data without authentication. The configuration uses principalType="Anonymous" and generateAuthorizationHeader="false", and the callout code doesn't add any Authorization header or API key. This allows anyone who discovers the endpoint to send unauthorized data to Cal.com. Add authentication using either Named Credential's per-user or per-org authentication, or implement API key authentication in the HTTP request headers.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/salesforce/sfdc-package/force-app/main/default/namedCredentials/CalCom_Production.namedCredential-meta.xml, line 6:

<comment>Critical security vulnerability: Named Credential sends sensitive user data without authentication. The configuration uses principalType="Anonymous" and generateAuthorizationHeader="false", and the callout code doesn't add any Authorization header or API key. This allows anyone who discovers the endpoint to send unauthorized data to Cal.com. Add authentication using either Named Credential's per-user or per-org authentication, or implement API key authentication in the HTTP request headers.</comment>

<file context>
@@ -0,0 +1,12 @@
+    <fullName>CalCom_Production</fullName>
+    <label>Cal.com Production</label>
+    <endpoint>https://app.cal.com</endpoint>
+    <principalType>Anonymous</principalType>
+    <protocol>Custom</protocol>
+    <allowMergeFieldsInBody>false</allowMergeFieldsInBody>
</file context>
Fix with Cubic

Copy link
Member

Choose a reason for hiding this comment

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

It is planned for stacked PR I believe

Comment on lines +35 to +41
for (String fieldName : newFields.keySet()) {
Object oldValue = oldFields.get(fieldName);
Object newValue = newFields.get(fieldName);

if (oldValue != newValue) {
changedFields.put(fieldName, newValue);
}
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 12, 2026

Choose a reason for hiding this comment

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

P1: Change detection misses cleared/nullified fields. When a User field is set to null, it won't appear in newFields.keySet() (since getPopulatedFieldsAsMap() only returns populated fields), so the change won't be detected. This causes field deletions to not sync to Cal.com.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/salesforce/sfdc-package/force-app/main/default/classes/UserUpdateHandler.cls, line 35:

<comment>Change detection misses cleared/nullified fields. When a User field is set to null, it won't appear in `newFields.keySet()` (since `getPopulatedFieldsAsMap()` only returns populated fields), so the change won't be detected. This causes field deletions to not sync to Cal.com.</comment>

<file context>
@@ -0,0 +1,46 @@
+        Map<String, Object> oldFields = oldUser.getPopulatedFieldsAsMap();
+        Map<String, Object> newFields = newUser.getPopulatedFieldsAsMap();
+
+        for (String fieldName : newFields.keySet()) {
+            Object oldValue = oldFields.get(fieldName);
+            Object newValue = newFields.get(fieldName);
</file context>

Fix confidence (alpha): 8/10

Suggested change
for (String fieldName : newFields.keySet()) {
Object oldValue = oldFields.get(fieldName);
Object newValue = newFields.get(fieldName);
if (oldValue != newValue) {
changedFields.put(fieldName, newValue);
}
Set<String> allFields = new Set<String>();
allFields.addAll(oldFields.keySet());
allFields.addAll(newFields.keySet());
for (String fieldName : allFields) {
Object oldValue = oldFields.get(fieldName);
Object newValue = newFields.get(fieldName);
Fix with Cubic

Copy link
Member

Choose a reason for hiding this comment

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

Seems valid concern again @joeauyeung

public void execute(QueueableContext context) {
String namedCredential = getNamedCredential();

for (UserChangePayload payload : payloads) {
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 12, 2026

Choose a reason for hiding this comment

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

P2: Sequential HTTP callouts in a loop may cause performance issues or timeout errors with multiple payloads. Consider checking Limits.getCallouts() before each callout and potentially chaining to a new Queueable job if the limit is approached, or batch payloads into a single API request if the endpoint supports it.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/salesforce/sfdc-package/force-app/main/default/classes/CalComCalloutQueueable.cls, line 14:

<comment>Sequential HTTP callouts in a loop may cause performance issues or timeout errors with multiple payloads. Consider checking Limits.getCallouts() before each callout and potentially chaining to a new Queueable job if the limit is approached, or batch payloads into a single API request if the endpoint supports it.</comment>

<file context>
@@ -0,0 +1,54 @@
+    public void execute(QueueableContext context) {
+        String namedCredential = getNamedCredential();
+
+        for (UserChangePayload payload : payloads) {
+            sendPayload(namedCredential, payload);
+        }
</file context>
Fix with Cubic

Copy link
Member

Choose a reason for hiding this comment

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

Valid point. I think we should batch the payloads if possible and send multiple of them in a single salesforce/user-sync request.

Queueable seems to have a limit of 100 callouts

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the feedback! I've saved this as a new learning to improve future reviews.

@github-actions
Copy link
Contributor

Devin AI is addressing Cubic AI's review feedback

A Devin session has been created to address the issues identified by Cubic AI.

View Devin Session

- Remove sensitive fields (email, changedFields) from logging in user-sync.ts
- Rename scratch-org:open to scratch-org:start in package.json to match README

Co-Authored-By: unknown <>
Base automatically changed from attribute-sync-ui to main January 12, 2026 22:57
@joeauyeung joeauyeung requested a review from a team as a code owner January 12, 2026 22:57
@@ -1,8 +1,20 @@
## Working With GraphQL
# Creating a Salesforce test org
If you have the Salesforce CLI installed, you can create a test org using the following command `yarn scratch-org:create`
Copy link
Member

@hariombalhara hariombalhara Jan 13, 2026

Choose a reason for hiding this comment

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

image This throws error for me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I'll add a point that you need to have the SF CLI tool installed

if (res.getStatusCode() >= 200 && res.getStatusCode() < 300) {
System.debug(LoggingLevel.INFO, 'Successfully sent user update to Cal.com for user: ' + payload.salesforceUserId);
} else {
System.debug(LoggingLevel.ERROR, 'Failed to send user update to Cal.com. Status: ' + res.getStatusCode() + ' Body: ' + res.getBody());
Copy link
Member

Choose a reason for hiding this comment

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

How do we plan to handle failures in call out? Will we retry them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's one option but I think warrants a discussion

hariombalhara
hariombalhara previously approved these changes Jan 13, 2026
Copy link
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

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

Tested. Seems to work !!

@volnei volnei enabled auto-merge (squash) January 13, 2026 23:01
@volnei volnei merged commit 2664eb4 into main Jan 13, 2026
45 checks passed
@volnei volnei deleted the init-salesforce-package branch January 13, 2026 23:01
@volnei volnei restored the init-salesforce-package branch January 13, 2026 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO ready-for-e2e size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants