snipe / snipe-it

A free open source IT asset/license management system
https://snipeitapp.com
GNU Affero General Public License v3.0
11.21k stars 3.2k forks source link

[Feature Request]: Consistent API data returns #15450

Open TheFuzzyTech opened 2 months ago

TheFuzzyTech commented 2 months ago

Is your feature request related to a problem? Please describe.

When searching for an asset by serial number using the API, If the asset does not exist, the API returns the JSON below:

{
  "status": "error",
  "messages": "Asset does not exist.",
  "payload": null
}

When the Asset does exist, the API returns an object containing the total number of assets found, and an array of those assets.

This makes it inconsistent when using the API to write automation that depend on knowing the if the asset exists or not.

Describe the solution you'd like

I would like the api to return the below json when an asset cannot be found, or something similar.

{
  "status": "error",
  "messages": "Asset does not exist.",
  "payload": null,
  "total": 0
}

Describe alternatives you've considered

I could use a try/except when looking for the total key, but I think it would make more sense that the total key exists even if no assets are found.

Additional context

This is being used for an integration utilizing jamf webhooks to update assets in SnipeIT.

welcome[bot] commented 2 months ago

šŸ‘‹ Thanks for opening your first issue here! If you're reporting a šŸž bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

uberbrady commented 2 months ago

Weren't you also saying that it used to be like that? Or we would at least return an empty array? Just making sure if this is a 'feature request' or a 'regression'.

TheFuzzyTech commented 2 months ago

It either used to return an empty array or would return total: 0. I have automation that I wrote 5 years ago that looks for total: 0. I just happened to be updating it today and noticed that error coming through.

snipe commented 2 months ago

Generally speaking, if one object is expected as a response, we don't provide the total field, since it's either going to be there or it's not, and there wouldn't be more than one result.

What's the URL query / API endpoint you're using?

TheFuzzyTech commented 2 months ago

/hardware/byserial/:serial which I believe is just filtering based on serial, which is why when an asset exists it returns a total and an array. I don't believe serial numbers are unique which would explain the behavior.

This is where my old code is that I'm refactoring that's causing the error. Please excuse beginner python me and awful code there.

The issue very well could be handled on my end, I would just like more consistent output from the API. Does an error really need to be returned there? Would an empty array be appropriate?

snipe commented 2 months ago

We don't return empty arrays on things that would normally return only one thing tho. If you go to api/v1/users/094580345809680089, we return:

{
    "status": "error",
    "messages": "User does not exist or you do not have permission view them.",
    "payload": null
}

without a total, and without an empty array. We expect one and only one, so we tell you that one thing doesn't exist.

snipe commented 2 months ago

You'd really want to be parsing the "status": "error", part

TheFuzzyTech commented 2 months ago

I can do that, but I think it's important to note that the endpoint doesn't return one thing. It returns an array of things. I'm sure that's just there to support when enforcing unique serials is disabled, but the distinction is important. When searching for a user by id an object is returned. It just feels inconsistent.

snipe commented 2 months ago

It returns an array of things.

{
  "status": "error",
  "messages": "Asset does not exist.",
  "payload": null
}

That's JSON, not an array of things.

TheFuzzyTech commented 2 months ago

It's an array of things if it exists, even if one thing is expected. Or more accurately an object containing an array of assets. That's what I'm saying differs between your example of a user and an asset.

Yes in reality I should probably just except a KeyError for total, but that doesn't make it feel any less inconsistent.


  "total": 1,
  "rows": [
    {
      "id": 2597,
      "name": "",
      "asset_tag": "1257475821",
      "serial": "c0d5b6b8-a5de-36c4-9b37-167b6076e0a7",
      "model": {
        "id": 18,
        "name": "Ultrasharp U2415"
      },
      "byod": false,
      "requestable": false,
      "model_number": "4024007152147426",
      "eol": "12 months",
      "asset_eol_date": {
        "date": "2025-08-17",
        "formatted": "Sun Aug 17, 2025"
      },
      "status_label": {
        "id": 1,
        "name": "Ready to Deploy",
        "status_type": "deployable",
        "status_meta": "deployable"
      },
      "category": {
        "id": 5,
        "name": "Displays"
      },
      "manufacturer": {
        "id": 3,
        "name": "Dell"
      },
      "supplier": {
        "id": 4,
        "name": "McClure Group"
      },
      "notes": "Created by DB seeder",
      "order_number": "8167525",
      "company": null,
      "location": {
        "id": 4,
        "name": "West Brannon"
      },
      "rtd_location": {
        "id": 4,
        "name": "West Brannon"
      },
      "image": "https://develop.snipeitapp.com/uploads/models/ultrasharp.jpg",
      "qr": "https://develop.snipeitapp.com/uploads/barcodes/qr-1257475821-2597.png",
      "alt_barcode": "https://develop.snipeitapp.com/uploads/barcodes/c128-1257475821.png",
      "assigned_to": null,
      "warranty_months": null,
      "warranty_expires": null,
      "created_at": {
        "datetime": "2024-09-05 14:42:56",
        "formatted": "Thu Sep 05, 2024 2:42PM"
      },
      "updated_at": {
        "datetime": "2024-09-05 14:43:25",
        "formatted": "Thu Sep 05, 2024 2:43PM"
      },
      "last_audit_date": null,
      "next_audit_date": null,
      "deleted_at": null,
      "purchase_date": {
        "date": "2024-08-17",
        "formatted": "Sat Aug 17, 2024"
      },
      "age": "2 weeks ago",
      "last_checkout": null,
      "last_checkin": null,
      "expected_checkin": null,
      "purchase_cost": "2,618.57",
      "checkin_counter": 0,
      "checkout_counter": 0,
      "requests_counter": 0,
      "user_can_checkout": true,
      "book_value": "2,618.57",
      "custom_fields": {},
      "available_actions": {
        "checkout": true,
        "checkin": true,
        "clone": true,
        "restore": false,
        "update": true,
        "delete": true
      }
    }
  ]
}```
snipe commented 2 months ago

The last time this code was changed was Jan 2023, so the issue is that if we change it now, we'll break newer integrations. šŸ˜µā€šŸ’«