Attached patch converts the nodeFiles action from the file resource into a relationship on the node resource. Will be following up with the same for loadNodeComments most likely.

More API changes ftw.

CommentFileSizeAuthor
#5 1051158.patch6.12 KBgdd
#4 1051158.patch6.19 KBgdd
#1 1051158.patch6.3 KBgdd

Comments

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
commented
Status: Active » Needs review
StatusFileSize
new 1051158.patch6.3 KB

Here it is. Also fixes a bug in the old file resource when the field existed but had no uploads.

kylebrowning’s picture

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

Works great, commit it!

voxpelli’s picture

voxpelli commented

Some feedback:

+++ resources/node_resource.inc	4 Feb 2011 15:24:45 -0000
@@ -105,6 +105,33 @@ function _node_resource_definition() {
+          'access callback' => '_node_resource_access',

Different access method than for the old callback - is this intentional?

+++ resources/node_resource.inc	4 Feb 2011 15:24:45 -0000
@@ -105,6 +105,33 @@ function _node_resource_definition() {
+              'optional' => FALSE,

Why not optional?

+++ resources/node_resource.inc	4 Feb 2011 15:24:45 -0000
@@ -339,3 +366,47 @@ function _node_resource_access($op = 'vi
+            module_load_include('inc', 'services', 'resources/file_resource');

Put this outside of the foreach loop?

+++ resources/node_resource.inc	4 Feb 2011 15:24:45 -0000
@@ -339,3 +366,47 @@ function _node_resource_access($op = 'vi
+    return services_error(t('There are no files on given node.'), 406);

I understand that this comes from the old function - but why are we returning an error code when no files are present? An empty response with a 2xx code should be returned instead I think - but perhaps such a change should be made in another patch?

Powered by Dreditor.

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 1051158.patch6.19 KB

1) In normal Drupal functioning, whether or not you can view a node's files is controlled by whether or not you can view the node. So I thought it made more sense this way.

2) Fixed

3) Fixed

4) Concur, just review a blank array now.

Nice catches!

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
commented
StatusFileSize
new 1051158.patch6.12 KB

Here is another reroll with the include removed from the outer loop as well as the inner one

kylebrowning’s picture

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

Tested this morning, looks good.

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
commented
Version: 7.x-3.x-dev » 6.x-3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

OK this is committed, switching to 6.x for backport

kylebrowning’s picture

kylebrowning commented
Status: Patch (to be ported) » Fixed

Status: Fixed » Closed (fixed)

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