API page: https://api.drupal.org/api/drupal/core%21core.api.php/group/config_api/8

This page is supposed to give you the basics of the Configuration API.

It was apparently never updated after the getEditable() requirement went in (if you want to alter config, you need to call getEditable() on the config object because the default config object you get is not editable).

So the examples involving changing/saving configuration in the Simple Configuration section of this page need to be updated to use code similar to what you see on
https://www.drupal.org/node/1809490

Should be a fairly straightforward Novice project I think...

CommentFileSizeAuthor
#56 interdiff-2658412-46-54.txt1.25 KBleolandotan
#56 config-api-getEditable-2658412-54.patch2.48 KBleolandotan
#49 interdiff-2658412-46-49.txt1.25 KBleolandotan
#49 config-api-getEditable-2658412-49.patch16.92 KBleolandotan
#46 interdiff-2658412-41-46.txt1.33 KBleolandotan
#46 config-api-getEditable-2658412-46.patch2.34 KBleolandotan
#41 config-api-getEditable-2658412-41.patch2.47 KBleolandotan
#41 interdiff-2658412-38-41.txt848 bytesleolandotan
#38 config-api-getEditable-2658412-33-38-interdiff.txt3.11 KBlokapujya
#38 config-api-getEditable-2658412-38.patch2.48 KBlokapujya
#33 interdiff-2658412-31-33.txt923 bytesleolandotan
#33 config-api-getEditable-2658412-33.patch2.87 KBleolandotan
#31 interdiff-2658412-28-31.txt3.08 KBharsha012
#31 config-api-2658412-31.patch3.08 KBharsha012
#28 interdiff-2658412-22-27.txt3.03 KBleolandotan
#28 config-api-getEditable-2658412-27.patch3.15 KBleolandotan
#23 interdiff-2658412-19-22.txt1.4 KBleolandotan
#22 config-api-getEditable-2658412-22.patch2.35 KBleolandotan
#19 2658412-16-19.txt1.09 KBrakesh.gectcr
#19 2658412-19.patch2.07 KBrakesh.gectcr
#16 interdiff-2658412-13-16.txt994 bytesleolandotan
#16 config-api-getEditable-2658412-16.patch1.98 KBleolandotan
#13 config-api-getEditable-2658412-13.patch1.95 KBthisisit
#9 config-api-getEditable-2658412-9.patch2.15 KBzweishar
#2 config-api-getEditable-2658412-2.patch3.12 KBmiteshmap
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon created an issue. See original summary.

miteshmap’s picture

Status: Active » Needs review
FileSize
3.12 KB

Here is the initial attempt, I hope it's in a right direction. :)

jhodgdon’s picture

Status: Needs review » Needs work

Thanks for the patch!

But this patch actually doesn't fix the problems with the existing code sample from that topic:

// Update a value. Nesting works the same as get().
$config->set('bar.baz', 'string2');
// Nothing actually happens with set() until you call save().
$config->save();

This part of the code sample will not work, because above $config was created using:

$config = \Drupal::config('mymodule.foo');

Also, although the text here is useful, it is much more wordy than what we aim for in the topics for api.drupal.org (which is why we also have documentation pages on drupal.org that can have many more details and be more tutorial-like). What we want in the topic/@defgroup is just the bare minimum information.

So. Instead of this patch, can we:

a) Take out the incorrect part of the code that is currently in the topic (which I just mentioned in this comment), which won't work (the part with -> set and -> save methods on a non-editable config object).

b) Add a *small* explanation (like one sentence, and we don't really need the "mutable" terminology I think) about editable config objects. Then make a small code block with one line to get the editable config object and then put in those two -> set and -> save lines that were already in the topic, but now they will actually work.

c) I think you're right that adding something about delete is also useful, but it could just be one sentence of explanation and about 2 lines of code, I think?

d) Put (b) and (c) into the existing "Simple configuration" section of the topic rather than making new sections.

Thanks!

mikemiles86’s picture

rakesh.gectcr’s picture

Assigned: Unassigned » rakesh.gectcr
rakesh.gectcr’s picture

Assigned: rakesh.gectcr » Unassigned
alvar0hurtad0’s picture

Assigned: Unassigned » alvar0hurtad0

Working on this.

alvar0hurtad0’s picture

Assigned: alvar0hurtad0 » Unassigned

Too much documentation tasks for my poor english.

zweishar’s picture

Reworked initial commit from @miteshmap based on feedback from @jhodgdon.

zweishar’s picture

Status: Needs work » Needs review
cilefen’s picture

Status: Needs review » Needs work
  1. +++ b/core/core.api.php
    @@ -283,35 +283,28 @@
    - * This will be an object of class \Drupal\Core\Config\Config, which has methods
    - * for getting and setting configuration information.  For instance, if your
    - * YAML file structure looks like this:
    - * @code
    - * enabled: '0'
    - * bar:
    - *   baz: 'string1'
    - *   boo: 34
    - * @endcode
    - * you can make calls such as:
      * @code
    - * // Get a single value.
    - * $enabled = $config->get('enabled');
    - * // Get an associative array.
    - * $bar = $config->get('bar');
    - * // Get one element of the array.
    - * $bar_baz = $config->get('bar.baz');
    

    Why throw this example of a settings tree away? Let's use this example in the read-only case then write one of the same settings please.

  2. +++ b/core/core.api.php
    @@ -283,35 +283,28 @@
      * // Update a value. Nesting works the same as get().
      * $config->set('bar.baz', 'string2');
      * // Nothing actually happens with set() until you call save().
      * $config->save();
      * @endcode
    

    This section makes no sense here now.

  3. +++ b/core/core.api.php
    @@ -283,35 +283,28 @@
    + * ¶
    

    random space

  4. +++ b/core/core.api.php
    @@ -283,35 +283,28 @@
    + * @ref sec_yaml above). To change configuration you will need to get an
    + * instance of Config by making a call to getEditable() on the config factory.
    + * You can do so like this:
    

    Should the entire \Drupal\Core\Config\Config class path be used here?

thisisit’s picture

Assigned: Unassigned » thisisit
Issue tags: +#drupalconasia2016
thisisit’s picture

Assigned: thisisit » Unassigned
Status: Needs work » Needs review
Issue tags: -#drupalconasia2016 +drupalconasia2016
FileSize
1.95 KB

Applied the first three changes mentioned by cilefen in #11. Didn't understand fourth point mentioned.

naveenvalecha’s picture

Status: Needs review » Needs work
+++ b/core/core.api.php
@@ -307,11 +303,20 @@
+ * ¶

Extra space.

leolandotan’s picture

Assigned: Unassigned » leolandotan
leolandotan’s picture

Status: Needs work » Needs review
FileSize
1.98 KB
994 bytes

Hi,

Here I removed the random space and added the \Drupal\Core\Config\Config class to the getEditable() method.

Thanks!

cilefen’s picture

Status: Needs review » Needs work
  1. +++ b/core/core.api.php
    @@ -283,13 +283,10 @@
    + * instance of Config by making a call to \Drupal\Core\Config\Config::getEditable()
    

    This line exceeds 80 columns.

  2. +++ b/core/core.api.php
    @@ -283,13 +283,10 @@
    + * You can do so like this:
      * This will be an object of class \Drupal\Core\Config\Config, which has methods
    

    I do not understand how these two sentences fit together.

+++ b/core/core.api.php
@@ -307,11 +304,20 @@
+ * @code
+ * $config = \Drupal::service('config.factory')->getEditable('mymodule.foo');
+ *
+ * Individual configuration values can be unset using the clear() function,

This @code block is not closed and it seems in the wrong place.

leolandotan’s picture

Thank you for the review @cilefen. Will work on this.

rakesh.gectcr’s picture

Assigned: leolandotan » Unassigned
Status: Needs work » Needs review
FileSize
2.07 KB
1.09 KB
cilefen’s picture

+++ b/core/core.api.php
@@ -283,14 +283,11 @@
+ * @ref sec_yaml above). To change configuration you will need to get an
+ * instance of Config by making a call to
+ * \Drupal\Core\Config\Config::getEditable() on the config factory.
+ * You can do so like this:
+ * Create an object of class \Drupal\Core\Config\Config, which has methods
  * for getting and setting configuration information.  For instance, if your

The same thing is said twice here in different ways. I call ::getEditable() or create and object of class Config. Which is it? (I know the answer, but developers reading this will be confused).

lokapujya’s picture

+++ b/core/core.api.php
@@ -283,14 +283,11 @@
  * @code
@@ -307,11 +304,21 @@

@@ -307,11 +304,21 @@
  * $bar = $config->get('bar');
  * // Get one element of the array.
  * $bar_baz = $config->get('bar.baz');
