Problem/Motivation

The test entity_reference_test module, found in the core system module is where one would typically add custom Entity Reference Selection plugins used in testing. The test views_test_entity_reference module, found in the core views module is needed to implement the views_data_alter hook which enables the EntityReference views filter plugin for EntityReference fields.

The issue is both of these modules cannot be enabled in the same test class as they both install a view with the same machine name test_entity_reference. As a result I am unable to add a custom EntityReferenceSelection plugin that I can use when testing the EntityReference filter.

Issue #3542714: Unable to get/set form_state values in SelectionPluginBase::validateConfigurationForm when called from EntityReference::validateExtraOptionsForm is an example of where this type of testing is needed.

Steps to reproduce

  1. Set up a fresh Drupal 11 dev site
  2. Edit the existing \Drupal\Tests\views_ui\Functional\FilterEntityReferenceWebTest class and add the entity_reference_test module to the existing $modules property
  3. Run any test within the FilterEntityReferenceWebTest class
  4. The test fails with the following error

Drupal\Core\Config\PreExistingConfigException: Configuration objects (views.view.test_entity_reference) provided by views_test_entity_reference already exist in active configuration

Proposed resolution

  1. Rename the existing test_entity_reference view in the views_test_entity_reference test module to test_entity_reference_view
  2. Update existing tests that reference this view to the new machine name

Remaining tasks

  • MR review

User interface changes

None

Introduced terminology

None

API changes

None

Data model changes

None

Release notes snippet

None

CommentFileSizeAuthor
#7 3543036_error.png109.47 KBsolimanharkas

Issue fork drupal-3543036

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

tame4tex created an issue. See original summary.

tame4tex’s picture

tame4tex commented
Assigned: tame4tex » Unassigned
Issue summary: View changes
Status: Active » Needs review

Pushed fix. Ready for review!

tame4tex opened merge request !13081

nicxvan’s picture

nicxvan commented

Seems pretty straightforward, but should the view machine name start with the whole test module name to prevent future conflicts?

solimanharkas made their first commit to this issue’s fork.

solimanharkas opened merge request !13083

solimanharkas’s picture

solimanharkas commented
StatusFileSize
new 3543036_error.png109.47 KB

I can confirm this issue exists.

Steps to reproduce:

  • Added entity_reference_test to the $modules array in FilterEntityReferenceWebTest.php
  • Ran PHPUnit 10.5 with Drupal 11.2.3 and PHP 8.3
  • Got the error:
    Drupal\Tests\views_ui\Functional\FilterEntityReferenceWebTest::testFilterUi
Drupal\Core\Config\PreExistingConfigException: Configuration objects (views.view.test_entity_reference) provided by views_test_entity_reference already exist in active configuration

    Please see the screenshot

#2 Fix the Problem. Following the suggestion in #4, I created a new merge request that renames the view to use the full test module name as a prefix.
The view is now named views_test_entity_reference_view instead of test_entity_reference_view for better namespace isolation.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

@solimanharkas there was already an MR though doing something very similar why a new one?

solimanharkas’s picture

solimanharkas commented

@smustgrave I wasn’t sure which ID / file name would be better, so I didn’t want to overwrite the existing solution in case it’s already correct and sufficient.

tame4tex’s picture

tame4tex commented
Status: Needs work » Needs review

@solimanharkas while I appreciate the code review, given it has been less than 24 hours since I posted the MR and I hadn't yet had a chance to respond to @nicxvan comments, I am wondering if adding MR code comments with code suggestions or simply adding your suggested machine name in a comment might have been less confusing than adding another MR. If I hadn't replied in a few days, than I think jumping in and changing the existing MR would have also been ok.

@nicxvan thanks for the review and good point!

I have modified the machine name to include the module name but also tried to make it descriptive of what it is. I also changed the label and added a description to provide more information. I feel like this could help when writing new tests and seeing if there is an existing view that can be used instead of creating another one. Back to NR.

@solimanharkas given I have used a different machine name and made additional changes, I have hidden your MR to prevent confusion.

tame4tex changed the visibility of the branch 3543036-views-test-entity-reference-naming-conflict to hidden.

nicxvan’s picture

nicxvan commented

@solimanharkas I just wanted to make sure it is clear we appreciate your help! Please don't be discouraged from contributing by the feedback here!

But to highlight there are a couple of general Drupal etiquette items as I see for core.

Generally for any issue it's customary to give the original author a couple of days to respond to feedback, especially if they have contributed recently. As mentioned if you still want to help in that situation opening MR suggestions can be a good way to do that. An important note, that creating suggestions based on another user's feedback may not always reach the bar of getting credit on a core issue.

Second, creating a new MR is usually reserved for a completely new approach or if an MR gets too far out of date so rebasing is not possible.

To summarize the preference is to give the contributor some time to reply and provide MR suggestions if you are trying to help out.

If it goes stale you can leave a comment mentioning you are going to pick it up, then you can push up the changes directly or create a new MR depending on what makes sense for the change your making.

nicxvan’s picture

nicxvan commented
Status: Needs review » Reviewed & tested by the community

I think this is good now!

Less likely to conflict and much more semantic for the name.

smustgrave’s picture

smustgrave commented

Thanks everyone!

smustgrave’s picture

smustgrave commented

Saving credit

tame4tex’s picture

tame4tex commented

Thanks @nicxvan & @smustgrave for the quick response!

@solimanharkas I want to echo @nicxvan’s comment, I definitely don’t want to discourage you from helping. We need all the help we can get! If my earlier comment came across otherwise, apologies. I really appreciate your review.

solimanharkas’s picture

solimanharkas commented

Thank you all a lot for your kind replies and respect . I really appreciate it.

And please @tame4tex , you don’t need to apologize at all. I truly value the feedback and support.

Honestly, the only reason I opened a new MR was because I wasn’t sure if adding commits to the existing one might break something that already works. I just wanted to be careful.

I don’t have much experience with Drupal core issues yet, but your comments really helped me understand the process better. I’ll know how to handle it next time.

Thanks again, it really means a lot.

quietone’s picture

quietone commented

Triaging the RTBC queue.

This is straightforwards as was said in #4. I applied the diff and searched for 'test_entity_reference' and confirmed that the correct instances are changed. I didn't find any that should be changed that are not. Well done.

I checked credit. Everything is in order here.

And thanks to @nicxvan for helping with how we work in the Drupal core issue queue.

astonvictor’s picture

astonvictor commented

+1 RTBC

catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 11.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 11.3.x and 11.2.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • catch committed 78114f24 on 11.3.x 
    fix: #3543036 Cannot enable both views_test_entity_reference and...

  • catch committed 89fb06f1 on 11.x 
    fix: #3543036 Cannot enable both views_test_entity_reference and...

catch closed merge request !13081

  • catch committed 324c8fa3 on 11.2.x 
    fix: #3543036 Cannot enable both views_test_entity_reference and...
herved’s picture

herved commented

It seems this is causing failures, https://git.drupalcode.org/issue/drupal-3559030/-/jobs/7370601

Filter Entity Reference Web (Drupal\Tests\views_ui\Functional\FilterEntityReferenceWeb)
     ✘ Filter ui
       ┐
       ├ InvalidArgumentException: Input "options[reference_views][view][view_and_display]" cannot take "test_entity_reference:entity_reference" as a value (possible values: "views_test_entity_reference_filtered_display:entity_reference").
       │
       │ /builds/issue/drupal-3559030/vendor/symfony/dom-crawler/Field/ChoiceFormField.php:126
       │ /builds/issue/drupal-3559030/vendor/behat/mink-browserkit-driver/src/BrowserKitDriver.php:422
       │ /builds/issue/drupal-3559030/vendor/behat/mink/src/Element/NodeElement.php:118
       │ /builds/issue/drupal-3559030/core/tests/Drupal/Tests/UiHelperTrait.php:107
       │ /builds/issue/drupal-3559030/core/modules/views_ui/tests/src/Functional/FilterEntityReferenceWebTest.php:110
       ┴

  • catch committed b5e99584 on 11.2.x 
    Revert "fix: #3543036 Cannot enable both views_test_entity_reference and...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.2.x-dev » 11.x-dev
Status: Fixed » Needs work

Reverted, let's try again.

  • catch committed e69616b2 on 11.3.x 
    Revert "fix: #3543036 Cannot enable both views_test_entity_reference and...

  • catch committed f12a6d44 on 11.x 
    Revert "fix: #3543036 Cannot enable both views_test_entity_reference and...

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.