We have been talking about this for a while but this has most recently emerged from the discussion in #1834828: Add ability to create view snapshots.

We should abstract the usage of $entityInfo['config_prefix'] in to a getConfigPrefix method; This allows this to easily be overridden by other storage controllers if needed, as well as being able to add additional logic to the value returned too.

CommentFileSizeAuthor
#3 1849792-2.patch3.74 KBdamiankloip
#1 1849792.patch3.74 KBdamiankloip

Comments

damiankloip’s picture

damiankloip commented
Status: Active » Needs review
StatusFileSize
new 1849792.patch3.74 KB

Here is a new patch, I have added the start of some unit tests (only tests the getConfigPrefix() method currently). I also added getConfigPrefix usage in the save() method.

Unfortunately we can't use this when getting the manifest file (yet?) as we don't want a '.' appended to the prefix.

dawehner’s picture

dawehner
Primary language German
commented

The actual code is looking fine, just found some small nitpicks.

+++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityUnitTest.phpundefined
@@ -0,0 +1,43 @@
+ * Definition of Drupal\config\Tests\ConfigEntityUnitTest.

Contains :)

+++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityUnitTest.phpundefined
@@ -0,0 +1,43 @@
+  function testStorageControllerMethods() {

Sshould be probably be public.

damiankloip’s picture

damiankloip commented
StatusFileSize
new 1849792-2.patch3.74 KB

Yeah, both good points :)

xjm’s picture

xjm
she/her
Primary language English
commented

See also:
#1760358: Provide a way to extract the ID from a config object name
#1831774: Config import assumes that 'config_prefix' contains one dot only

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Component: configuration system » configuration entity system
Status: Needs review » Reviewed & tested by the community

This looks great.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Makes sense. Committed/pushed to 8.x.

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