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

Salesforce - truncate activity name to 80 characters #8989

Merged

Conversation

kuzmany
Copy link
Member

@kuzmany kuzmany commented Jul 6, 2020

Please be sure you are submitting this against the staging branch.

Q A
Bug fix? Y
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks?
Deprecations?

Description:

Salesforce activities sync https://docs.mautic.org/en/plugins/salesforce stop working If activity name has more like 80 characters. And it happend in many cases for us, for example Page hit.

Because we are not able to extend this limit (hardcoded limit by Salesforce), this PR truncates the activity name.

image

Steps to reproduce the bug:

Instead wasting time, just code review should be enoght.

  1. Setup the Salesforce integration https://docs.mautic.org/en/plugins/salesforce (make sure to set up Mautic's Custom Object in Salesforce or you'll get errors!)
  2. In the Salesforce integration settings, make sure to add "Page hit" to the "Events to include in the activity sync":
    image
  3. Create a landing page in Mautic with a very long name (80+ characters)
  4. Make sure a contact has a "Page Hit" event on that page:
    image
  5. Sync contacts with Salesforce (bin/console mautic:integration:synccontacts -i Salesforce --env=dev)
  6. Sync events with Salesforce (php bin/console mautic:integration:pushactivity -i Salesforce --env=dev)
  7. Activity is not synced. If you are on dev mode, you can check logs ans see error in log

image

Steps to test this PR:

  1. Load up this PR
  2. Repeat all steps and activity should synced

@kuzmany kuzmany added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test labels Jul 6, 2020
@kuzmany kuzmany added this to the 3.0.2 milestone Jul 6, 2020
@RCheesley RCheesley added code-review-needed PR's that require a code review before merging T3 Hard difficulty to fix (issue) or test (PR) labels Jul 6, 2020
@dennisameling dennisameling self-requested a review July 18, 2020 15:23
@dennisameling dennisameling changed the base branch from staging to 3.0 July 18, 2020 15:24
@dennisameling
Copy link
Member

Closing and re-opening for Travis

@codecov
Copy link

codecov bot commented Jul 18, 2020

Codecov Report

Merging #8989 into 3.0 will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##                3.0    #8989      +/-   ##
============================================
- Coverage     35.24%   35.23%   -0.01%     
- Complexity    27900    27949      +49     
============================================
  Files          1731     1731              
  Lines         96443    96561     +118     
============================================
+ Hits          33987    34020      +33     
- Misses        62456    62541      +85     
Impacted Files Coverage Δ Complexity Δ
...pp/bundles/EmailBundle/Form/Type/EmailOpenType.php 0.00% <0.00%> (ø) 7.00% <0.00%> (+2.00%)
...bundles/PluginBundle/Controller/AjaxController.php 0.00% <0.00%> (ø) 82.00% <0.00%> (+34.00%)
app/bundles/PointBundle/Model/PointModel.php 25.44% <0.00%> (+1.17%) 43.00% <0.00%> (ø%)
...dles/EmailBundle/EventListener/PointSubscriber.php 79.59% <0.00%> (+6.86%) 20.00% <0.00%> (+13.00%)

Copy link
Member

@dennisameling dennisameling left a comment

Choose a reason for hiding this comment

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

NOTE FOR 2nd REVIEWER: code review is enough, the PR works as described

Can confirm the bug (took quite some time to set up the whole integration, good lord 😂):

    {
      "body": [
        {
          "message": "Timeline Name: data value too large: Page hit - https:\/\/mautic3.ddev.site\/test-salesforce-very-long-name-blablabalbaasdfsdfjsdlkfjsdlkfjsdlfkasdfjkhsfkljhsdfljkhsdjklfhsdljkfsjkldfhskjldhflsjkhfjklsdhfkljsdhfkljsdhlfkjhdflksdhflkshflksdhflkdshalkfjhlkdjhfslkhfldk (max length=80)",
          "errorCode": "STRING_TOO_LONG",
          "fields": [
            "Name"
          ]
        }
      ],
      "httpHeaders": {},
      "httpStatusCode": 400,
      "referenceId": "5-00Q2X00001MNkPDUA1"
    }

After applying this PR, the activity is created as expected in Salesforce 🚀

image

@dennisameling dennisameling added pending-test-confirmation PR's that require one test before they can be merged and removed ready-to-test PR's that are ready to test labels Jul 18, 2020
@RCheesley
Copy link
Sponsor Member

LGTM 🎉

@RCheesley RCheesley added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged labels Jul 19, 2020
@RCheesley RCheesley merged commit 87f6649 into mautic:3.0 Jul 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs code-review-needed PR's that require a code review before merging ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged T3 Hard difficulty to fix (issue) or test (PR)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants