Here's my patch possibly solving the support-feature I posted earlier

As I already mentioned in the original post, the patch is straight forward and I am not sure if it suffices the php/Drupal coding standards (I am not a php programmer). However it works for me...

Please review.

Florian

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

flier’s picture

Category: Bug report » Feature request

Status: Needs review » Needs work

The last submitted patch, multi-value-field.patch, failed testing.

flier’s picture

New patch, this time I read the Drupal FAQs about patching. Sorry for that.

flier’s picture

flier’s picture

Issue summary: View changes
marcingy’s picture

Version: 7.x-3.7 » 7.x-3.x-dev
Status: Needs work » Needs review
marcingy’s picture

Status: Needs review » Needs work

The patch needs brought upto drupal coding standards see https://drupal.org/coding-standards

Also

$form_state['field'][$key]['und']['items_count'] = $value;

This should be LANGUAGE_NONE instead of und in this case, but a more significant issue is that the patch won't work with sites that have translations enabled.

marcingy’s picture

Issue tags: -multi-value fields, -fields, -node create
flier’s picture

The patch needs brought upto drupal coding standards see https://drupal.org/coding-standards

Also

$form_state['field'][$key]['und']['items_count'] = $value;
This should be LANGUAGE_NONE instead of und in this case, but a more significant issue is that the patch won't work with sites that have translations enabled.

The language issue could be solved by, i.e,

$lang = $form_state['language']
$form_state['field'][$key][$lang]['items_count'] = $value;

However, I also ran into this problem when updating a node, I basically used the same method
as proposed in my patch. If someone is willing to make the new patch coding standards compatible...
I will share it, of course.

mzgajner’s picture

In case this helps anyone:

I had the same issue with multi-value fields. It appeared when upgrading from 3.5 to 3.7. I tried the patch from this thread, which worked (multi value fields worked again), but it broke some other things.

The node being created with Services did not respect the default value of a specific taxonomy field that was set on the content type. I do not pass any value for this field when I do the post and without the patch, the general default value was used. With the patch, no value is passed and stored. My site has translations enabled, so it might have something to do with it.

For now, I reverted to 3.5 where everything works.

mccrodp’s picture

Hi,

Thank you for your post Matek. I am using Drupal 7.27 and Services 3.7 to upload a JSON node via CURL referencing existing files within Drupal. It was not working as explained in this issue. I applied the patch and it was still not working still not working.

I then downgraded to Services 3.5 without applying the patch and it worked. I have since tested Services 3.6, 3.7 and current dev without the patch, and none of them work.

Did something break between 3.5 and 3.6?

I will stick with 3.5 for the moment as both file and image are working for me.

Thank you.

{
  "node": {
	"title": "Testing testing...",
    "type": "page",
    "body": {
		"und": [
		  {
			"value": "This is body content!",
			"summary": "",
			"format": "svg"
		  }
		]
	},
    "field_image": {
		"und": [
		  {
			"fid": "58"
		  },
		  {
		  	"fid": "56"
		  },
		  {
		  	"fid": "60"
		  }
		]
	},
	"field_file": {
		"und": [
		  {
			"fid": "58"
		  },
		  {
		  	"fid": "56"
		  },
		  {
		  	"fid": "60"
		  }
		]
	}
  }
}
mnshantz’s picture

When we went from Services 3.5 to 3.7 we also had an issue with it not working. I was able to trace it back to commit 0e39f46, specifically when this was added:
$stub_node = (object) array_intersect_key($node, array_flip(array('type', 'language')));
$stub_node->name = $user->name;
drupal_form_submit($node_type . '_node_form', $form_state, (object)$stub_node);
And this removed:
drupal_form_submit($node_type . '_node_form', $form_state, (object)$node);

By reverting back to the "non" stub_node everything began working the same way it did.

streiten’s picture

Same issue here. (drupal 7.26 / services 3.7)
I am trying to populate a unlimited multi-value field with color field module/widget (https://drupal.org/project/color_field) for existing nodes by means of a cURL PUT request.
$data_string = '{ "field_colors": { "und":[ { "rgb":"#AAAFFF" },{ "rgb":"#000111" },{ "rgb":"#FBFBFB" } ] }}';

neither the patch nor #12 work!

if the fields are already there updating works as expected.
issuing the request repeatedly seems to add the fields though.
so i guess i am stuck PUTing in a loop for now.

Atomox’s picture

This is just a guess, but I know when AJAX adding fields are in the picture, Drupal checks the form array to confirm that if a value was passed, the field existed beforehand. This is a security measure, which keeps AJAX from inserting malicious data that was not expected by Drupal. I'm seeing this issue via services. Only the first field in any given taxonomy term (unlimited field) is added via node create. It doesn't matter which valid value is passed, it only adds the first one.

This would explain why bzoo (in #13) is only seeing updates work when the fields already exist. I'll bet all the money in my pockets that if bzoo tries editing his node, and adding 2 more fields, that only the first (if any) new field will be added.

Again, this will occur when you have unlimited selected, but only allow 1 tag/reference/etc per field. This should not happen for multi-select fields, or free tagging.

Watching this one, as I suspect this is a problem with implementing using the Form API. I'm hoping I'm wrong, as I'd love to see a simple fix.

marcingy’s picture

zarexogre’s picture

#3 worked a treat thanks for this

kylebrowning’s picture

This is needs work as it needs tests, but would love to see this in Services to help the ailment of adding values to multi-value fields.

mrfelton’s picture

Status: Needs work » Needs review
FileSize
3.46 KB

Here is a tidied up patch based on #3 that also enables support for multivalue fields on the user resource.

marcingy’s picture

Status: Needs review » Needs work

This code hunk is repeated

+  // Save key and value pairs to be expanded on drupal_form_submit() later on
+  $tmp_vals = array();
+  foreach ($form_state['values'] as $key => $value) {
+    if (preg_match('/^field_/', $key)) {
+      $num = count($form_state['values'][$key]['und']);
+      if ($num >= 2) {
+        $tmp_vals[$key]=$num;
+      }
+    }
+  }

Lets move it into a function that can be called. Also

$tmp_vals[$key]=$num;

needs spaces round = to meet code standards

Extra white space here

// Increment items_count if more than	one value is to	be added
+  if ( count($expand_values) > 0 ) {

And missing newline at end of file.

ParisLiakos’s picture

#18 works for me for multiple entityreference fields, thanks

daekano’s picture

Same problem as #13. We have a high volume site, looping PUTs is simply not an OPTION (heh).

Is this being represented as an issue with the Form API?

cudevdev’s picture

The documentation should include some information on how to submit a multivalue field.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 18: services-create-multivalued-field-nodes-2224803.18-D7.patch, failed testing.

The last submitted patch, 3: services-create-multivalued-field-nodes-2224803-D7.patch, failed testing.

rynebl’s picture

Similar code should also be applied to the update function, as it is has the same issue as create node, hopefully this helps others trying to solve this.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 18: services-create-multivalued-field-nodes-2224803.18-D7.patch, failed testing.

firoz2456’s picture

I am using Drupal 7.37 and Services 3.12. Patch #3 and Patch #18 is not working for me. But downgrading services version from 3.12 to 3.5 is working fine. @mzgajner thanks for your comment.

Shiraz Dindar’s picture

Not sure on what the status of the patch is in #18. We simply applied the patch from #12 which undoes what was done in 3.6 and multivals work fine again. Perhaps someone can confirm that #12 is safe. I am unclear on the purpose of the stub node.

pebosi’s picture

Removing

$stub_node = (object) array_intersect_key($node, array_flip(array('type', 'language')));
$stub_node->name = $user->name;

and changing this

$stub_form = drupal_get_form($node_type . '_node_form', (object) $stub_node);
$form_state['triggering_element'] = $stub_form['actions']['submit'];

drupal_form_submit($node_type . '_node_form', $form_state, (object) $stub_node);

to

$stub_form = drupal_get_form($node_type . '_node_form', (object) $node);
$form_state['triggering_element'] = $stub_form['actions']['submit'];

drupal_form_submit($node_type . '_node_form', $form_state, (object) $node);

works for me...

Psycle Interactive’s picture

Wondering on the state of this issue, as currently any field that is defined to allow an unlimited number of values does not work when creating/editing a node through the services module.

For example:
I have a field called 'subpages', this is configured as:
Field type - Text
Widget - Text field
Number of values - unlimited

When i try to create a now with this field through services, it only saves the first value and ignores any others set, even though the field is defined as unlimited.
I am sending the following for the subpages field in the post data:
field_subpages[und][0][value]=value1&field_subpages[und][1][value]=value2&field_subpages[und][2][value]=value3

briantes’s picture

I have the same issue. I have applied the patch #18 and the suggestion from pebosi, but I doesn't work.
I can't populate a field with an unlimited values allowed.
I have 3.12 version. Then, must I downgrade to 3.5 in order to get it working? There is a lot of fixed bugs and improvement from 3.5 to 3.12 and I want it.
Is there any other patch or solution? Thanks

Psycle Interactive’s picture

I have created the following patch against the 7.x-3.x branch, which also works on the 7.x-3.12 release. This fixed the issue i was having with unlimited multivalue fields not saving more than 1 value when creating nodes. The update node action still works and i have tested adding more values and removing values.

I hope this will prompt the fix to be rolled into a future release.

kylebrowning’s picture

Status: Needs work » Needs review
gkucsko’s picture

Just tested #35. It seems like it indeed works, such that you can now create an additional entry in the file field array. However it is still limited to 1 new entry per request.

Psycle Interactive’s picture

@gkucsko

Just to clarify what you are saying, is there still an issue with File/Image field types that are defined as unlimited/multiple? If so, i will try to replicate and see if i can fix it.

gkucsko’s picture

yes, though it seems to be a general issue with the service module that you can only add one item at a time to a multi-valued field (probably related to drupal form management). see also here: https://www.drupal.org/node/1985390

peterlolty’s picture

I use Services Entity API module to deal with the multi-valued field, no extra code/hook is added, it works in native,
I think the correct format of the POST data is the key of getting it done.

For example, I have a entity reference field which is multi-value (set to unlimited)
what i need to POST the data are as follows,

NOT Url encoded
any_thing_else_key=any_thing_else_value&borrow_record_multiple[0]=5284&borrow_record_multiple[1]=5283&any_thing_else_key=any_thing_else_value

With Url encoded
any_thing_else_key=any_thing_else_value&borrow_record_multiple%5B0%5D=5284&borrow_record_multiple%5B1%5D=5283&any_thing_else_key=any_thing_else_value

And the C# code are as follows

NameValueCollection nvc = new NameValueCollection();
nvc.Add("any_thing_else_key", "any_thing_else_value");
for (int i = 0; i < returnSelectedList.Length; i++)
{
    nvc.Add("borrow_record_multiple["+i+"]",  returnSelectedList[i].ID + "");
}
restReqID = getCurrentTimeStamp();
rest.EntityCreate<DrupalMyCustomEntity>(restReqID, nvc);

And the result are as follows

......
"borrow_record_multiple": [2]
0:  {
"uri": "http://192.168.1.20/vtcdemo/rest/entity_borrowrecord/5284"
"id": "5284"
"resource": "borrowrecord"
}-
1:  {
"uri": "http://192.168.1.20/vtcdemo/rest/entity_borrowrecord/5283"
"id": "5283"
"resource": "borrowrecord"
}
........

However, I am dealing with my custom entity, not node, so you may try entity_node instead of node after Services Entity API installed.

Version I am using,
Services 7.x-3.12
Services Entity API 7.x-2.0-alpha8

Cheers!

Anonymous’s picture

Unfortunately none of the patches have allowed my to upload more than file to a multi-value field. Not only for unlimited cardinality, but even for fields with values between 2-10 as well. Only one file is saved, delta 0, overwriting what was there before. Other files after the first in the JSON data object sent to the resource are ignored.

Where is there any documentation regarding how to upload files to multi-value fields with Services? peterlolty's method doesn't work since I am using JSON, not URL encoding.

kylebrowning’s picture

#32 seems to be the official way.

Anonymous’s picture

Tried that, couldn't get it to work for a multi-value image field on a node. Discovered the targeted action allowing file attachments directly to nodes. I got that to work after a lot of wrangling trying to get AngularJS to get the data in the right format for multipart/form-data Content-Type. It would have been so much easier if this targeted action accepted base64 data & Content-Type: application/json. Anyway, I never got this to work, so I can't contribute anything. Hopefully somehow this gets sorted out.

peterlolty’s picture

I also tried it on JSON, doesnt work.
But if you are using URL encoding as the request content type, you can try my posted method as I am currently using this.

norbertwolfgraf’s picture

#32, #35 worked like a charm. Thank you so much. Works on services 7.x-3.13 for 'number of values' set to unlimited and 1-10.

jerrac’s picture

#35 seems to have worked for me on 3.14.

hanoii’s picture

#35 worked for me for adding, but I cannot figure out how to remove an item, @PsycleInteractive, you mentioned removal works, how do you do that?

hanoii’s picture

Now that I tried better, I got to remove a field collection items by setting all of the values as blank, however, a PUT (update) doesn't seem to create new elements of the multiple valued field collection, creates the first one. I attempted a similar fix without success.

Chipie’s picture

After applying patch #35 the nodes created by services are unpublished by default. Anyone knows why?

Chipie’s picture

#35 can't be applied to 7.x-3.15.

Chipie’s picture

#35 does not consider the default settings of the contenttype (e.g. status, promote, sticky), so I have modified it for 7.x-3.14.

Chipie’s picture

Here is a version for 7.x-3.15

marcingy’s picture

Issue tags: +Needs tests

Before we can even think about committing this it needs tests

marcingy’s picture

Status: Needs review » Needs work
jomarocas’s picture

Status: Needs work » Needs review

The patch attach need review

tyler.frankenstein’s picture

Status: Needs review » Needs work

jomarocas, I think marcingy meant it needs work because tests need to be written and then re-rolled into patch.

poldown’s picture

Same issue for update. The same logic as in #52 (for _node_resource_create) needs to be added to _node_resource_update:

...
$node_type = $node['type'];
node_object_prepare($old_node);

$old_node = (object) array_merge((array) $old_node, (array) $node);  // ---ADDED---

// Setup form_state.
$form_state = array();
$form_state['values'] = $node;
$form_state['values']['op'] = variable_get('services_node_save_button_' . $node_type . '_resource_update', t('Save'));
$form_state['node'] = $old_node;
$form_state['programmed_bypass_access_check'] = FALSE;
...
tyler.frankenstein’s picture

This patch addresses #57 by also adding multi value support when updating nodes. We'll still need to add this similar behavior to the other core entity types (users, comments, vocabularies, terms) and add tests.

tyler.frankenstein’s picture

FileSize
1.59 KB

This patch adds additional support for multi value fields during user update. I briefly tried adding support for multi value fields during user creation, but as far as I can tell it isn't quite as straightforward as the technique we used with node creation multi values.

stupiddingo’s picture

Tested patch #59. It applies cleanly and handles multiple value unlimited text fields and double fields correctly for node create, node edit and user update.

Given the coversation above and lack of full resource coverage I'm hesitant to mark RTBC so I'll going to leave the status unchanged.

That being said, this patch resolved a major bug and a huge thanks to y'all for chasing this down with a clean simple fix. I spent a couple days totally going the wrong direction...

/me hat tip

jerrac’s picture

#59 works for me as well.

What, exactly, is left to do before it can be merged?

kylebrowning’s picture

Per #59, this doesn't work on user entities yet, and may possible break user creation.

Tests are needed before we commit

Erhvervswebdesign’s picture

Patch #59 works well for me. By patching we are now able to upload multiple images through services.

Any idea, if or when there will be an update for the services module including the feature?

kylebrowning’s picture

@Erhvervswebdesign see #69 and #62

khiminrm’s picture

@tyler.frankenstein, thanks for your patch in #59!
It works well for me for updating multi value text field in users. I used this patch with services 7.x-3.19.

xpersonas’s picture

@tyler.frankenstein - Thank... you. Saved my sanity.
Patch in #59 works for me using Services 7.x-3.20.
Worked out the issue I had with multi-value image fields.

Jeff Veit’s picture

It took most of a day to find this problem, and this patch. Non-intuitive because it was on a text field. Patch in 59 applies cleanly to the current services (7.x-3.20) and works.

Jeff Veit’s picture

Reviewing the code. Will comment later.

sangas’s picture

This patch not work for me
Console error

POST: https://example.ru/?q=drupalgap/node.json - 406 - Not Acceptable : An illegal choice has been detected. Please contact the site administrator. An illegal choice has been detected. Please contact the site administrator. .
jdrupal-7.0.5.min.js:2 {"form_errors":{"field_12121][und":"\u0421\u0434\u0435\u043b\u0430\u043d \u043d\u0435\u0434\u043e\u043f\u0443\u0441\u0442\u0438\u043c\u044b\u0439 \u0432\u044b\u0431\u043e\u0440. \u041f\u043e\u0436\u0430\u043b\u0443\u0439\u0441\u0442\u0430, \u043e\u0431\u0440\u0430\u0442\u0438\u0442\u0435\u0441\u044c \u043a \u0430\u0434\u043c\u0438\u043d\u0438\u0441\u0442\u0440\u0430\u0442\u043e\u0440\u0443 \u0441\u0430\u0439\u0442\u0430."}}
jdrupal-7.0.5.min.js:2 date: Wed, 06 Dec 2017 14:37:10 GMT
x-content-type-options: nosniff
server: nginx/1.12.0
x-powered-by: PHP/7.0.25
transfer-encoding: chunked
content-language: ru
cache-control: no-cache, must-revalidate
connection: keep-alive
content-type: application/json
expires: Sun, 19 Nov 1978 05:00:00 GMT
tyler.frankenstein’s picture

@sangas The patch is separate from the issue you are experiencing, you are sending bogus data up to the field, you need to adjust the widget in Drupal and then adjust the JSON delivery to match.

tyler.frankenstein’s picture

Restoring important info provided by @Jeff Veit:

The patch contains this line:

$old_node = (object) array_merge((array) $old_node, (array) $node);

I think this is potentially unsafe. I think this code takes the trusted
input, $old_node, and replaces it in the merge with untrusted, unvalidated
input which is held in $node. Potentially that's a security risk.

I think there's some mitigation because later the $old_node and the $node are
pass through drupal_form_submit, in which the $node is validated. However, if
there's a case where $node fails without generating an error, then it might
be exploitable. For instance, if you send an array into a text field, there's
no error. Luckily, in this case the input is changed to ''. But in general, I
think this merge is an unsafe thing to do and it happens in other places in
this patch too.

However, taking that particular line out still leaves the patch working for
me. So I'm not sure why it's needed.

Thank you Jeff for this info, it's interesting to hear that particular line may not in fact be needed at all. As far as the mitigation, from my understanding it would be safe because everything still runs through the drupal form submit layer, and all validation should be picked up that way. It'd be great to hear other opinions on that matter, as well as comments about the array_merge() call.

Jeff Veit’s picture

I tried some simple exploits without success, but I reckon to what you want is some code in node_submit which takes the input and decides that it's not ok, and discards it in favour of the existing value. If that happens, then the value in $old_node is our exploit data. And I reckon the best chance of that is data which is malformed in some way, which is why I first tried sending an array to a text field.

I think the patch should avoid mixing clean and dirty data. Validate the data before it's merged if the merge is necessary. (And I'm not sure it is.) That way the submit function and the merged data match and the situation described above can't occur.

All this applies to the other clean/dirty merges too.

sangas’s picture

The drupalgap generates this json code for unlimited fields
field_tip_obiavlenia1":{"und":{"value":["1","2",""]}}}
But the correct query
field_tip_obiavlenia1":{"und":[{"value":"1"},{"value":"2"}]}

alexmoreno’s picture

used the patch for services 3.20 after it was running me crazy with this issue. Works like a dream, thank you!!!!

karaoglanoglou’s picture

Just logged in to say that patch #59 works like a charm for a multivalued link field in Services 3.20.

You saved me tons of work, thank you!

skylord’s picture

#59 applies nicely on 3.21. Working OK - thanks!
Wonder - will this issue still be not RTBC after 3-years anniversary of that patch in august? :-)

alexmoreno’s picture

Status: Needs work » Reviewed & tested by the community
stupiddingo’s picture

tyler.frankenstein’s picture

Thank you for the re-roll in #78, I am now hiding #59.

avanish12’s picture

Hello All,

I am still not able to add multiple image during node update. I am getting error : 500 Service unavailable (with message).
No message print on error log and watchdog.Please help on it.

Thanks in advance !!

sano’s picture

The patch solved my problem on the services 7.x-3.24 version. Thank you.

ireferpesa’s picture

Hello,
I have version 3.27 and can't add files that are multiple values.

field_image works because you can only upload one image. field_gallery and field_download do not work

{
"type": "press_area_notes",
"title": "prueba v1",
"body": {
"und": [{
"value": "test body!"
}]
},
"language" : "es",
"field_image": {
"und": [{
"fid": "2228"
}]
},
"field_download":{
"und": [{
"fid": "2469"
}]
},
"field_gallery":{
"und": [
{"fid": "2223"},
{"fid": "2229"}
]
}
}