🇭🇺Hungary @junkuncz

@fago you are right. I've just rebased and all looks good (see attachments) cc.: @roderik

junkuncz → made their first commit to this issue’s fork.

Works well, good to go!

Trait has been added and functions are called in FileCeFieldFormatter however there is no action yet (eg.: setting attribute in build()?) based on the configured image style. Honestly I'm not sure about it, the rest is ready for review.

junkuncz → created an issue.

Code looks good and functionality is fine.

Phpstan report is irrelevant regarding code changes. Ready for review.

junkuncz → created an issue.

roderik → credited junkuncz → .

DONE:
- empty plugin settings forms should not see a "settingsForm" edit icon
- hide "extra fields" on EntityCustomElementsDisplayEditForm
- move some logic from buildForm() to form() (it was a debt from an earlier MR)
- a few tiny code clean-up

Okay I'll do the class merge.

Regarding the local tasks: looks like unfortunately we cannot drop out that hook at
https://git.drupalcode.org/project/views_local_tasks/-/blob/1.0.x/views_...
https://git.drupalcode.org/project/custom_elements/-/blob/3.x/modules/cu...

It's weird but probably there is no better way atm.

junkuncz → made their first commit to this issue’s fork.

Yes, added it.

See: MR 53.

Changes:

- CE display is prepared for the case when layout_builder module is enabled (hidden form + "Manage Layout" button)
- Views modes are following core approach so "default" one cannot be deleted
- Broken access handler for CE display is fixed

junkuncz → changed the visibility of the branch 3359601-re-implement-with-new-concept to hidden.

junkuncz → changed the visibility of the branch 8.x-2.x to hidden.

junkuncz → changed the visibility of the branch 3.x to hidden.

junkuncz → changed the visibility of the branch new-ce-formatter to hidden.

junkuncz → changed the visibility of the branch 3362625-customelementformatter-plugin-type to active.

junkuncz → made their first commit to this issue’s fork.

Since there are a lot of changes in an other MR: https://git.drupalcode.org/project/custom_elements/-/merge_requests/29/d...
we decided to merge this with the failing unit test and fix it here: https://www.drupal.org/project/custom_elements/issues/3362625 📌 CustomElementFormatter plugin type for entity_ce_display Active

junkuncz → created an issue.

junkuncz → created an issue.

Okay, added changes. Build is green. Let's handle the 3.x related changes in a dedicated ticket, I'll create it.

3427892-stabilise-ci-build-2.x is ready for review.

junkuncz → created an issue.

Okay it's time to do another check. (tests are fine locally)

Sure, changes have been added.
Looks like test runner is still wrong here, did a check locally and all are still green.

PHPUnit 9.6.17 by Sebastian Bergmann and contributors.

Testing /var/www/html/web/modules/contrib/custom_elements
......                                                              6 / 6 (100%)

Time: 00:28.257, Memory: 4.00 MB

OK (6 tests, 32 assertions)

@fago thanks for the review.
Accidentally I started a review instead of clicking on "Add new comment" button so that's why you didn't see my feedbacks.
All remarks should be covered now.
https://git.drupalcode.org/project/custom_elements/-/merge_requests/33#n...
There is a question here.

Thanks @fago.
Remarks are addressed, please take a look.

Running unit test with strict config schema gives the following error:

Drupal\Tests\custom_elements\Functional\CustomElementsRenderMarkupVue3Test::testNodeRendering
Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for media.type.image with the following errors: media.type.image:source_configuration.gather_exif missing schema

Tried to fix it quickly but I couldn't do, it needs more investigation and deeper understanding of schema structure.

junkuncz → created an issue.

junkuncz → made their first commit to this issue’s fork.

Are you sure we need to do anything about it? I was doing some testing and looks like entity_ce_display config object is deleted automatically during bundle delete and if I uninstall the module all related configs are removed.
Maybe we could add a tiny snippet to hook_uninstall() to make sure all entity_ce_display configs are properly deleted. Something like this:

. $storage = \Drupal::service('entity_type.manager')->getStorage('entity_ce_display');
  $configs = $storage->loadMultiple();
  if (!empty($configs)) {
    $storage->deelete($configs);
  }

What do you think?

Okay it looks good, I was confused because the testes entity did not have bundles.

I'll do more testing on the Eventsubscriber but the first round of review can be started I guess.

junkuncz → created an issue.

junkuncz → made their first commit to this issue’s fork.

Tests should be checked/adjusted? however it works for me.

Works for me as well, thanks!

I can also confirm #12.

junkuncz → created an issue.

MR has been created.

junkuncz → created an issue.

purge-2853613-12.patch patch works well and the code looks very solid however unfortunately I had no time to do a detailed review/report but definitely a good candidate to merge.

I see, okay. Good to go from my side

Adding these changes in patch format, you can quickly apply if you agree of course.

Ignore the error above, it was a configuration issue (used the same zone id twice).
The module does its jobb very well. Regarding the "Purge bunnyCDN" button I agree it's more abstract and there is already a working patch for that in Purge module: https://www.drupal.org/project/purge/issues/2853613 ✨ Invalidate a purger's cache from UI Needs review

I've discovered only two things that should be added/changes I guess:
- add purge module as a dependency to module.info.yml
- "BunnyPurger" class should implement "PurgerInterface"

Looking forward you feedback.

Hello @DieterHolvoet
Thank you for your nice contribution first.
I was doing some testing and it works quite good, except one thing which is maybe(?) my environment issue I'm not sure: when I want to purge the pull zone with ddev drush p:invalidate everything -y (parameters are already added via the config form of course) I'm getting an error which relates to Guzzle I guess Bunny purge request failed. Status code 400: Client error: `POST https://api.bunny.net/pullzone//purgeCache` resulted in a `400 Bad Request` response: {"Message":"The request is invalid."}.
However I can complete the request with a Postman call. Looks like Guzzle wants to send the request "twice" for some reason, I've attached the log (AccessKey was sanitised because of privacy)
I've found a ticket which relates to this error: https://github.com/guzzle/guzzle/issues/1432
As far as I understand when the response code is 204 and the content body is empty it throws and exception.
Could you please check it on your side as well? I'm using a strandard drupal install with Drupal core 10.2 in ddev.

Thanks!

Okay, thanks for the quick reply and explanation.

junkuncz → created an issue.

Bruhh I made some mess here.. anyways the tests are working and I also added changes by Roderik's remarks.
Ready for review now: https://git.drupalcode.org/project/lupus_decoupled/-/merge_requests/72

Looks good to me.