WEEE-Open / tarallo

T.A.R.A.L.L.O. Inventory System
GNU Affero General Public License v3.0
16 stars 9 forks source link

updateFeatures does not work properly #133

Closed morpheusthewhite closed 4 years ago

morpheusthewhite commented 4 years ago

updateFeatures does not work properly in 2 different situations

  1. When trying to eliminate a feature on an existing item the answer is a 204 but the feature isn't actually removed. How to reproduce it:
    curl -X PATCH -H 'Content-Type: application/json' -i 'http://127.0.0.1:8080/v2/items/r133/features' --data '{
    "frequency-hertz": null
    }'
  2. When trying to add a feature to a non existing item the answer is again a 204. How to reproduce it:
    curl -X PATCH -H 'Content-Type: application/json' -i 'http://127.0.0.1:8080/v2/items/NONEXISTANT/features' --data '{
    "frequency-hertz": null
    }'
lvps commented 4 years ago

Upon further inspection, point 1 is correct as is: that feature is part of the product, if you delete it from the item the operation succeeds. More precisely, the item didn't have that feature and doesn't have it after, so the operation is a success.

You can use ?separate to get separate item and product to examine this distinction. E.g. /v2/items/R133?separate returns:

{
  "code": "R133",
  "features": {
    "brand": "Samsung",
    "model": "S800ABC1024",
    "sn": "ASD217236147246F",
    "variant": "v1",
    "working": "no"
  },
  "product": {
    "brand": "Samsung",
    "model": "S800ABC1024",
    "variant": "v1",
    "features": {
      "capacity-byte": 1073741824,
      "color": "green",
      "frequency-hertz": 800000000,
      "ram-ecc": "no",
      "ram-form-factor": "dimm",
      "ram-type": "ddr2",
      "type": "ram"
    }
  },
  "location": [
    "Polito",
    "Chernobyl",
    "Table",
    "RamBox"
  ]
}

For the pytarallo tests you can use R70 and R71, which I've just added: they don't have a product at all and the features are hardcoded and constant.

Point 2 happened because that request translates to an SQL DELETE, which achieves the final state of not having a line with NONEXISTANT and frequency-hertz. However I agree that returning 404 would be better, so a fix is coming in the next 30 seconds.