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
Comment | File | Size | Author |
---|---|---|---|
#12 | ncount.patch | 672 bytes | andypost |
#6 | comment_schema-6.patch | 6.99 KB | andypost |
#5 | comment_schema.patch | 4.55 KB | andypost |
#4 | comment_schema.patch | 4.33 KB | andypost |
Comments
Comment #1
andypost@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 :)
Comment #2
larowlanHey 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.
Comment #2.0
andypostUpdated issue summary.
Comment #3
andypostStorage 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 trickComment #4
andypostSuppose this should be enough?
If ok I fixed also a comments. Suppose RTBC
Comment #5
andypostAll indexes should be recreated when table is renamed
Comment #6
andypostDefault values are wrongly set too
Comment #7
andypost@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?
Comment #8
larowlanHi 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
Comment #9
andypostThere'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?
Comment #10
andypostThe 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
Comment #11
andypostCurrently 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
Comment #12
andypostAnother 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()
Comment #12.0
andypostUpdated issue summary.
Comment #12.1
andypostUpdated issue summary.
Comment #12.2
andypostUpdated issue summary.
Comment #12.3
andypostadded twig issue
Comment #13
andypostWe 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
Comment #14
andypostPatch needs merge after:
#1735118: Convert Field API to CMI
#1967294: Add a dedicated @EntityType annotation
Comment #15
andypostAlso needs review after #1374116: Move bundle CRUD API out of Field API
comment_field_create_instance() & comment_field_delete_instance()
Comment #16
larowlanNew 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
Comment #17
larowlanNew issues
*form alter that adds the translation checkbox needs tests
*form alter that limits cardinality needs tests
Comment #18
xjmClosing 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.
Comment #19
xjmComment #20
Xano#18 was supposed to link to #731724: Convert comment settings into a field to make them work with CMI and non-node entities.
Comment #20.0
XanoUpdated issue summary.