Skip to content

Conversation

aarohiprasad
Copy link
Contributor

@aarohiprasad aarohiprasad commented May 16, 2025

Q A
Bug fix? (use the a.b branch) No
New feature/enhancement? (use the a.x branch) Enhancement
Deprecations? No
BC breaks? (use the c.x branch) No
Automated tests included? Yes
Related user documentation PR URL mautic/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed

Description:

While importing a customItems with LinkedContactIds, If provided contactId is not valid or deleted from instance it is failing whole import process, hence the remaining records in csv are able to import.
Now, we are checking if provided contactId is valid or not and if it is not valid just log the warning and continue import process for remaining records in csv.

Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Create a csv for import CustomItems with LinkedContactIds
  3. add some invalid contactIds for LinkedContactIds column in csv
  4. Import the CustomItems for customObject.
  5. Observe that all customItems should import and you can see the warnings for invalid contacts on import details page.

dadarya0 and others added 4 commits May 16, 2025 16:08
MAUT-11097 : Logging warnings while importing contacts or customitems
@aarohiprasad aarohiprasad added unforking Used for PRs in the Acquia's unforking initiative WIP PR's that are not ready for review and are currently in progress labels May 16, 2025
Copy link

codecov bot commented May 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.87%. Comparing base (c5ee255) to head (4928ce5).
Report is 19 commits behind head on 7.x.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                7.x   #15008   +/-   ##
=========================================
  Coverage     65.87%   65.87%           
- Complexity    35033    35037    +4     
=========================================
  Files          2303     2303           
  Lines        140919   140934   +15     
=========================================
+ Hits          92829    92845   +16     
+ Misses        48090    48089    -1     
Files with missing lines Coverage Δ
app/bundles/LeadBundle/Entity/LeadRepository.php 68.24% <100.00%> (+0.28%) ⬆️
...pp/bundles/LeadBundle/Event/ImportProcessEvent.php 100.00% <100.00%> (ø)
...p/bundles/LeadBundle/Event/ImportValidateEvent.php 100.00% <ø> (ø)
app/bundles/LeadBundle/Model/ImportModel.php 84.21% <100.00%> (+0.16%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aarohiprasad aarohiprasad added code-review-needed PR's that require a code review before merging and removed WIP PR's that are not ready for review and are currently in progress labels May 19, 2025
@aarohiprasad aarohiprasad requested review from a team, escopecz and nileshlohar and removed request for a team May 19, 2025 18:30
Copy link
Contributor

@nileshlohar nileshlohar left a comment

Choose a reason for hiding this comment

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

Code changes LGTM

Copy link
Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

I fixed a few issues I saw in the code changes. I tested contact import and also error reporting. For example this is what happens for 1 contact in the middle of the CSV with an invalid email address:
Screenshot 2025-05-20 at 11 54 22

I know this PR is aiming to improve import error reporting for the custom objects plugin. But it doesn't work on M7 yet. This is what I got when I tried to install it on its 5.x branch:

bin/console m:p:u

In CheckExceptionOnInvalidReferenceBehaviorPass.php line 119:
                                                                                                                                                   
  The service "mautic.custom.model.export_scheduler" has a dependency on a non-existent service "mautic.helper.export". Did you mean one of these  
  : "mautic.helper.paths", "mautic.helper.user", "mautic.helper.bundle", "mautic.helper.cookie", "mautic.helper.update", "mautic.helper.cache", "  
  mautic.helper.theme", "mautic.helper.url", "mautic.helper.composer", "mautic.helper.menu", "mautic.helper.hash", "mautic.helper.random", "mauti  
  c.helper.sms", "mautic.helper.mailer"? 

But I wouldn't delay merging this PR because of it. The core functionality works correctly 👍

@escopecz escopecz merged commit 6f5101b into mautic:7.x May 20, 2025
20 checks passed
@escopecz escopecz added enhancement Any improvement to an existing feature or functionality import-export Anything related to importing and exporting labels May 20, 2025
@escopecz escopecz added this to the 7.0.0-alpha milestone May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-review-needed PR's that require a code review before merging enhancement Any improvement to an existing feature or functionality import-export Anything related to importing and exporting unforking Used for PRs in the Acquia's unforking initiative
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants