#Novice

Hoping we can unblock this issue based on https://www.drupal.org/project/drupal/issues/1798540#comment-16136875 ✨ Add link to add a new entity in an empty entity list controller table Needs work

I've done some testing and reviewed the approach here which seems to be based on #62 and #63

The problem is as soon as we remove the entry from BlockContentViewsData the plugin is no longer functional. Any existing views will not be able to load the plugin and will get a "Broken/missing handler" link on the views UI admin page. We want to remove it so it doesn't leak into new config after being deprecated so I think really the only option here is to write an update hook to update existing views.

I think the upgrade path should be pretty straightforward given we have ViewsConfigUpdater.

With that, I think we can just leave the class as is (not empty it out) so that if for some reason someone is extending it in a way they will at least get the deprecation error and functionality will remain the same?

@julio_retkwa I'm seeing the same composer issues when trying to create a MR against the 8.x-2.x branch, if you have some time to look on it, that would be great, if not, no worries, I'll look on it later.

Automatically closed - issue fixed for 2 weeks with no activity.

because it's not been caused by a form handler.

The form handler is the second part of the _entity_form string, e.g the example above maps to the overview form handler of the Vocabulary entity type.
I also wondered if we should just through the error when the entity type is parsed, but technically at the moment you can have form handlers that aren't tied to routes and therefore don't need to implement the interface.
In Vocabulary's case it had the form handler but the route for it isn't using the _entity_form method and afaik form handlers aren't used in many other ways?
We also might need to throw a deprecation first before a full exception so sites don't blow up?

Maybe SYMFONY_DEPRECATIONS_HELPER is parsed but it does absolutely nothing now and 📌 Upgrade PHPUnit to 10, drop Symfony PHPUnit-bridge dependency Fixed added displayDetailsOnTestsThatTriggerDeprecations="true" which is the only thing that now does something and of course it went in undocumented because there was no phpcs rule to demand documentation and heaven forbid documentation gets written which helps developers.

Ah, I hadn't spotted that!

The exception message will be wrong in that scenario though:

> 'The "%s" form handler of the "%s" entity type specifies a class "%s" that does not extend "%s".'

because it's not been caused by a form handler.

@smustgrave, if the testing was completed and reviewed, please update this issue status.. thank you.

@julio_retkwa it is ok, work on the branch 8.x-2.x, which is the one I still want to maintain, if possible.

Thanks a lot for your work.

I created a MR against branch 8.x-2.x-dev but its failing on composer step
https://git.drupalcode.org/issue/google_calendar_service-3528488/-/jobs/...

I'm missing something here?

PS: I see there are some PHPCS issues on branch 1.x should those be addressed as well? Let me know
https://git.drupalcode.org/issue/google_calendar_service-3528488/-/jobs/...

Thanks!

Hi @danrod sorry, yes

I was not aware that you was working on this, sorry for jumping in

@julio_retkwa are you going to take this one?

I'm running into the same issue on 11.1.7. The proposed patch is not working for me. For some reason, the current and new_state values in the form state are empty. Even when looking at the fields in the form array, I see every attribute but value is empty.

It does catch it, try adding this (mimics VocabularyRouteProvider::getOverviewPageRoute)

entity.vocab.overview:
  path: '/admin/structure/taxonomy/manage/{taxonomy_vocabulary}/overview2'
  defaults:
    _entity_form: 'taxonomy_vocabulary.overview'
    _title: 'Test'
  requirements:
    _entity_access: 'taxonomy_vocabulary.access taxonomy overview'
  options:
    _admin_route: TRUE
  parameters:
    taxonomy_vocabulary:
      with_config_overrides: TRUE

EntityRouteEnhancer::enhanceEntityForm sets the controller to controller.entity_form:getContentResult based on the _entity_form default. That calls HtmlEntityFormController::getFormObject which calls EntityTypeManager::getFormObject

The _entity_form route default is tied directly to the form handler class.

Update on the issue:

An initial attempt was made to fix this by implementing \Drupal\jsonapi_extras\Entity\JsonapiResourceConfig::onDependencyRemoval(), as pointed out by @Wim Leers, with the goal of cleaning up deleted fields from the resourceFields array. However, it turned out that the method onDependencyRemoval() wasn’t being triggered at all when a field was removed.

