Posted by chriscalip on July 27, 2012 at 8:54pm
10 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | book.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | API change, Config novice, Configuration system |
Issue Summary
Convert book settings to the new configuration system
The book settings at :
admin/content/book/settings
admin/structure/block/manage/book/navigation/configure aka hook_block_configure
Tasks
Create a default config file and add it to the module.
Convert the admin UI in core/modules/book/book.admin.inc core/modules/book/book.module to read to/write from the appropriate config.
Convert any book code that currently accesses the variables used to use the config system.
Write an upgrade path from D7 -> D8 to migrate existing data to the new system and drop the appropriate variables.
This would be a good task for someone wanting to get up to speed on how the new config system works. That would be me, chriscalip.
Comments
#1
first try.
#2
#3
+++ b/core/modules/book/book.installundefined@@ -93,3 +86,16 @@ function book_schema() {
\ No newline at end of file
Need to add a newline here.
Great work!
#4
a.) per #3 drupal coding standards compliance code change, adding new line.
b.) variable name changes; updated mapping name to correct one from 'all_pages' to intended 'block_mode'
#5
The last submitted patch, drupal8.config-book-settings.4.patch, failed testing.
#6
Chris, two things
1. you need a config directory with your yml file in the patch
2. you should look at the book tests and see if there are things you need to change in there for handling these variables. A find and replace on the variable_get and variable_sets in there.
#7
Chris,
Yeah my git diff patch didnt take into account the added new file.
adding now..
#8
+++ b/core/modules/book/config/book.settings.ymlundefined@@ -0,0 +1,4 @@
\ No newline at end of file
No newline
#9
adding line.
#10
#11
+++ b/core/modules/book/config/book.settings.ymlundefined@@ -0,0 +1,6 @@
+
one newline to much
Besides of that this looks good
-29 days to next Drupal core point release.
#12
removed a line too far.
#13
#14
+++ b/core/modules/book/config/book.settings.ymlundefined@@ -0,0 +1,5 @@
+
Looks like there still is a line to much... damnit, srry for that. It just must end with a newline so if you remove all the trailing lines and press enter after "all pages" it should be fine
-29 days to next Drupal core point release.
#15
no worries, i'm just not used to yml on my ide.. spacing dyslexia
#16
+++ b/core/modules/book/book.admin.inc@@ -32,12 +32,13 @@ function book_admin_overview() {
+function book_admin_settings($form, $form_state) {
&$form_state always needs to be taken by reference. :)
+++ b/core/modules/book/book.module@@ -284,7 +284,7 @@ function book_block_view($delta = '') {
+ if (config('book.settings')->get('block_mode') == 'all pages') {
I think I'd expect to find this in the key block.navigation.mode
#17
Lets see...
#18
Really, really minor nitpick...
+++ b/core/modules/book/book.admin.incundefined@@ -66,6 +68,18 @@ function book_admin_settings_validate($form, &$form_state) {
/**
+ * Form submission handler for book_admin_settings().
+ *
+ * @see book_admin_settings()
+ */
+function book_admin_settings_submit($form, &$form_state) {
+ config('book.settings')
+ ->set('allowed_types', $form_state['values']['book_allowed_types'])
+ ->set('child_type', $form_state['values']['book_child_type'])
+ ->save();
@see book_admin_settings() is superflous here
#19
actually it has to point to the validate function: http://drupal.org/node/1354#forms
#20
Here's the change from @see book_admin_settings() to @see book_admin_settings_vaidate()
#21
+++ b/core/modules/book/book.installundefined@@ -93,3 +86,16 @@ function book_schema() {
+function book_update_8000() {
+ update_variables_to_config('book.settings', array(
+ 'book_allowed_types' => 'allowed_types',
+ 'book_child_type' => 'child_type',
+ 'book_block_mode' => 'block_mode',
+ ));
Need to update
'block_mode'to the new config key'block.navigation.mode'#22
Updated.
#23
Looks good to go! Thanks for all the work!
#24
Corrected update hook docblock to use the imperative as per our standard:
http://drupal.org/node/1354#hookimpl
#25
Keep forgetting to attach interdiffs. :)
And, the name of this account is
no_commit_credit. Which means, I did not actually contribute to this patch, so please don't mention the account in the commit message.#26
Committed to 8.x.
#27
Thanks!
#28
Automatically closed -- issue fixed for 2 weeks with no activity.