Problem/Motivation

I'm working on tour_ui and right now, i'm forced to list manually all configuration attributes of plugin to be able to export them.
Information is already available in configuration protected attribute in TipPluginBase, so getting this data will help a lot.

Proposed resolution

Add getConfiguration() method to TipPluginBase so we can get all configuration attributes.
getConfiguration() method is part of ConfigurablePluginInterface, so TipPluginInterface should extends ConfigurablePluginInterface and PluginBase implement this the interface methods.

CommentFileSizeAuthor
#19 interdiff-2851166-16-19.txt1.08 KBGoZ
#19 extends-2851166-19.patch3.06 KBGoZ
#16 interdiff-2851166-13-16.txt1.22 KBGoZ
#16 add_a-2851166-16.patch2.46 KBGoZ
#13 interdiff-2851166-8-13.txt700 bytesGoZ
#13 add_a-2851166-13.patch2.06 KBGoZ
#8 interdiff-2851166-2-8.txt1.6 KBGoZ
#8 add_a-2851166-8.patch1.89 KBGoZ
#2 add_a-2851166-2.patch1.32 KBGoZ
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

GoZ created an issue. See original summary.

GoZ’s picture

GoZ CreditAttribution: GoZ at Centarro commented
Status: Active » Needs review
FileSize
add_a-2851166-2.patch1.32 KB
gaurav.kapoor’s picture

gaurav.kapoor CreditAttribution: gaurav.kapoor at OpenSense Labs commented
Status: Needs review » Reviewed & tested by the community

configuration protected attribute in TipPluginBase is now available in tip plugin interface.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Needs work

I can see how having this method available would be better DX. However, shouldn't it instead extend ConfigurablePluginInterface?

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
gaurav.kapoor’s picture

gaurav.kapoor CreditAttribution: gaurav.kapoor at OpenSense Labs commented

Checked documentation for ConfigurablePluginInterface , other modules such as search are also using this method.Similar should be the case with Tour.

GoZ’s picture

GoZ CreditAttribution: GoZ at Centarro commented

However, shouldn't it instead extend ConfigurablePluginInterface?

I hesitated tout use this interface because that means we also have to implement setConfiguration() and defautConfiguration().

GoZ’s picture

GoZ CreditAttribution: GoZ at Centarro commented
Status: Needs work » Needs review
FileSize
add_a-2851166-8.patch1.89 KB
interdiff-2851166-2-8.txt1.6 KB
gaurav.kapoor’s picture

gaurav.kapoor CreditAttribution: gaurav.kapoor at OpenSense Labs commented
Status: Needs review » Reviewed & tested by the community

Now it looks better.

gaurav.kapoor’s picture

gaurav.kapoor CreditAttribution: gaurav.kapoor at OpenSense Labs commented

Now it looks better.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and at Chapter Three commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/tour/src/TipPluginBase.php
@@ -79,4 +79,32 @@ public function set($key, $value) {
+  /**
+   * {@inheritdoc}
+   */
+  public function setConfiguration(array $configuration) {
+    $this->configuration = $configuration + $this->defaultConfiguration();
+  }

I think this should be:

  /**
   * {@inheritdoc}
   */
  public function setConfiguration(array $configuration) {
    $this->configuration = NestedArray::mergeDeep(
      $this->defaultConfiguration(),
      $configuration
    );
  }

It's just a bit more future proof. And would cope with merging the attributes array if the defaults contained values.

Also the issue summary needs an update since making TipPluginInterface implement \Drupal\Component\Plugin\ConfigurablePluginInterface is a big change - and one that probably needs a CR and some thought about BC layers extra. Maybe Tour UI should extend TipPluginBase and do this itself?

GoZ’s picture

GoZ CreditAttribution: GoZ at Centarro commented

Ok with NestedArray::mergeDeep, i removed this and only use '+' to be make code simpler but it's ok for me to.

Unfortunately, Tour UI cannot do this itself. Right now, in case a custom tip does not provide getConfiguration(), we cannot save a tour.

GoZ’s picture

GoZ CreditAttribution: GoZ at Centarro commented
Status: Needs work » Needs review
FileSize
add_a-2851166-13.patch2.06 KB
interdiff-2851166-8-13.txt700 bytes
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and at Chapter Three commented
Version: 8.3.x-dev » 8.4.x-dev
Issue tags: +Contributed project blocker

@GoZ okay - we need to update the issue summary and create change record for this. And it probably will not make it into 8.3.x since it is an API change.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented 
  1. +++ b/core/modules/tour/src/TipPluginBase.php
@@ -79,4 +80,35 @@ public function set($key, $value) {
+  public function setConfiguration(array $configuration) {

    This should be called from the constructor instead of just doing $this->configuration = $configuration;

    See https://www.drupal.org/node/2851635 

  2. +++ b/core/modules/tour/src/TipPluginBase.php
@@ -79,4 +80,35 @@ public function set($key, $value) {
+    $this->configuration = NestedArray::mergeDeep(
+      $this->defaultConfiguration(),
+      $configuration
+    );

    This should be one line 

  3. +++ b/core/modules/tour/src/TipPluginBase.php
@@ -79,4 +80,35 @@ public function set($key, $value) {
+    return ['id' => '', 'label' => '', 'weight' => 0, 'attributes' => []];

    This should be multiple lines :)

GoZ’s picture

GoZ CreditAttribution: GoZ at Centarro commented
Title: Add a getConfiguration() method to TipPluginBase » Extends TipPluginInterface with ConfigurablePluginInterface
Issue summary: View changes
FileSize
add_a-2851166-16.patch2.46 KB
interdiff-2851166-13-16.txt1.22 KB
GoZ’s picture

GoZ CreditAttribution: GoZ at Centarro commented

Change record has been created https://www.drupal.org/node/2852401

Status: Needs review » Needs work

The last submitted patch, 16: add_a-2851166-16.patch, failed testing.

GoZ’s picture

GoZ CreditAttribution: GoZ at Centarro commented
Status: Needs work » Needs review
FileSize
extends-2851166-19.patch3.06 KB
interdiff-2851166-16-19.txt1.08 KB

Fix configuration set in constructor and add defaultConfiguration() in TipPluginText to add own default configuration attributes.

I see thanks to last failed test that tour still have simpletest tests.

gaurav.kapoor’s picture

gaurav.kapoor CreditAttribution: gaurav.kapoor at OpenSense Labs commented
Status: Needs review » Reviewed & tested by the community
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Title: Extends TipPluginInterface with ConfigurablePluginInterface » Extend TipPluginInterface with ConfigurablePluginInterface
Status: Reviewed & tested by the community » Postponed

We can use the new trait in #2852463: Create a trait and base class to implement \Drupal\Component\Plugin\ConfigurableInterface for this actually! Postponing briefly on that.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented

This extension is being deprecated, see #3336033: [Meta] Tasks to deprecate Tour module. It will be removed from core and moved to a contrib project, #3376099: [11.x] [Meta] Tasks to remove Tour.

This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Project: Drupal core » Tour
Version: 11.x-dev » 1.0.x-dev
Component: tour.module » Code

Moving over but blocking ticket #21 is still not complete after 7 years wonder if we can do something else in contrib.