Upon further investigation, it appears the root cause is that \Drupal\jsonapi_extras\Entity\JsonapiResourceConfig::calculateDependencies() only adds the only adds the bundle (e.g. node.type.*) as a dependency; it doesn’t include individual field configs like field.storage.node.field_foo. Without those dependencies, the entity system doesn’t invoke onDependencyRemoval() when fields are deleted.

Proposed solution:

The plan is to update calculateDependencies() to explicitly add each field listed in resourceFields as a dependency (e.g., field.storage.node.field_foo). With that in place, onDependencyRemoval() will be properly triggered when a field is deleted.

Inside onDependencyRemoval(), any removed fields will be automatically cleaned out of the override config. This prevents stale field entries from lingering in the YAML and eliminates the need for manual cleanup.

Just wanted to check in and see if this approach makes sense before finalising the patch. Let me know your thoughts!

The check in core/lib/Drupal/Core/Entity/EntityTypeManager.php is not what I was expecting, and it's a good idea because it'll give a better error for people who use the wrong sort of form class as an entity handler.

But it won't catch the case where you use _entity_form directly in a route definition.

Working on it.

Hi @danrod,

Reviewed and applied changes and working fine. Thanks.

Hi @danrod,
I have reviewed and applied the changes, and everything is working fine.
Here before and after screenshots attached.
Thank you!

Encountered this in ✨ Allow vocabulary overview form class to be overridden Active and I agree we should improve this by throwing an error. I don't think this is novice based on the test coverage required.

Putting it to "Needs Review" just in case anyone has any suggestions.

It looks like the pipeline passed with a few warnings: https://git.drupalcode.org/project/sagepay/-/pipelines/513061

Good!

I'll mark this as complete and open a new issue for fixing the PHPCS/PHPSTAN/STYLEINT issues.

I have updated the ComponentValidationTest.php and .schema file as per the suggestions. For next steps I have to confirm that we will be creating "ValidStructuredDataPropExpression.php" and "ValidStructuredDataPropExpressionValidator.php" inside the /src/Plugin/Validation/Constraint , right?

Perfect, thanks!

@nod_ sorry for the delay. I've raised the MR and attached some images for clarity. Moving the issue to NR.

sandip → reopened merge request !11620

I created a MR for this, please review: https://git.drupalcode.org/project/search_api_page/-/merge_requests/14

Screenshot attached:

I'll move this to "Fixed" and merge it to the main branch then.

I applied the change and it looks much better !

Does anyone would like to review this? for extra credits?

I'll leave it to "Needs Review"

danrod → changed the visibility of the branch 3527965-add-some-space to active.

danrod → changed the visibility of the branch 3527965-add-some-space to hidden.

that looks reasonable

If you agree i am updating this in the MR so.

Hi @catch, @mherchel
Can we try something like this below. Is it the right approach for it?

