Skip to content

DPMMA-3058 Add configuration option to limit maximum Contacts export #14691

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 6, 2025
Merged

DPMMA-3058 Add configuration option to limit maximum Contacts export #14691

merged 4 commits into from
Jun 6, 2025

Conversation

patrykgruszka
Copy link
Member

@patrykgruszka patrykgruszka commented Mar 7, 2025 •
edited
Loading

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

Description

This feature introduces a new configuration option that allows users to set a limit on the maximum number of contacts that can be exported at once. By default, the limit is set to 0, which disables the restriction to maintain backward compatibility.

image

📋 Steps to test this PR:

  1. Navigate to the configuration settings in the Mautic admin panel.
  2. Locate the "Maximum Number of Contacts to Export" option in Contact Settings section.
  3. Verify that the default value is set to 0 (no limit).
  4. Set a specific number to limit the export or leave it at 0 to disable the limit.
  5. Save the configuration.
  6. Attempt to export contacts and verify that the export adheres to the specified limit.
  7. Change the limit and repeat the export to ensure functionality.

@codecov Codecov
Copy link

codecov bot commented Mar 7, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.26%. Comparing base (600909e) to head (5903993).
Report is 5 commits behind head on 7.x.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                7.x   #14691   +/-   ##
=========================================
  Coverage     66.25%   66.26%           
- Complexity    35158    35160    +2     
=========================================
  Files          2315     2315           
  Lines        141609   141640   +31     
=========================================
+ Hits          93829    93860   +31     
  Misses        47780    47780
Files with missing lines Coverage Δ
...p/bundles/LeadBundle/Controller/LeadController.php 62.11% <100.00%> (+0.35%) ⬆️
...dles/LeadBundle/EventListener/ConfigSubscriber.php 100.00% <100.00%> (ø)
app/bundles/LeadBundle/Form/Type/ConfigType.php 100.00% <100.00%> (ø)
🚀 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.

@patrykgruszka patrykgruszka added ready-to-test PR's that are ready to test 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 labels Mar 7, 2025
@patrykgruszka patrykgruszka moved this to 🧑🏻‍💻 Needs a code review in Open Source Fridays Mar 7, 2025
@matbcvo matbcvo added this to the 7.0.0-alpha milestone Mar 7, 2025
imaabasiee
imaabasiee suggested changes Mar 17, 2025
View reviewed changes
Copy link

@imaabasiee imaabasiee left a comment

Choose a reason for hiding this comment

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

Verified that the default value is set to 0 (no limit). Then I changed the limit to 2, attempted to export 4 contacts and it went through. There was no message to restrict or warn that I could only export a maximum of 2 contacts at once.
https://www.loom.com/share/47a80fb873994f1692ff5d34a76eebaf?sid=3cc3b358-d362-452d-9395-77b9b29b7a08

@github-project-automation github-project-automation bot moved this from 🧑🏻‍💻 Needs a code review to 🚨 Developer revision needed in Open Source Fridays Mar 17, 2025
@patrykgruszka patrykgruszka added the pending-feedback PR's and issues that are awaiting feedback from the author label Mar 17, 2025
@patrykgruszka patrykgruszka force-pushed the DPMMA-3058_export-limit-gh7 branch from 39c76b7 to 1f36e7f Compare March 18, 2025 14:44
@patrykgruszka
Copy link
Member Author

@imaabasiee thanks for your comment. I've changed the behavior to prevent limiting the import rows without notifying the user. Now, the export is blocked, and an alert is displayed to inform the user.
image

@patrykgruszka patrykgruszka added ready-to-test PR's that are ready to test and removed ready-to-test PR's that are ready to test pending-feedback PR's and issues that are awaiting feedback from the author labels Mar 19, 2025
imaabasiee
imaabasiee approved these changes Mar 21, 2025
View reviewed changes
Copy link

@imaabasiee imaabasiee left a comment

Choose a reason for hiding this comment

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

Works as expected now. Well done @patrykgruszka
https://www.loom.com/share/3161c15a6dfe4a77a44637d29a867b41?sid=9b305318-9499-46f1-9227-aa3b62f31d5e

@kuzmany
Copy link
Member

kuzmany commented Mar 21, 2025

What is the reason for this feature?
If we offer export, it should be fully available.
I know it's an optionally. Do you have any wrong experiences with large data export?

@patrykgruszka
Copy link
Member Author

Yes, when you have large database, like > 300k contacts, then you may want to limit your exports because of:

  • The server might not handle such a large export.
  • The scheduled task (cron job) could fail if there's not enough memory, and will try creating a new file again until there is no more free space on the filesystem
  • It could freeze the system for users if they try to download a big file directly in their browser.

Bastian2718
Bastian2718 approved these changes Apr 3, 2025
View reviewed changes
Copy link

@Bastian2718 Bastian2718 left a comment

Choose a reason for hiding this comment

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

Looks good!

@patrykgruszka patrykgruszka moved this from 🚨 Developer revision needed to 🧑🏻‍💻 Needs a code review in Open Source Fridays Apr 9, 2025
AlanWierzchonCA
AlanWierzchonCA approved these changes Jun 2, 2025
View reviewed changes
Copy link
Contributor

@AlanWierzchonCA AlanWierzchonCA left a comment

Choose a reason for hiding this comment

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

Thanks for the changes – looks good to me and I approve! 👍

@patrykgruszka patrykgruszka moved this from 🧑🏻‍💻 Needs a code review to 🎉 Ready to commit in Open Source Fridays Jun 2, 2025
@patrykgruszka patrykgruszka added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging labels Jun 2, 2025
@escopecz escopecz merged commit 8e5ca0c into mautic:7.x Jun 6, 2025
20 checks passed
@github-project-automation github-project-automation bot moved this from 🎉 Ready to commit to 🥳 Done in Open Source Fridays Jun 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@escopecz escopecz escopecz approved these changes

+3 more reviewers

@AlanWierzchonCA AlanWierzchonCA AlanWierzchonCA approved these changes

@imaabasiee imaabasiee imaabasiee approved these changes

@Bastian2718 Bastian2718 Bastian2718 approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
enhancement Any improvement to an existing feature or functionality import-export Anything related to importing and exporting ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged
Projects
Open Source Fridays
Status: 🥳 Done
Milestone
7.0.0-alpha
Development

Successfully merging this pull request may close these issues.
7 participants
@patrykgruszka @kuzmany @escopecz @AlanWierzchonCA @imaabasiee @Bastian2718 @matbcvo