- * // Update a value. Nesting works the same as get().
- * $config->set('bar.baz', 'string2');
- * // Nothing actually happens with set() until you call save().
- * $config->save();
+ *
+ * @code
+ * $config = \Drupal::service('config.factory')->getEditable('mymodule.foo');
+ * @endcode

Isn't this one big singualar code block?

leolandotan’s picture

Thanks for the additional patch and reviews.

@cilefen, I have tried to minimize the redundancy of the said class. I hope it's better.
@lokapujya, I have merged the last line of code into the previous code block.

Hope everything are in order. Thanks!

leolandotan’s picture

Here is the interdiff for comment #22.

leolandotan’s picture

Assigned: leolandotan » Unassigned
lokapujya’s picture

Status: Needs review » Needs work

For next patch, please follow comment #3 (specifically part b.). Use https://www.drupal.org/node/1809490 as a reference.
The idea is that the first part doesn't actually NEED to call getEditable() since nothing is saved. It just does:
$config = \Drupal::config('mymodule.foo');

The edit and save come after:
$config = \Drupal::service('config.factory')->getEditable('mymodule.foo');
(the call to getEditable(), edit(), and save() can maybe be in it's own code block with one line of explanation text before it.)

lokapujya’s picture

+++ b/core/core.api.php
@@ -301,18 +296,26 @@
+ * @code
+ * $config = \Drupal::service('config.factory')->getEditable('system.performance');
+ * $config->clear('cache.page.max_age')->save();
+ * $page_cache_data = $config->get('cache.page');

This code example doesn't have the .yml example that goes with it from the API docs. Not sure what to do about that.

leolandotan’s picture

Assigned: Unassigned » leolandotan
leolandotan’s picture

Status: Needs work » Needs review
FileSize
3.15 KB
3.03 KB

Thank you for the review and feedback @lokapujya!

Things done:

  • Followed @lokapujya recommendation to base the documentation updates from comment #3 to get the editable object, setting and saving it.
  • Used Simple Configuration API as a reference for the explanations and codes.
  • Changed some explanations for the code blocks
  • Added a sample "configuration data" as an example when unsetting configurations.
leolandotan’s picture

Assigned: leolandotan » Unassigned
lokapujya’s picture

Status: Needs review » Needs work

Almost there.

  1. +++ b/core/core.api.php
    @@ -307,12 +305,50 @@
    + * \Drupal::service('config.factory')->getEditable('system.performance');
    ...
    + * $config = \Drupal::service('config.factory')->getEditable('system.performance');
    ...
    + * $config = \Drupal::service('config.factory')->getEditable('system.performance');
    

    repeated 3 times. If the first code block shows how to get an editable config object (with the "$config = "), should the next 2 code blocks assume you've already done that?

  2. +++ b/core/core.api.php
    @@ -307,12 +305,50 @@
    + * @encdcode
    

    should be @endcode

  3. +++ b/core/core.api.php
    @@ -307,12 +305,50 @@
    + * then to unset a value is done in the following way:
    

    This sentence doesn't sound right. I think we can take out the new text from "For the system..." down to and including this line. Also we can remove the "In this example..." sentence. Those maybe belong in the tutorial but not in the code comments?

harsha012’s picture

Status: Needs work » Needs review
FileSize
3.08 KB
3.08 KB

Added patch as per comment #30

lokapujya’s picture

Status: Needs review » Needs work

Thanks! That fixed #30.1 and #30.2. The interdiff was not really an interdiff, it was the same as the patch.

#30.3 still needs work.

+++ b/core/core.api.php
@@ -307,12 +305,49 @@
+ * For the system performance settings in system.performance.yml, we can unset
+ * the previous code block, the configuration would look like:

This sentence still doesn't make sense. a.) "For the system performance" is repeated from a sentence above, b.) we are not unsetting the previous code block, c.) it's not configuration that follows (it's code.) We could say, "We can unset configuration with the following code:"

Or, Perhaps we just go with what's on the doc page?:

Individual configuration values can be unset using the clear() function, which is also chainable.

leolandotan’s picture

Assigned: Unassigned » leolandotan
Status: Needs work » Needs review
FileSize
2.87 KB
923 bytes

I have tried to work on the explanations to keep them a bit shorter and direct to the point to address comment #30 item 3 and the feedback on #32.

Thanks!

leolandotan’s picture

Assigned: leolandotan » Unassigned
lokapujya’s picture

Looks good to me. RTBC for me, but let's get a last look from someone else.

jhodgdon’s picture

Status: Needs review » Needs work

Well, it's definitely better than the last time I looked at it, and thanks for all the patches and reviews!

However, it still needs some reworking before I would be ready to add it to the docs:

  1. +++ b/core/core.api.php
    @@ -283,16 +283,14 @@
    - * @ref sec_yaml above). Once you have done that, you can retrieve the
    - * active configuration object that corresponds to configuration file
    - * mymodule.foo.yml with a call to:
    + * @ref sec_yaml above). To change configuration you will need to get an
    + * instance of Config by making a call to:
      * @code
      * $config = \Drupal::config('mymodule.foo');
      * @endcode
    

    This change is incorrect.

    If you want to *change* config, you have to do what the part of the patch added below does to get a getEditable(), not this.

    So you need to change these 3 lines back to what they were.

  2. +++ b/core/core.api.php
    @@ -283,16 +283,14 @@
      * This will be an object of class \Drupal\Core\Config\Config, which has methods
    - * for getting and setting configuration information.  For instance, if your
    - * YAML file structure looks like this:
    + * for getting and setting configuration information. For instance, if your YAML
    + * file structure looks like this:
    

    I think here we should also take out the "and setting" part, because although yes you can set things, you can't save the changes, which kind of defeats the purpose. So can we just say "getting" here and take out "and setting"?

  3. +++ b/core/core.api.php
    @@ -307,12 +305,46 @@
    + * To change a configuration you will need to get an instance of
    

    So here I think this needs a bit more explanation that we are starting over. Something like:

    The Config object that was obtained and used in the previous examples does not allow you to change configuration. If you want to change configuration, you will instead need to...

  4. +++ b/core/core.api.php
    @@ -307,12 +305,46 @@
    + * factory.
    

    Before @code samples, end the line in :

    Please check on this throughout the patch.

  5. +++ b/core/core.api.php
    @@ -307,12 +305,46 @@
    + * $config =\Drupal::service('config.factory')->getEditable('system.performance');
    

    Please use the same example as in the code above, of 'mymodule.foo' so that it all is similar and it does not depend on a particular Core config to be defined. Also I don't really think we need both an array and a non-array set() example. One is enough. This is supposed to be bare minimum docs, not comprehensive.

  6. +++ b/core/core.api.php
    @@ -307,12 +305,46 @@
    + * $page_cache_data = array('enabled' => 1, 'max_age' => 5);
    + * $config->set('cache.page', $page_cache_data);
    

    If you do use an array example, please use the [] array notation not array(), which we are gradually getting rid of, and try to keep things compact (like we don't really need this local variable when the array value can be passed into set() directly).

  7. +++ b/core/core.api.php
    @@ -307,12 +305,46 @@
    + * // Save your data to the file system.
    

    ????? TO the *file system*??? No. It is not saved to the file system. Just say "Save the configuration." or something like that.

  8. +++ b/core/core.api.php
    @@ -307,12 +305,46 @@
    + * which is also chainable. Given the following configuration definition:
    

    Again, please use the mymodule.foo example that was already defined for this section, rather than introducing a new one.

    And... Really, adding all of this stuff about delete() and clear() is out of scope for this issue, and adds too much bulk to this page IMO. We should just be giving the bare minimum in api.drupal.org topics. and linking to drupal.org pages for more details. So please take that stuff out. Thanks.

lokapujya’s picture

Assigned: Unassigned » lokapujya

Will work on #36.

lokapujya’s picture

Assigned: lokapujya » Unassigned
Status: Needs work » Needs review
FileSize
2.48 KB
3.11 KB

Addressed all points in #36.

lokapujya’s picture

+++ b/core/core.api.php
@@ -307,43 +308,30 @@
+ * $config_data = $config->get('bar.boo');

Oops, this should be 'bar', not 'bar.boo'.

jhodgdon’s picture

Status: Needs review » Needs work

Almost, thanks!

  1. +++ b/core/core.api.php
    @@ -307,12 +306,33 @@
    + * $config =\Drupal::service('config.factory')->getEditable('system.performance');
    

    This still needs to say 'mymodule.foo' here, not 'system.performance'.

  2. +++ b/core/core.api.php
    @@ -307,12 +306,33 @@
    + * $config->clear('bar.boo')->save();
    + * $config_data = $config->get('bar.boo');
    + * @endcode
    + * In this example $config_data would return an array with one key - 'baz' -
    + * because 'boo' was unset.
    

    This doesn't make sense to me. WE clear bar.boo, then get bar.boo, then the text below says it still contains something? Something is wrong here.

    Oh, yeah, you figured that out in #39. Needs fixing.

leolandotan’s picture

Assigned: Unassigned » leolandotan
Status: Needs work » Needs review
FileSize
848 bytes
2.47 KB

Thanks @lokapujya and @jhodgdon. Here I have applied the changes specified on #39 and #40.

leolandotan’s picture

Assigned: leolandotan » Unassigned
lokapujya’s picture

Thank You @leolando.tan too! Nice patch.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Looks good now, thanks all!

catch’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/core.api.php
@@ -307,12 +306,33 @@
+ * Configuration is changed or added using the set() method and saved using the

This should be'Individual configuration values' as below I think.

leolandotan’s picture

Assigned: Unassigned » leolandotan
Status: Needs work » Needs review
FileSize
2.34 KB
1.33 KB

Thank you for the reviews and feedback! I also understood what @catch is referring to. So I:

  • Restructured the doc a little, separated the individual steps to somehow define them clearer.
  • Updated the "clear() function" phrase to "clear() method" to make them uniformed with the phase used for set() and save().

Here I have included an updated patch and interdiff file.

leolandotan’s picture

Assigned: leolandotan » Unassigned
jhodgdon’s picture

Status: Needs review » Needs work

Quick question:

+++ b/core/core.api.php
@@ -283,16 +283,15 @@
  * $config = \Drupal::config('mymodule.foo');
  * @endcode
- *
  * This will be an object of class \Drupal\Core\Config\Config, which has methods

Why are we deleting this blank line? It seems like a new paragraph here is a good idea.

Also... I don't think I like the rearrangement here. The save() docs got moved to the end, so it's way far away from the set() call and could be missed. Also, the clear() section already talks about save(). Please put it back more like it was.

leolandotan’s picture

Assigned: Unassigned » leolandotan
Status: Needs work » Needs review
FileSize
16.92 KB
1.25 KB

Thanks for the review @jhodgdon!

Here I have added back the blank line to add the paragraph. My bad I didn't notice the other save() chained with the clean() method. So I placed placed the codes back to where they were and just added some additional details to the first "individual configuration" paragraph. Hope this makes more sense.

Thanks!

leolandotan’s picture

Assigned: leolandotan » Unassigned
jhodgdon’s picture

Status: Needs review » Needs work

The patch looks fine, thanks! Except that most of the changes in the patch are changes that seem to be for other issues. Please make a new patch that only contains the changes for this issue. Thanks!

jhodgdon’s picture

Version: 8.0.x-dev » 8.2.x-dev
Issue tags: +needs backport to 8.1.x, +needs backport to 8.0.x

Also setting this to 8.2.x, with backport tags.

leolandotan’s picture

Oh my! My bad. I'll fix this. Should I still branch out of 8.0.x for the patch that I'll submit?

cilefen’s picture

I asked @xjm for details, and she pointed me to https://groups.drupal.org/node/508968. For this one, it is marked for backport, so make the patch against 8.2.x please.

leolandotan’s picture

Assigned: Unassigned » leolandotan

Thanks for the confirmation @cilefen.

leolandotan’s picture

Status: Needs work » Needs review
FileSize
2.48 KB
1.25 KB

Here I'm working with the patch from #46 as #49's patch is wrong. So please neglect #49.

I have applied patch from comment 46 to 8.2.x and was applied cleanly, applied the recommendations of @jhodgdon from comment #48. Hope everything is in order.

Sorry my patch comment number is not right. :( If it needs to be corrected, I'd be willing to fix it.

Thanks!

leolandotan’s picture

Assigned: leolandotan » Unassigned
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Patch numbers are, in my opinion, not very important that they exactly match the comment number, so don't bother to make a new one just for that!

Anyway... this patch looks fine now, thanks!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed aff19bd and pushed to 8.0.x, 8.1.x and 8.2.x. Thanks!

  • alexpott committed 68a1fbc on 8.2.x
    Issue #2658412 by leolando.tan, lokapujya, rakesh.gectcr, harsha012,...

  • alexpott committed b07c29b on 8.1.x
    Issue #2658412 by leolando.tan, lokapujya, rakesh.gectcr, harsha012,...

  • alexpott committed aff19bd on 8.0.x
    Issue #2658412 by leolando.tan, lokapujya, rakesh.gectcr, harsha012,...

Status: Fixed » Closed (fixed)

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