diff --git a/core/themes/olivero/olivero.theme b/core/themes/olivero/olivero.theme
index b2f3bff2684..0ee7fed1c81 100644
--- a/core/themes/olivero/olivero.theme
+++ b/core/themes/olivero/olivero.theme
@@ -365,6 +365,7 @@ function olivero_preprocess_field(&$variables): void {
 
   if (in_array($variables['field_type'], $rich_field_types, TRUE)) {
     $variables['attributes']['class'][] = 'text-content';
+    $variables['#attached']['library'][] = 'olivero/olivero.table';
   }
 
   if ($variables['field_type'] == 'image' && $variables['element']['#view_mode'] == 'full' && !$variables["element"]["#is_multiple"] && $variables['field_name'] !== 'user_picture') {

New change:

/**
 * Implements hook_preprocess_HOOK().
 */
function olivero_preprocess_field(&$variables): void {
  $rich_field_types = ['text_with_summary', 'text', 'text_long'];

  if (in_array($variables['field_type'], $rich_field_types, TRUE)) {
    $variables['attributes']['class'][] = 'text-content';
    $variables['#attached']['library'][] = 'olivero/olivero.table';
  }

  if ($variables['field_type'] == 'image' && $variables['element']['#view_mode'] == 'full' && !$variables["element"]["#is_multiple"] && $variables['field_name'] !== 'user_picture') {
    $variables['attributes']['class'][] = 'wide-content';
  }
}

@mherchel - best to re-open issues if there's a problem or create a follow-up, it's easy for comments on fixed issues to go missing.

I've reverted this from 11.x and 11.2.x for more discussion. We could potentially add the library when text fields are rendered too.

I resolved the phpstan issue by adding " // @phpstan-ignore argument.type" comment by taking reference from

KeyForEverySdcProp

validator file. Please review and let me know if anymore changes are required.

Unassinging myself so that someone else can work for test cases and keeping the issue to needs work only.

I'm triaging RTBC issues → . I read the IS, comments, and the MR. I didn't find any unanswered questions or other work to do.

I updated credit.

Leaving at RTBC.

Moving this to "Fixed" for now, then. If anyone is having a particular issue with this task, please let me know and I'll re-open the task

I applied the changes and works like a charm except when the screen width is bigger than 768px and the main menu has a lot of items (see attachments)
Works fine when the screen width is less than 768px and when the main menu and the left menu don't stack.

FullyValidatable: ~ constraint is missing

Hey @lavanyatalwar I applied some changes to fix this, I'll update the sandbox environment so you can try it on mobile devices as well:

https://dev-d10-date-timepicker.pantheonsite.io/

I'll mark this task as "Fixed" for now.

Automatically closed - issue fixed for 2 weeks with no activity.

I made some changes and I centered the search bar, and increased the size of the input field for screen sizes bigger than 768 px:

I'll move it to "Needs Review" for now.

I'll look into this myself

Automatically closed - issue fixed for 2 weeks with no activity.

I'll move it to the 1.0.x branch for now, I plan to create some more tests, in addition to the PHPUnit tests of course.

I'll merge it to the 8.x-1.x branch for now, if you see any other issues please let me know @esha_kundu thanks !

@quietone that work for you? Moving to RTBC to put in committer eyes.

Believe this is a net improvement.

Seems like valid conditions.

Installed the condition_test module and used configuration_inspector

Ticket talks about deprecating a method but don't see the deprecation

smustgrave → closed merge request !11663

Tried my best to review this one. Based on the files mentioned in the summary believe they have been fixed, If I'm wrong I apologize now.

Has test failures.

Hi @esha_kundu , I tested your changes and It worked like a charm !:

commiting the changes to a new MR and moving this to "Needs Review" just in case anyone has some suggestions.

I created a test page, index.html which you can find in the root of the project, in the example-html-files directory:

You can use these fields to tests the jQuery deprecated functions, I plan to add more, once I add more deprecated files, this is a work in progress, but I think this is good for now.

Sending this to "Needs Review" if anyone wants to have a look on it.

Hello @joachim,
I've updated the text, please check and review.

Thank you!

Since patches are no longer tested, a merge request needs to be provided.

I pushed the changes to the Drupal core branch and updated the issue status to 'Needs review'.

I’ve assigned this issue to myself. I initially made the changes in the XB core's user.schema.yml, but later realized that the changes should go into Drupal core. I’d appreciate some guidance on how to proceed and push the changes to the correct branch.

meghasharma → closed merge request !12279

Added ConfigExists constraint to user_role condition schema

Fix looks good, so good that it broke the tests that assumed a theme that it's not installed. We need to fix those tests.

please review this https://git.drupalcode.org/project/drupal/-/merge_requests/12277

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

Hello @danrod,
Sorry for the late reply, I've tested the MR!52 in Drupal version 10.4.6. The MR applied cleanly, and the logo is visible and is the correct size. However, I found that you have used a static width for the image, due to which when Use the logo supplied by the theme is disabled and a custom logo is used, the logo renders in a very small size. IMO, we can make use of the below snippet

.navbar-header .site-branding img {
  max-width: 80px;
  max-height: 56px;
}

I'm attaching the SS for reference.
Will be glad you hear from you.
Thanks

Also related 📌 Replace comment constants with enums Active .

That introduces a CommentStatus enum, but it could become EntityStatus instead. The implementation is already done in the MR. We could consider moving it here instead and leave that issue for all the other comment constants.