Problem/Motivation

The examples are have errors and used deprecated functions
Most of examples does not reflect NG changes.
Examples are highly dependent on comment module and will conflict with #731724: Convert comment settings into a field to make them work with CMI and non-node entities

Proposed resolution

Rewrite examples without comment module related parts

CommentFileSizeAuthor
#31 node-api-clean-up-2030191-31.patch2.62 KBj2r
#25 node-api-clean-up-2030191-24.patch2.62 KBj2r
#24 node-api-clean-up-2030191-23.patch4.09 KBhenk
#19 node-api-clean-up-2030191-19.patch3.97 KBmillerbennett
#17 patch_error.png24.66 KBgippy
#15 drupal-clean-up-api-examples-of-node-module-2030191-15.patch10.35 KBandypost
#14 drupal-clean-up-api-examples-of-node-module-2030191-14.patch2.81 KBishadakota
#11 drupal-clean-up-api-examples-of-node-module-2030191-11.patch2.82 KBinternetdevels
#5 2030191-nodeapi-5.patch2.33 KBforbesgraham
#1 2030191-nodeapi-1.patch2.94 KBandypost

Comments

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
StatusFileSize
new 2030191-nodeapi-1.patch2.94 KB

Initial changes

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: -Documentation

#1: 2030191-nodeapi-1.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2030191-nodeapi-1.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue summary: View changes
Issue tags: +Novice
forbesgraham’s picture

forbesgraham commented
StatusFileSize
new 2030191-nodeapi-5.patch2.33 KB

This patch updates all uses of the user_access function. This is only my third commit to core, so I hope I got it right! :)

forbesgraham’s picture

forbesgraham commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: 2030191-nodeapi-5.patch, failed testing.

tarekdj’s picture

tarekdj commented 
-    if ($op == 'create' && user_access('create ' . $type . ' content', $account)) {
+    if ($op == 'create' && $account->hasPermission('create ' . $type . ' content')) {
+    if ($op == 'create' && \

I think you have to delete the 3rd line from code above.

andypost’s picture

andypost
he/him
Primary language Russian
commented 
+++ b/core/modules/node/node.api.php
@@ -180,8 +181,8 @@
+function hook_node_grants(\Drupal\Core\Session\AccountInterface $account, $op) {

@@ -563,23 +564,24 @@ function hook_node_load($nodes, $types) {
+function hook_node_access(\Drupal\node\NodeInterface $node, $op, \Drupal\Core\Session\AccountInterface $account, $langcode) {

Fill namespace should be added to @param docblock
and use NodeInterface & AccountInterface in arguments

xjm’s picture

xjm
she/her
Primary language English
commented
Component: node.module » node system

(Merging "node system" and "node.module" components for 8.x; disregard.)

internetdevels’s picture

internetdevels commented
Status: Needs work » Needs review
StatusFileSize
new drupal-clean-up-api-examples-of-node-module-2030191-11.patch2.82 KB
andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

Great!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Needs a reroll

error: patch failed: core/modules/node/node.api.php:567
error: core/modules/node/node.api.php: patch does not apply
ishadakota’s picture

ishadakota commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new drupal-clean-up-api-examples-of-node-module-2030191-14.patch2.81 KB

Re-rolled

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new drupal-clean-up-api-examples-of-node-module-2030191-15.patch10.35 KB

Other places should be update as well

Also docs should be cleaned for type-hint to \Drupal\node\NodeInterface so this needs some work to detect what hooks are passing.
Currently file contains a mix of both interfaces

andypost’s picture

andypost
he/him
Primary language Russian
commented

NodeInterface part would be changed in #2067345: Change EntityInterface type hints to NodeInterface where appropriate

gippy’s picture

gippy commented
StatusFileSize
new patch_error.png24.66 KB

Patch 15 no longer matches the current head.

gippy’s picture

gippy commented
Status: Needs review » Needs work
millerbennett’s picture

millerbennett commented
StatusFileSize
new node-api-clean-up-2030191-19.patch3.97 KB

Re-rolled manually

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review

+1 rtbc

henk queued 19: node-api-clean-up-2030191-19.patch for re-testing.

henk’s picture

henk commented
Assigned: Unassigned » henk
Status: Needs review » Needs work

Patch needs re-roll, tested on latest code Drupal 8.0.x.

Checking patch core/modules/node/node.api.php...
Hunk #1 succeeded at 148 (offset 3 lines).
Hunk #2 succeeded at 209 (offset 5 lines).
Hunk #3 succeeded at 257 (offset 5 lines).
error: while searching for:
 *   - "view"
 * @param \Drupal\Core\Session\AccountInterface $account
 *   The user object to perform the access check operation on.
 * @param object $langcode
 *   The language code to perform the access check operation on.
 *
 * @return string

error: patch failed: core/modules/node/node.api.php:313
error: core/modules/node/node.api.php: patch does not apply

The last submitted patch, 19: node-api-clean-up-2030191-19.patch, failed testing.

henk’s picture

henk commented
Assigned: henk » Unassigned
Status: Needs work » Needs review
StatusFileSize
new node-api-clean-up-2030191-23.patch4.09 KB

Ok, patch is rerolled. Should be ok now.

j2r’s picture

j2r commented
StatusFileSize
new node-api-clean-up-2030191-24.patch2.62 KB

Last patch fail with this message

Checking patch core/modules/node/node.api.php...
error: while searching for:
 *
 * @param \Drupal\node\NodeInterface $node
 *   The node being validated.
 * @param $form
 *   The form being used to edit the node.
 * @param $form_state
 *   The current state of the form.
 *
 * @ingroup entity_crud
 */

error: patch failed: core/modules/node/node.api.php:424
error: core/modules/node/node.api.php: patch does not apply

Adding a new patch with minor changes from last patch.

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
Issue tags: +SrijanSprintDay
RavindraSingh’s picture

RavindraSingh commented

@j2r, Patch seems fine to me, but it will be great if you can add interdiff also to show the different between #24 and #25

To know more https://www.drupal.org/documentation/git/interdiff
And do not forget to add you have re-rolled the patch. Also please make sure if the test for the node.api is having the same scenario/comments.

j2r’s picture

j2r commented

@RavindraSingh : I think interdiff is not possible as I am not able to apply the previous patch.

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented

yup, interdiff is not necessary while we are rerolling a patch :)

googletorp’s picture

googletorp commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.

j2r’s picture

j2r commented
StatusFileSize
new node-api-clean-up-2030191-31.patch2.62 KB

Sorry for adding one more file. Just missed a space in last 'langcode'.

@googletorp - Thanks for review and RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed
Issue tags: +Documentation

Documentation changes are permitted during beta. Committed 150dc50 and pushed to 8.0.x. Thanks!

  • alexpott committed 150dc50 on 8.0.x 
    Issue #2030191 by andypost, j2r, forbesgraham, henk, InternetDevels,...

Status: Fixed » Closed (fixed)

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