This seems too obvious, but the book module's outline is not restricted by allowed content types on the node/add form.
To reproduce:
1) Standard profile installation 8.2.7 and enable the Book module.
2) Go to /admin/structure/book/settings and select only the "Article" content type for "Content types allowed in book outlines".
3) Go to /node/add/page to create a new "Basic Page". Notice, the "Book Outline" fieldset is visible although you'd expect the settings in #2 to restrict the Basic Page content type.
Comment | File | Size | Author |
---|---|---|---|
#49 | 2862291-nr-bot.txt | 1.49 KB | needs-review-queue-bot |
#45 | interdiff-2862291-43-45.txt | 2.83 KB | Liam Morland |
#45 | drupal-book_outline-2862291-45.patch | 10.84 KB | Liam Morland |
#43 | 2862291-43.patch | 10.87 KB | balagan |
#40 | 2862291-nr-bot.txt | 144 bytes | needs-review-queue-bot |
Issue fork drupal-2862291
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
empesan CreditAttribution: empesan commentedComment #3
empesan CreditAttribution: empesan commentedReproduced issue and patch added.
Comment #4
ericpughI've tested this patch on Drupal 8.2.7 and it gets rid of the issue, however I'm seeing some (strange) errors in the dblog when creating content.
To reproduce:
1. Follow the steps in the original description, so that the "Article" content type is the only allowed content type in a book outline.
2. Create an article /node/add/article filling out only the Title and Body fields and adding to a book outline.
3. Errors in the dblog: /admin/reports/dblog
I'm not sure how the (attached) errors are related to this patch, but I did tested this on an identical clean install without the patch, and wasn't able to create the error.
Comment #5
Hazapatch rerolled against 8.4.x
I didn't checked about the errors for now.
Comment #7
HazaRelaunched tests against 8.4.x
Comment #8
mathysp CreditAttribution: mathysp at Dazzle commentedI've tested the patch. Seems to work correctly on drupal v8.3. I do not see any errors in my dblog.
This patch will need tests though.
Comment #9
rakesh.gectcrWorking on the test
Comment #10
rakesh.gectcrComment #11
rakesh.gectcrComment #12
mr.baileysThis issue has been raised in the past (see for example #848470: _book_outline_access not restricting allowed types), and the fact that users with "Administer book outlines" permission can add all types to a book is noted on the book settings page:
So this is either a "Closed (works as designed)", or a Feature/Task instead of a bug.
Comment #13
mathysp CreditAttribution: mathysp at Dazzle commentedI think it's worthy enough to consider this as a separate feature. Either way, this case is relevant; As of right now there is no good way to give a user role permission to only edit the book outlines, but still be limited to only the selected content-types.
I think there are two possible improvements that can be made:
1) Split up the permission so it is two-fold;
* Administer book outline:
* Can use all content-types in book outline
2) Add/enable an extra option so we can define wether or not admins can add all CT's or not.
* This can be a radio added to the structure/book/settings page
Meanwhile, we will use the patch from this issue.
Comment #16
jhedstromI'm not sure how this would be considered a feature since sites should be able to disable content types from being in books. Ideally we could update the help text here and then add a change notice.
Also removing the needs tests tag as the latest patch has tests.
Small nit here, the short description should be a single line.
Comment #17
borisson_#16, the latest patch doesn't implement the correct behavior, see #12.
Comment #20
u_tiwari CreditAttribution: u_tiwari commentedWhy is this patch not merged to the core yet? This issue still exists as of Drupal version
Should we just use the patch for now? This is a legitimate use case where we want Book Outline form element to appear only for particular nodes and not all.
Comment #21
kevin.dutra CreditAttribution: kevin.dutra at Workday, Inc. commentedReroll for 8.8.x
Comment #22
wturrell CreditAttribution: wturrell as a volunteer commentedI don't suppose anyone's already using a patch that applies cleanly to 8.7.x?
Comment #24
interdruper CreditAttribution: interdruper at Interdruper commentedComment #27
PCate CreditAttribution: PCate commentedPatch needs a re-roll for Drupal 9.
Comment #28
PCate CreditAttribution: PCate commentedComment #29
MattDanger CreditAttribution: MattDanger commentedRerolled for 8.9.11
Comment #30
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedApplied patch #29 on 8.9.x and it works fine.Adding screenshots
Before:
After:
But the patch can't be applied on 9.2.x,it is showing error
Comment #31
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedComment #32
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedRe-rolled for 9.2.x
Comment #34
adityasingh CreditAttribution: adityasingh as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedFixed test case. Please review the patch.
Comment #35
tstoecklerUnfortunately I don't think this issue can be committed, as allowing other node types is considered a feature. See #502430: Allow the book outline functionality on non-book-enabled node types to be hidden from users with the "Administer book outlines" permission for more information on that and on a possible solution that will still allow people to fix this on their systems while keeping the existing feature for whoever needs it.
I think this should be closed (either as "won't fix" or as a duplicate of the linked issue) but will let someone who has worked on this make the call.
Comment #40
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #42
quietone CreditAttribution: quietone at PreviousNext commentedThis extension is being deprecated, see #3376070: [Meta] Tasks to deprecate Book module. It will be removed from core and moved to a contrib project, #3376101: [11.x] [Meta] Tasks to remove Book.
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.
This issue may be re-opened if it can be considered critical, If unsure, re-open the issue and ask in a comment.
Comment #43
balagan CreditAttribution: balagan at Gizra commentedReroll of patch 2862291-34.patch
Comment #44
balagan CreditAttribution: balagan at Gizra commentedAlso, in the book.module file book_form_node_form_alter() function the 'Change book (update list of parents)' submit button is placed in the forms, if user has 'administer book outlines' permission, disregarding the allowed node type.
After further thinking, with 'add content to book' permission the allowed node type is checked, so probably this is intentional.
Comment #45
Liam MorlandThis is patch #43 with the issues from the automated tests fixed.
There doesn't appear to be any progress towards moving the book module to contrib.
Comment #46
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue.
While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)
Comment #47
Liam MorlandI created an issue fork with patch 45 plus an additional fix. It has not displayed the button to make a merge request.
Comment #48
Liam MorlandComment #49
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #52
ankithashettyOpened an MR for the changes made in #47, thanks!
Comment #53
poker10 CreditAttribution: poker10 at ActivIT s.r.o. commentedI do not think we could easily change this for reasons mentioned in #35 and #12. Changing this will break existing sites already using this feature. So if we want to do this, we should think about existing sites and BC.
On the other hand, I do not think this issue is critical enough, so I suppose it should remain as Postponed, see #42. I see some progress in child issues here: #3376070: [Meta] Tasks to deprecate Book module. Thanks!