There's no so much time left before feature complete phase.

So if we wanna have this feature in D8 we need a clear roadmap and a list of tasks.
Splitting this into a doable amount of is only way to make it real.

Current state
1) Comment Entity - done, needs some clean-ups and optimization
2) Comment Field - done, has follow-ups for counter part (statistics and history modules integration)
3) Views integration - done, but sometimes has fails and fixes in re-rolls
4) Tests - done, supported
5) Upgrate path - partially, in progress of optimization
6) Theme layer - in progress, templates for wrapper an others, twig

Issue to test patches #1907960: Helper issue for "Comment field"

Related issues
#1901110: Improve the UX for comment bundle pages and comment field settings
#1805932: Resolve menu UI issues on 'Comment fields' tab of bundle field ui pages
#1029708: History table for any entity
#1920042: Upgrade path changes
#1898054: comment.module - Convert PHPTemplate templates to Twig

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

@larowlan I'd like to take a part so please update a summary of this issue and let's make it!

Probably you have a different vision but this let's file a separate issue for each of tasks to discuss and have no collision.
Also here's a big difference in timezones :)

larowlan’s picture

Hey latest code is in comment-field API 4 branch
You have commit access. Just failing tests to go.
I will aim to have them back to green by Friday.

andypost’s picture

Issue summary: View changes

Updated issue summary.

andypost’s picture

Storage for new comment should use shorter PK
'primary key' => array('entity_id', 'entity_type', 'field_name'),

Suppose we need it not more then 333 per #1852896: Throw an exception if a schema defines a key that would be over 1000 bytes in MySQL

so maybe 'primary key' => array('entity_id', array('entity_type', 32), array('field_name',32)), do the trick

andypost’s picture

Status: Active » Needs review
FileSize
4.33 KB

Suppose this should be enough?
If ok I fixed also a comments. Suppose RTBC

andypost’s picture

FileSize
4.55 KB

All indexes should be recreated when table is renamed

andypost’s picture

FileSize
6.99 KB

Default values are wrongly set too

andypost’s picture

@larowlan Please clirify the default mode for field

I think better have this setiing in field settings COMMENT_OPEN https://gist.github.com/4710742

But the function used HIDDEN as default function comment_add_default_comment_field($entity_type, $bundle, $field_name = 'comment', $default_value = COMMENT_HIDDEN)

Is there any reason for this?

larowlan’s picture

Hi Andy
Thanks for working on this, I've been busy on other issues and with work but hope to get time for this across the next few days.
I'm pretty sure I wanted it to default to hidden for security/spam reasons.
Drupal 7 defaults to open and that is always the first thing you change when setting up a content type.
But given as the 'automatically create a content type and you get comment functionality' is gone with this patch, there is no reason this shouldn't be open.

Lee

andypost’s picture

There's a big progress with settings moved to widget #1907960-12: Helper issue for "Comment field"

@larowlan Why do you choose to make a lot of different fields in upgrade path?

// IN comment_update_8006()
    $field = array(
      'cardinality' => '1',
      // We need one per content type to match the existing bundles.
      'field_name' => 'comment_node_' . $node_type,
      'module' => 'comment',
andypost’s picture

The roadmap
1) Upgrade path - fix field creation and entity displays Asked a question in #1852966-126: Rework entity display settings around EntityDisplay config entity
2) Provide default value for field when created in field UI
3) Fix tests and views integration

andypost’s picture

Currently waiting for the main issue commit

Next steps:
- upgrade path change - create field only for commented||open node types (patch + upgrade path test mostly done) #1920042: Upgrade path changes
- Implement second formatter and get rid of comment_entity_view() #1901110: Improve the UX for comment bundle pages and comment field settings
- Move part of instance settings to formatters #1919834: Field instance got no default value when created in field UI
- Granular permissions #1903138: Move global comment permissions to comment-type level
- History & tracker #1029708: History table for any entity

andypost’s picture

FileSize
672 bytes

Another issue would be remove the remains of comment in node.module node_title_list() has regression it does not displays the number of comments on node. I think we just need to remove that checking
Other way would be introduce hook_query_alter()

andypost’s picture

Issue summary: View changes

Updated issue summary.

andypost’s picture

Issue summary: View changes

Updated issue summary.

andypost’s picture

Issue summary: View changes

Updated issue summary.

andypost’s picture

Issue summary: View changes

added twig issue

andypost’s picture

We need to investigate the theme layer follow-ups and twig specials

Probably better to file separate issue for twig conversion #1962846: Use field instance name for header of comment list, drop comment-wrapper template

EDIT added twig conversion link #1898054: comment.module - Convert PHPTemplate templates to Twig
details about workaround #1938948: Temporarily allow PHPTemplate themes to use .html.twig templates during Twig conversion

andypost’s picture

andypost’s picture

Also needs review after #1374116: Move bundle CRUD API out of Field API

comment_field_create_instance() & comment_field_delete_instance()

larowlan’s picture

New issues:
*use update_variable_get instead of variable_get in upgrade
*check for fields before creating them in update
*don't use drupal_container()
*convert any new hook_menu entries to routing.yml/controllers

larowlan’s picture

New issues
*form alter that adds the translation checkbox needs tests
*form alter that limits cardinality needs tests

xjm’s picture

Closing in favor of #1900132: [Meta] Comment as field -- we are already tracking the rest of the followups there, and there's a lot more visibility on that issue.

xjm’s picture

Status: Needs review » Closed (duplicate)
Xano’s picture

Xano’s picture

Issue summary: View changes

Updated issue summary.