Problem/Motivation

Extend tmgmt_demo to depend to paragraphs_demo and add tmgmt specific configuration.

Proposed resolution

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#15 2499619-15.patch1.78 KBjoshi.rohit100
#11 Screen Shot 2015-07-13 at 16.42.25.png25.46 KBsasanikolic
#11 extend_tmgmt_demo_to-2499619-11-interdiff.txt919 bytessasanikolic
#11 extend_tmgmt_demo_to-2499619-11.patch2.28 KBsasanikolic
#9 extend_tmgmt_demo_to-2499619-9-interdiff.txt381 bytessasanikolic
#9 extend_tmgmt_demo_to-2499619-9.patch1.38 KBsasanikolic
#8 extend_tmgmt_demo_to-2499619-8-interdiff.txt898 bytessasanikolic
#8 extend_tmgmt_demo_to-2499619-8.patch1.75 KBsasanikolic
#3 extend_tmgmt_demo_to-2499619-3-interdiff.txt5.5 KBsasanikolic
#3 extend_tmgmt_demo_to-2499619-3.patch1.95 KBsasanikolic
#1 extend_tmgmt_demo_to-2499619-1.patch5.35 KBsasanikolic

Comments

sasanikolic’s picture

sasanikolic commented
StatusFileSize
new extend_tmgmt_demo_to-2499619-1.patch5.35 KB

Added the dependency to paragraphs_demo and the configuration for enabling paragraph translation.

Status: Needs review » Needs work

The last submitted patch, 1: extend_tmgmt_demo_to-2499619-1.patch, failed testing.

sasanikolic’s picture

sasanikolic commented
StatusFileSize
new extend_tmgmt_demo_to-2499619-3.patch1.95 KB
new extend_tmgmt_demo_to-2499619-3-interdiff.txt5.5 KB

Removed the dependency and the config files into the paragraph patch. #2461689: demo module: Multilingual setup

sasanikolic’s picture

sasanikolic commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: extend_tmgmt_demo_to-2499619-3.patch, failed testing.

sasanikolic queued 3: extend_tmgmt_demo_to-2499619-3.patch for re-testing.

arla’s picture

arla commented

I am not very well introduced with TMGMT but let me just add a few review comments here :)

  1. +++ b/config/install/tmgmt.settings.yml
@@ -2,4 +2,4 @@ quick_checkout: 1
-respect_text_format: 1
+respect_text_format: 1
\ No newline at end of file

    Accidental removal of newline. You may want to configure your IDE to always add newline. In PhpStorm: Preferences/Editor/General/"Ensure newline at file end on Save". 

  2. +++ b/config/install/tmgmt.settings.yml
--- /dev/null
+++ b/modules/demo/config/install/tmgmt_content.settings.yml

+++ b/modules/demo/config/install/tmgmt_content.settings.yml
+++ b/modules/demo/config/install/tmgmt_content.settings.yml
@@ -0,0 +1 @@

@@ -0,0 +1 @@
+embedded_fields: {  }

    This file may be unnecessary? Or maybe should be provided by tmgmt_content rather than tmgmt_demo?

sasanikolic’s picture

sasanikolic commented
Status: Needs work » Needs review
StatusFileSize
new extend_tmgmt_demo_to-2499619-8.patch1.75 KB
new extend_tmgmt_demo_to-2499619-8-interdiff.txt898 bytes

Thanks for the review!

sasanikolic’s picture

sasanikolic commented
StatusFileSize
new extend_tmgmt_demo_to-2499619-9.patch1.38 KB
new extend_tmgmt_demo_to-2499619-9-interdiff.txt381 bytes

Another empty line...

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Needs work

TMGMT should get a commend in the README about support for paragraphs_demo. Explain how to setup. Example: First setup paragraphs_demo module, then tmgmt_demo

I still like to have paragraphs_demo part of simplytest dependencies.
The problem with the patch above: When visiting admin/config/regional/tmgmt_settings the paragraphs field does not show up.
Also you might need to check for the field existence before setting it to translatable.

Looks like the setup is incomplete.

sasanikolic’s picture

sasanikolic commented
Status: Needs work » Needs review
StatusFileSize
new extend_tmgmt_demo_to-2499619-11.patch2.28 KB
new extend_tmgmt_demo_to-2499619-11-interdiff.txt919 bytes
new Screen Shot 2015-07-13 at 16.42.25.png25.46 KB

Added a few lines to the README file and added the simplytest dependency. I spoke with Berdir about it and tested simplytest, but it is not supported for Drupal 8 yet.

The paragraphs field shows up correctly for me. I have this patch applied. See the attached screenshot.

Also discussed with Berdir about checking for the field and decided that this check is not needed. Even if the field gets deleted after the module is enabled, It does not break anything...

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

I forgot the paragraphs_demo is not yet multilingual. So let's push that forward ASAP first... :-)

Berdir queued 11: extend_tmgmt_demo_to-2499619-11.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 11: extend_tmgmt_demo_to-2499619-11.patch, failed testing.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Status: Needs work » Needs review
StatusFileSize
new 2499619-15.patch1.78 KB

Re-rolled the #11

Berdir queued 15: 2499619-15.patch for re-testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

Tested on simplytest.me, works great. Committed.

  • Berdir committed 774e088 on 8.x-1.x authored by joshi.rohit100 
    Issue #2499619 by sasanikolic, joshi.rohit100: Extend tmgmt_demo to...

Status: Fixed » Closed (fixed)

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