Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to update field values for custom field without changing the name #9949

Closed
1 task done
zackbennis opened this issue Aug 18, 2021 · 12 comments
Closed
1 task done
Assignees

Comments

@zackbennis
Copy link

Please confirm you have done the following before posting your bug report:

Describe the bug
When using the PUT for the 'fields/:id' endpoint to update a custom field's "field_values" for element type "listbox", the 'name' value is required, despite the field number already being specified. We don't want to update the name, so we tried using the same name, but we get the error, "The name has already been taken."

Since we're only updating the listbox values, we shouldn't have to supply a name when the field id has already been passed in the URL. When using no 'name' value, it returns an error saying that the 'name' field is required.

To Reproduce
Steps to reproduce the behavior:

  1. Use the /fields/:id PUT endpoint to update the listbox of a custom field.
  2. In the JSON, omit the name, or use the same name as the field you're updating.
  3. Error: Returns that the name field is required or that the name is already in use.

Expected behavior
Should not have to provide a name for an update action when the field ID is already supplied.

Server (please complete the following information):

  • Hosted Snipe-IT, v5.1.6

Desktop (please complete the following information):

  • Insomnia, PHP
@snipe
Copy link
Owner

snipe commented Aug 18, 2021

You should be using a PATCH for that then, as PATCH updates only a partial record.

@zackbennis
Copy link
Author

zackbennis commented Aug 18, 2021

I tried that, but it didn't work. I also couldn't find documentation on a PATCH for that endpoint.

edit: Also, thanks for the quick reply :-)

@snipe
Copy link
Owner

snipe commented Aug 18, 2021

You're totally right - I don't see that capability in the API. Give me a little bit and we'll have that up for you. Also please send an email to [email protected] so we can make sure to bump your version once the fix is pushed - if you think to this issue, they can take it from there :)

@zackbennis
Copy link
Author

Will do; thanks again for your super expedient replies!

@snipe
Copy link
Owner

snipe commented Aug 18, 2021

Actually, I lied. This method here should do it:

public function update(Request $request, $id)
{
$this->authorize('update', CustomField::class);
$field = CustomField::findOrFail($id);
/**
* Updated values for the field,
* without the "field_encrypted" flag, preventing the change of encryption status
* @var array
*/
$data = $request->except(['field_encrypted']);
$validator = Validator::make($data, $field->validationRules());
if ($validator->fails()) {
return response()->json(Helper::formatStandardApiResponse('error', null, $validator->errors()));
}
$field->fill($data);
if ($field->save()) {
return response()->json(Helper::formatStandardApiResponse('success', $field, trans('admin/custom_fields/message.field.update.success')));
}
return response()->json(Helper::formatStandardApiResponse('error', null, $field->getErrors()));
}

When you say it didn't work, did you get the same error as when you tried a PUT?

@snipe
Copy link
Owner

snipe commented Aug 18, 2021

(Also if you can post a cURL command with your Bearer token XXXXXXed out, I can try to replicate. (Or just open a ticket, whichever works best for you. I'll probably be the one handling that ticket anyway :) )

@zackbennis
Copy link
Author

Here you go:

curl --request PATCH \ --url https://<snipe-instance-url>/api/v1/fields/16 \ --header 'Accept: '\''application/json'\''' \ --header 'Authorization: Bearer XXXXXX' \ --header 'Content-Type: '\''application/json'\''' \ --data '{ "name": "Test Field", "element": "listbox", "field_values": "Test1\r\nTest2\r\nTest3\r\nTest4" }'

@snipe
Copy link
Owner

snipe commented Aug 19, 2021

Thanks for the example - in digging into this yesterday I think this is something on our end (but not the obvious stuff). I’m investigating and will let you know what I find

@zackbennis
Copy link
Author

Any update on this? We have a workaround, but it's causing some bugginess in our workflow.

@StarlessNights
Copy link
Contributor

Just ran into this issue when I was investigating the API's capabilities. Like this it's not suitable for implementing list type custom fields with dynamic choices. Clearly a problem.

@zackbennis may I inquire about your workaround?

@zackbennis
Copy link
Author

@StarlessNights - In our script, we're simply updating the name with one curl, then pushing the changes and reverting the name to the original name with a second curl. The issue is we have a bunch of scripts checking this data, so if there's ever a hiccup and the temp name sticks (which has happened), it throws off a bunch of our other logic. We've built in some additional error checking for this, but it's less than ideal, for sure.

@viclou
Copy link
Collaborator

viclou commented Oct 20, 2022

+1 FD 31447

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants