Updated: Comment #0

Problem/Motivation

We should use yml files so the uuid is consistent

Proposed resolution

Make yml files for comment and taxonomy term fields and instances used by forum module

Remaining tasks

Patch
Review

User interface changes

None

API changes

None

Follow-up from #2077435: Use a yml file to create the forum vocabulary.

CommentFileSizeAuthor
#12 interdiff.txt5.95 KBandypost
#12 drupal8.forum-module.2106243-11.patch12.04 KBandypost
#10 drupal8.forum-module.2106243-10.patch10.71 KBjibran
#10 interdiff.txt3.47 KBjibran
#6 interdiff.txt1.47 KBlarowlan
#6 forum-yml-2106243.2.patch12.27 KBlarowlan
#4 forum-install-config-2106243-4.patch12.02 KBRainbowArray
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Assigned: Unassigned » larowlan

Working on this

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

Also there's related issue from list https://drupal.org/node/1653026#sub-tasks #2105963: Make sure all YML files in Forum module has no type-casting to string.

RainbowArray’s picture

RainbowArray
she/her
Primary language English
Location Twin Cities, Minnesota
CreditAttribution: RainbowArray commented
Assigned: larowlan » RainbowArray
RainbowArray’s picture

RainbowArray
she/her
Primary language English
Location Twin Cities, Minnesota
CreditAttribution: RainbowArray commented
Status: Active » Needs review
FileSize
forum-install-config-2106243-4.patch12.02 KB

Worked on this patch with larowlan's guidance. It doesn't get us all the way there due to the following issues:

https://drupal.org/node/2030073

https://drupal.org/node/2080823

Or at least that's part of the issue. The config files aren't loading in the correct order. They're loading by alpha order rather than by dependency.

Manually doing an entity load on the field was still leading to the same error when running the DBlog test, where on creating a forum topic, the forums select list is empty.

So this is a start. More work needed!

Status: Needs review » Needs work

The last submitted patch, forum-install-config-2106243-4.patch, failed testing.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Needs work » Needs review
FileSize
forum-yml-2106243.2.patch12.27 KB
interdiff.txt1.47 KB

This includes #2077435: Use a yml file to create the forum vocabulary so should be postponed on that (but want a test run first).

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

mmm green

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Needs review » Postponed

waiting on #2077435: Use a yml file to create the forum vocabulary

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Postponed » Active
Issue tags: +Needs reroll

#2077435: Use a yml file to create the forum vocabulary is in

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Active » Needs review
Issue tags: -Needs reroll
FileSize
interdiff.txt3.47 KB
drupal8.forum-module.2106243-10.patch10.71 KB

Interdiff contains manual changes in core/modules/forum/forum.install.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Needs review » Reviewed & tested by the community

yay forum_install() looks so. much. cleaner.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
FileSize
drupal8.forum-module.2106243-11.patch12.04 KB
interdiff.txt5.95 KB

The approach with comment body field is not working so I changed the field to forum_comment_body - I think it makes sense to store comment body for forum content in separate table
Also added uuid for 'General discussion' forum to disallow duplicate creation when module re-installed

EDIT related
#1969800: Add UUIDs to default configuration
#2106459: Core config has everything as string

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/modules/forum/forum.install
@@ -19,15 +19,19 @@ function forum_install() {
+      'uuid' => 'f7247c00-318d-11e3-aa6e-0800200c9a66',

Oh nice! I thought of this then thought it wasn't possible to specify a uuid. This is awesome, especially for deployability.

+++ b/core/modules/forum/config/field.field.comment.forum_comment_body.yml
@@ -1,8 +1,8 @@
+id: comment.forum_comment_body

There are several places in core that specify ->comment_body. For example:

  • rdf.module
  • standard profile's comment rdf mapping
  • recent comments view
  • comment tokens
  • comment admin page (although this is wrapped in a check to see if it exists first so irrelevant and only used for the truncated title attribute)

When you say 'The approach with comment body field is not working' can you specify why/where?

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

'The approach with comment body field is not working' can you specify why/where?

there's some reasons:
1) I got failure twice when installed forum module with error that instance can't be created without field (field is deleted when last instance removed - remove article content and then try to install forum)

2) performance - having separate table for body makes queries faster
3) deployment of separate table seems more useful

suppose it's easy to fix rdf and other tests

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

I guess the other option is to check for the field in forum module preinstall?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

I guess the other option is to check for the field in forum module preinstall?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

Can #1426804: Allow field storages to be persisted when they have no fields. help here?
If they comment body field can't be removed (even if no instances) then the problem goes away yeah?

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Issue tags: +Entity Field API

@larpwlan all your point are valid. But I still think that better to have separate fields for forum

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

12: drupal8.forum-module.2106243-11.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 12: drupal8.forum-module.2106243-11.patch, failed testing.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Issue summary: View changes
Status: Needs work » Closed (duplicate)

Fixed by other CMI issues