Cannot merge company with itself

[nishant-s7]

Q	A
Bug fix? (use the a.b branch)	[x]
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/mautic-documentation#...
Related developer documentation PR URL	mautic/developer-documentation#...
Issue(s) addressed	Fixes #12599

Description:

This PR blocks the possibility of merging a company with itself by not showing the company in the merge list.

Steps to test this PR:

1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
2. Go to Companies and click on any company.
3. Click on Merge (available in the header options).
4. In the Search options, verify whether the currently viewed company is present or not.

[RCheesley]

Works great - I don't see the current company in the list of available companies to merge into:

Thanks for the PR @nishant-s7 and welcome to the community! 🚀

[RCheesley]

Looks like there's still some issues which need to be addressed with the automated tests here @nishant-s7 - I've marked this for a second user test, please shout if you need help with addressing the test failures.

[nishant-s7]

I think the tests are failing because I added

'model_lookup_method' => 'getSimpleLookupResults',
                'lookup_arguments'    => fn (Options $options): array => [
                    'type'     => 'lead.company',
                    'exclude'  => $options['main_entity'],
                ],

to CompanyListType.php, so for each case it is using getSimpleLookupResults instead of getLookupResults (which is desired). So I think I need to pass an optional parameter CompanyController.php's mergeAction function. I will proceed with this approach, but if anyone has a better approach, please let me know.

Thanks!

[PatrickJenkner]

Worked as described when tested 👍

[codecov]

Codecov Report

Attention: Patch coverage is 73.80952% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 61.31%. Comparing base (4039055) to head (504292f).

❗ Current head 504292f differs from pull request most recent head 1ded149. Consider uploading reports for the commit 1ded149 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##                5.x   #13578      +/-   ##
============================================
- Coverage     61.38%   61.31%   -0.07%     
+ Complexity    34024    34012      -12     
============================================
  Files          2238     2238              
  Lines        101685   101679       -6     
============================================
- Hits          62420    62349      -71     
- Misses        39265    39330      +65     

Files	Coverage Δ
...dle/Form/ChoiceLoader/EntityLookupChoiceLoader.php	83.49% <100.00%> (-9.65%)	⬇️
.../bundles/CoreBundle/Form/Type/EntityLookupType.php	97.56% <100.00%> (ø)
...undles/LeadBundle/Controller/CompanyController.php	51.65% <100.00%> (+3.59%)	⬆️
...pp/bundles/LeadBundle/Entity/CompanyRepository.php	58.40% <100.00%> (+0.55%)	⬆️
...p/bundles/LeadBundle/Form/Type/CompanyListType.php	100.00% <100.00%> (ø)
.../bundles/LeadBundle/Form/Type/CompanyMergeType.php	100.00% <100.00%> (+100.00%)	⬆️
app/bundles/LeadBundle/Model/CompanyModel.php	69.80% <56.00%> (-0.92%)	⬇️

... and 19 files with indirect coverage changes

[nishant-s7]

Hello! How do I address this test failure?

Error: Codecov: Failed to properly upload: The process '/home/runner/work/_actions/codecov/codecov-action/v3/dist/codecov' failed with exit code 255

[escopecz]

It's a network issue. I restarted the failed job

[LordRembo]

Looking good! Tests failing again though. Seems to be happening on other PR's as well since couple of days.

[escopecz]

It's usually just a temporary issue. I re-triggered it again. @nishant-s7 by the rule book each PR should have a test covering the change functions as it should so it won't get broken with future changes and the whole platform is more stable. Do you know how to write a functional test? There are many examples of such test in each bundle. If you'll need help, feel free to ask here for on Slack.

[nishant-s7]

I've never written any functional tests, but I'll try to do so by referring to the examples👍
Thanks!

[escopecz]

@nishant-s7 thanks! You can check for example https://github.com/mautic/mautic/blob/5.x/app/bundles/LeadBundle/Tests/Functional/Controller/LeadControllerTest.php#L38

The test should:

1. Create 2 companies
2. Make a GET request to load the form to merge companies
3. Assert that only the other company is present in the HTML output, not the one from the request.

[nishant-s7]

Hello!
How do I run a specific test? As mentioned here, I tried running

bin/phpunit --bootstrap vendor/autoload.php --configuration app/phpunit.xml.dist --filter "/::testVariantEmailWeightsAreAppropriateForMultipleContacts( .*)?$/" Mautic\EmailBundle\Tests\EmailModelTest app/bundles/EmailBundle/Tests/Model/EmailModelTest.php

but it throws this error Cannot open file "MauticEmailBundleTestsEmailModelTest".

[shinde-rahul]

@nishant-s7, Use this bin/phpunit --bootstrap vendor/autoload.php --configuration app/phpunit.xml.dist --filter testVariantEmailWeightsAreAppropriateForMultipleContacts instead. I have never tried the way it is documented.

[nishant-s7]

Hi @shinde-rahul , on trying this, i got this error: Error: Undefined constant "Mautic\EmailBundle\MonitoredEmail\SORTARRIVAL". I tried for another simple test which I wrote, and it gave Error: Class "PDO" not found. I checked modules using php -m and PDO is present. Is there any procedure that I have to follow before testing?

[escopecz]

I usually run specific tests with

composer test -- --filter testVariantEmailWeightsAreAppropriateForMultipleContacts

Those are weird failures. Do you have the right PHP version and composer dependencies installed? If you use DDEV or other container setup for development, you have to run the test from the container. So in case of DDEV you'd firstly ddev ssh into the container and then run the test.

[escopecz]

Nice test! Thanks! Please check my question and a suggestion.