Skip to content

chore: Add logging to Salesforce#20167

Merged
keithwillcode merged 8 commits into
mainfrom
add-more-logging
Mar 18, 2025
Merged

chore: Add logging to Salesforce#20167
keithwillcode merged 8 commits into
mainfrom
add-more-logging

Conversation

@joeauyeung
Copy link
Copy Markdown
Contributor

@joeauyeung joeauyeung commented Mar 18, 2025

What does this PR do?

  • Fixes #XXXX (GitHub issue number)
  • Fixes CAL-XXXX (Linear issue number - should be visible at the bottom of the GitHub issue description)

Visual Demo (For contributors especially)

A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).

Video Demo (if applicable):

  • Show screen recordings of the issue or feature.
  • Demonstrate how to reproduce the issue, the behavior before and after the change.

Image Demo (if applicable):

  • Add side-by-side screenshots of the original and updated change.
  • Highlight any significant change(s).

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?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

@graphite-app graphite-app Bot requested a review from a team March 18, 2025 13:15
@keithwillcode keithwillcode added consumer core area: core, team members only labels Mar 18, 2025
@dosubot dosubot Bot added the crm-apps area: crm apps, salesforce, hubspot, close.com, sendgrid label Mar 18, 2025
@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented Mar 18, 2025

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (03/18/25)

1 reviewer was added to this PR based on Keith Williams's automation.

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Mar 18, 2025 7:26pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Mar 18, 2025 7:26pm

if (error.name === "DUPLICATES_DETECTED") {
const existingId = this.getExistingIdFromDuplicateError(error);
if (existingId) {
console.log("Using existing record:", existingId);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should be using the logger instead of console.log

Comment on lines -833 to +838
console.error(e);
log.error(`Error ensuring fields ${fieldsToTest} exist on object ${sobject} with error ${e}`);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same here, replacing console.log with logger

});
if (extractedText) {
writeOnRecordBody[field.name] = extractedText;
continue;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since each field has one type these continue statements are called after the field is processed by it's respective field type function.

Comment on lines +1118 to +1122
log.error(
`No value found for field ${field.name} with value ${
personRecord[field.name]
}, field config ${JSON.stringify(fieldConfig)} and Salesforce config ${JSON.stringify(field)}`
);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Additional error logging when a field isn't being written to. The Salesforce field config doesn't contain any PII. The only possible PII is the field value but we're already logging that.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 18, 2025

E2E results are ready!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

consumer core area: core, team members only crm-apps area: crm apps, salesforce, hubspot, close.com, sendgrid ready-for-e2e

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants