Forbid view/edit/delete in ContactMessageAccessControlHandler

xjm → closed merge request !1302

So the errors are:

Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'{"message":"No route found for \u0022POST \/entity\/contact_message\u0022"}'
+'{"message":"No route found for \u0022POST http:\/\/php-apache-jenkins-drupal-patches-158543\/subdirectory\/entity\/contact_message\u0022"}'

I believe they are stemming from this line:

 $this->assertResourceErrorResponse(404, 'No route found for "POST ' . str_replace($this->baseUrl, '', $this->getEntityResourcePostUrl()->setAbsolute()->toString()) . '"', $response);

In the following file:

core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php

Any thoughts on how to fix?

These are tricky but something with this test needs to be tweaked

$this->assertResourceErrorResponse(404, 'No route found for "POST ' . str_replace($this->baseUrl, '', $this->getEntityResourcePostUrl()->setAbsolute()->toString()) . '"', $response);

Was is the baseUrl being replace?

Working on this as part of #ContributionWeekend2023.

Any idea how I can run one specific test rather than a whole test group? That would hopefully make it easier to debug.

can probably setup your local to run tests and just run the one. Would have to google what you need to do to set that up though.

I think !3131 can be closed, but I didn't create it, so I don't have permission to close it.

Tests are now passing on !5167.

Turned out trying to remove the base path from the URL when calling assertResourceErrorResponse was the wrong way to go. Compared to similar checks elsewhere in core and figured out I could remove the replace from this test. This also meant the setting of basePath could be removed from setUp.

Tests are now passing, so moving to needs review.

1) Drupal\Tests\contact\Kernel\MessageEntityTest::testMessageMethods
Failed asserting that true is false.
/builds/issue/drupal-2878513/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122
/builds/issue/drupal-2878513/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/builds/issue/drupal-2878513/core/modules/contact/tests/src/Kernel/MessageEntityTest.php:70
/builds/issue/drupal-2878513/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
FAILURES!
Tests: 1, Assertions: 23, Failures: 1.

Rebased to run the test-only feature and got a failure as expected.

Seems most feedback has been addressed only moving to NW for the change record.

Good work!

I have created a change record: https://www.drupal.org/node/3479476 →

This is the first time I have created one, so I expect it will need some editing.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

lostcarpark → changed the visibility of the branch 2878513-10.1.x to hidden.

The MessageTest function doTestPostIndividual() was failing as it expects different return codes in different situations, but the contact Message entity always denies access, so always returns a 403.

I thought I could override this function with a custom function that expects 403, but this test is part of JsonAPI, not the contact module, so overriding the test causes other tests to fail.

As we know the contact entity will always return a 403, this test isn't really relevant to the contact module, and we have other tests that verify the 403 return code. I have added a check for the Message entity, and if found, the test is skipped.

I'm not certain this is the correct approach, so appreciate a review.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Fixed return parameter issue flagged by Queue Bot.

smustgrave → closed merge request !3131

Removing novice tag appears to be more advanced then that
Was also tagged for change record which sitll needs to be needed.