Don't expose entity types with string ids as a target option when creating comment types

Is the problem that the option has been moved from the edit page to manage fields page ?

Is that really critical?

I think having an issue summary with more information would be helpful :)

Interesting, this for sure sounds like a real bug we should fix :)

Tag work

We should disable creation of comment type for non-integer IDs for 8.0.x because of beta stage.
Also comment_form_field_ui_field_storage_add_form_alter() and new alter function should point to #2496699: Allow comments to be attached to entities using a string primary key

Also it would be great to update comment_help() to dynamically generate a list of entity types that can't be commented atm

Agree - the fix here is to prevent listing those entity types in the target-entity type field of the comment-type form.
If any do exist, we should add a description indicating why those aren't listed.

Added message and limited available entity-types.

Failing test.

Screenshot:

This interface looks pretty afwul, can we have a update on the issue summary why we need this what usecase its meant to solve etc?

Why is this DX? This is UI? I am not sure why we ever made this decision on hiding?

I agree with #3 here, the issue summary is incomplete.

This is UX problem - we can't add comment fields to entity type that has non-integer ID (internal)
So we need to show some message to user why he's failed to add commenting

I think the existing patch is still the best approach - @andypost - can you review?

I'm late to this particular party. Initial thought on first read of the issue summary was "why would we want to list the entities you can *not* add comments to?". It mostly lists stuff you likely don't want to add comments to anyway. Of course there will be exotic use cases where it would be needed. But that is not fixed by actually spelling out that you can’t anyway. I don't think we should explain up front why a certain technical limitation is reducing available options.

Not sure we need to call it out at all. If people feel strongly about somehow explaining this somewhere:

- Consider mentioning this in /admin/help/comment
- Have a "Missing some options here?" link to an explanation somewhere else (either the above help page or d.o)

But even as I type out these options I'm not sure it's helpful explaining this on the UI level *if there's nothing you can do to work around it*.

- Consider mentioning this in /admin/help/comment
- Have a "Missing some options here?" link to an explanation somewhere else (either the above help page or d.o)

+1 to #26, the current patch lists the unsupported entity types without an explanation which is confusing and against what this CR claimed: Comments allowed on any entity type.

However, telling site builders/drupal users the truth directly on the add comment field page, that comments does not work on string ID-based entity types, which sounds like Drupal sucks. Further more, some of them won't encounter it.

I think we'd all agree that what's most important here is hiding the entity types that don't work in the drop-down. Currently this is labelled a major bug.

What we should do explain this to users could be a lower priority follow-up task.

Let's simplify #11 to the actual uncontroversial bug fix and get it in first.

Let's simplify #11 to the actual uncontroversial bug fix and get it in first.

Well, made a patch based on #11, tagging Bug Smash Initiative

+++ b/core/modules/comment/src/CommentTypeForm.php
@@ -100,10 +101,10 @@ public function form(array $form, FormStateInterface $form_state) {
-        // Only expose entities that have field UI enabled, only those can
-        // get comment fields added in the UI.

Self-review: should not remove the comment.

Fixing the test cases.
I think in test cases we were asserting incorrect checks.
We should check "responseNotContains()". The reason is, we are expecting that entities with integer id should appear in the list.
Test is passing on local now.

Seems right to me.

Please do not ask the testbot to try again until #3207086: [HEAD BROKEN] Consistent failure in MonthDatePluginTest is fixed.

#3207086: [HEAD BROKEN] Consistent failure in MonthDatePluginTest is fixed. I have not reviewed the code.

Good to see this RTBC after so long. If we ever make comments work with string ID entity types we can reverse it of course.

Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!