search

moqui / mantle-usl

Mantle Universal Service Library
http://www.moqui.org/mantle.html
Other
29 stars 61 forks source link

Incomplete response from Entity Master API on product update #11

Closed muzykabart closed 8 years ago

muzykabart commented 8 years ago

Hi David,

When you send a PUT request to the API to update product you expect all of its data in response (similar to what you receive back on GET), don't you?

Our request is:

curl -X PUT -u john.doe:moqui -H 'Content-Type: application/json' -d '{"pseudoId":"100002","productName":"Updated Product","prices":[{...}],"parties":[{...},{...}],"contents":[{...},{...}],"status":{...}}' https://example.moqui.org/rest/m1/products/100002

Response from API:

{
    "oldStatusId": "PrvwPending",
    "parties": [
        {

        },
        {

        },
        {

        }
    ],
    "statusChanged": false,
    "productId": "100002",
    "prices": {
        "productPriceId": "100152"
    },
    "contents": [
        {
            "productContentId": "100000"
        },
        {
            "productContentId": "100001"
        }
    ]
}

There are two problems with that response:

  1. "parties" is an array of empty hashes - hashes should include at least all of the primary keys
  2. "prices" is a hash instead of array of hashes (Product has many Prices) - happens only when product has one price

Thanks! Bart

jonesde commented 8 years ago

Thank you for the feedback. Note that this is a partial duplicate of issue #10.

jonesde commented 8 years ago

Issue #10 is fixed in moqui-framework commit #5ae3805. Would you test your case again and see if the results are what you expect?

If not please send over a full curl command (no data excluded) so I can see exactly what causes the issue and can reproduce it.

muzykabart commented 8 years ago

Hi David,

Ok, just checked and prices are fixed, but the rest of the response is still incomplete.

Our full request:

curl -X PUT -u john.doe:moqui -H 'Content-Type: application/json' -d '{"pseudoId":"100002","productName":"Updated Product 2","description":"Updated Product 2 description","lastUpdatedStamp":"2016-04-01T10:33:58.000Z","statusId":"PrvwPending","originGeoId":"CHN_31","prices":[{"productPriceId":"100152","price":100,"standardLeadTimeDays":4,"fromDate":"2016-03-29T03:05:00.000Z","thruDate":null,"lastUpdatedStamp":"2016-04-01T10:33:58.000Z","priceTypeEnumId":"PptWholesale","productId":null,"priceUomId":"USD","type":{"enumId":"PptWholesale","description":"Wholesale Price","lastUpdatedStamp":"2016-03-08T09:23:37.000Z","enumTypeId":"ProductPriceType","parent":{"enumTypeId":"TrackingCodeType","lastUpdatedStamp":"2016-03-08T09:23:31+0000","description":"Access Code","_entity":"enums"},"parentEnumId":null,"related":{"enumTypeId":"TrackingCodeType","lastUpdatedStamp":"2016-03-08T09:23:31+0000","description":"Access Code","_entity":"enums"},"relatedEnumId":null,"type":{"enumTypeId":"ProductPriceType","description":"Product Price Type","lastUpdatedStamp":"2016-03-08T09:23:37.000Z"}},"product":"100002","priceUom":{"uomId":"USD","abbreviation":"USD","description":"United States Dollar","lastUpdatedStamp":"2016-03-08T09:23:16.000Z","uomTypeEnumId":"UT_CURRENCY_MEASURE","type":{"enumId":"UT_CURRENCY_MEASURE","description":"Currency","lastUpdatedStamp":"2016-03-08T09:23:13.000Z","enumTypeId":"UomType","parentEnumId":null,"relatedEnumId":null,"type":null}}}],"parties":[{"partyId":"EX_JOHN_DOE","fromDate":"2016-03-25T04:10:00.000Z","thruDate":null,"lastUpdatedStamp":"2016-04-01T10:33:58.000Z","role":{"lastUpdatedStamp":"2016-03-08T09:23:32+0000","description":"Manager","_entity":"mantle.party.RoleType"},"roleTypeId":"Manager","productId":null,"party":{"pseudoId":"EX_JOHN_DOE","lastUpdatedStamp":"2016-03-31T04:39:13+0000","comments":"This is a new Comment","person":{"lastName":"Gross","lastUpdatedStamp":"2016-03-31T04:39:13+0000","occupation":"Developeradad","_entity":"mantle.party.Person","firstName":"David","middleName":"W"},"partyTypeEnumId":"PtyPerson","_entity":"parties"},"product":"100002"},{"partyId":"ZiddlemanInc","fromDate":"2016-03-29T02:31:38.000Z","thruDate":null,"lastUpdatedStamp":"2016-04-01T10:33:58.000Z","role":{"lastUpdatedStamp":"2016-03-08T09:23:32+0000","description":"Supplier","_entity":"mantle.party.RoleType"},"roleTypeId":"Supplier","productId":null,"party":{"pseudoId":"ZiddlemanInc","lastUpdatedStamp":"2016-03-08T09:23:56+0000","organization":{"lastUpdatedStamp":"2016-03-08T09:23:56+0000","organizationName":"Ziddleman Incorporated","_entity":"mantle.party.Organization"},"partyTypeEnumId":"PtyOrganization","_entity":"parties"},"product":"100002"}],"contents":[{"productContentId":"100000","contentLocation":"https://example.com/img.jpg","fromDate":"2016-03-28T06:52:00.000Z","thruDate":null,"lastUpdatedStamp":"2016-04-01T10:33:58.000Z","productContentTypeEnumId":"PcntEmail","productId":null,"type":null,"product":"100002"},{"productContentId":"100001","contentLocation":"https://example.com/img2.jpg","fromDate":"2016-03-28T07:32:17.000Z","thruDate":null,"lastUpdatedStamp":"2016-04-01T10:33:58.000Z","productContentTypeEnumId":"PcntAddToCartImage","productId":null,"type":null,"product":"100002"}],"identifications":[{"productIdTypeEnumId":"PidtOther","idValue":"OTHER_ID","lastUpdatedStamp":"2016-04-01T10:33:58.000Z","productId":null,"type":{"enumId":"PidtOther","description":"Other","lastUpdatedStamp":"2016-03-08T09:23:37.000Z","enumTypeId":"ProductIdentificationType","parent":{"enumTypeId":"TrackingCodeType","lastUpdatedStamp":"2016-03-08T09:23:31+0000","description":"Access Code","_entity":"enums"},"parentEnumId":null,"related":{"enumTypeId":"TrackingCodeType","lastUpdatedStamp":"2016-03-08T09:23:31+0000","description":"Access Code","_entity":"enums"},"relatedEnumId":null,"type":{"enumTypeId":"ProductIdentificationType","description":"Product Identification Type","lastUpdatedStamp":"2016-03-08T09:23:37.000Z"}},"product":"100002"},{"productIdTypeEnumId":"PidtSku","idValue":"SKU_ID","lastUpdatedStamp":"2016-04-01T10:33:58.000Z","productId":null,"type":{"enumId":"PidtSku","description":"SKU","lastUpdatedStamp":"2016-03-08T09:23:37.000Z","enumTypeId":"ProductIdentificationType","parent":{"enumTypeId":"TrackingCodeType","lastUpdatedStamp":"2016-03-08T09:23:31+0000","description":"Access Code","_entity":"enums"},"parentEnumId":null,"related":{"enumTypeId":"TrackingCodeType","lastUpdatedStamp":"2016-03-08T09:23:31+0000","description":"Access Code","_entity":"enums"},"relatedEnumId":null,"type":{"enumTypeId":"ProductIdentificationType","description":"Product Identification Type","lastUpdatedStamp":"2016-03-08T09:23:37.000Z"}},"product":"100002"}],"dimensions":[{"dimensionTypeId":"Height","value":"100","lastUpdatedStamp":"2016-04-01T10:33:58.000Z","productId":null,"valueUomId":"LEN_cm","type":null,"product":"100002","valueUom":null},{"dimensionTypeId":"Weight","value":"50","lastUpdatedStamp":"2016-04-01T10:33:58.000Z","productId":null,"valueUomId":null,"type":null,"product":"100002","valueUom":null}],"status":{"statusId":"PrvwPending","statusCode":null,"description":"Pending","lastUpdatedStamp":"2016-03-08T09:23:37.000Z","statusTypeId":"ProductReview"},"originGeo":{"geoId":"CHN_31","geoName":"Shanghai","geoNameLocal":"","geoCodeAlpha2":null,"geoCodeAlpha3":null,"geoCodeNumeric":"310000","lastUpdatedStamp":"2016-03-08T09:23:41.000Z","geoTypeEnumId":"GEOT_PROVINCE","type":null}}' https://example.com/rest/m1/products/100002

Response from API:

{
    "oldStatusId": "PrvwPending",
    "productId": "100002",
    "contents": [
        {
            "productContentId": "100000"
        },
        {
            "productContentId": "100001"
        }
    ],
    "parties": [
        {

        },
        {

        }
    ],
    "statusChanged": false,
    "prices": [
        {
            "productPriceId": "100152"
        }
    ],
    "identifications": [
        {

        },
        {

        }
    ],
    "dimensions": [
        {

        },
        {

        }
    ]
}
jonesde commented 8 years ago

I grabbed the JSON text from your request (the curl -d text) and reformatted it to make it more readable. It looks like you did a GET for the Product default master and then resubmitted it... and I'm not sure why you'd ever do that. Generally in an update you wouldn't include fields like lastUpdatedStamp, and generally you wouldn't include type one relationships for seed data like prices[x].type. Type one relationships that are not specified as mutable will be ignored to reduce chances of updating something you didn't intend, but generally this is a bad idea... in this case it makes sense to specify priceTypeEnumId but not include a nested type object in the JSON.

In general the response the included looks correct. The response you get back is just the standard response from an entity-auto service call (i.e. calling a service with no package, a CrUD verb, and an entity name for the noun). If you pass in all PKs there is nothing for it to return in the response, and that is why various responses are empty.

In other words, I don't see an issue here unless you are requesting a new feature of some sort... and if so what is it that you would like to see coming back that is different from what is currently returned by entity-auto service calls?

muzykabart commented 8 years ago

OK, thanks for the reply. We've changed our curl request to:

curl -X PUT -u john.doe:moqui -H 'Content-Type: application/json' -d '{"pseudoId":"TEST_SKU","productName":"Test Product","description":"Test Description","statusId":"PrvwPending","originGeoId":"CHN_31","prices":[{"productPriceId":"100000","price":20,"standardLeadTimeDays":10,"fromDate":"2016-04-04T08:35:22.000Z","thruDate":null,"priceTypeEnumId":"PptWholesale","priceUomId":"USD","product":"100000"}],"parties":[{"partyId":"EX_JOHN_DOE","fromDate":"2016-04-04T08:28:40.000Z","thruDate":null,"roleTypeId":"Supplier","product":"100000"}],"contents":[{"productContentId":"100000","contentLocation":"https://example.com/img.jpg","fromDate":"2016-04-04T08:27:33.000Z","thruDate":null,"productContentTypeEnumId":"PcntImageUrlOriginal","product":"100000"}],"identifications":[{"productIdTypeEnumId":"PidtOther","idValue":"BARCODE","product":"100000"}],"dimensions":[]}' https://example.com/rest/m1/products/100000

More readable version of data sent:

{
    "pseudoId": "TEST_SKU",
    "productName": "Test Product",
    "description": "Test Description",
    "statusId": "PrvwPending",
    "originGeoId": "CHN_31",
    "prices": [
        {
            "productPriceId": "100000",
            "price": 20,
            "standardLeadTimeDays": 10,
            "fromDate": "2016-04-04T08:35:22.000Z",
            "thruDate": null,
            "priceTypeEnumId": "PptWholesale",
            "priceUomId": "USD",
            "product": "100000"
        }
    ],
    "parties": [
        {
            "partyId": "EX_JOHN_DOE",
            "fromDate": "2016-04-04T08:28:40.000Z",
            "thruDate": null,
            "roleTypeId": "Supplier",
            "product": "100000"
        }
    ],
    "contents": [
        {
            "productContentId": "100000",
            "contentLocation": "https://example.com/img.jpg",
            "fromDate": "2016-04-04T08:27:33.000Z",
            "thruDate": null,
            "productContentTypeEnumId": "PcntImageUrlOriginal",
            "product": "100000"
        }
    ],
    "identifications": [
        {
            "productIdTypeEnumId": "PidtOther",
            "idValue": "BARCODE",
            "product": "100000"
        }
    ],
    "dimensions": []
}

And the response we received from API is still missing some of the data (parties and identifications should at least include PKs):

{
    "oldStatusId": "PrvwPending",
    "productId": "100000",
    "contents": [
        {
            "productContentId": "100000"
        }
    ],
    "parties": [
        {

        }
    ],
    "statusChanged": false,
    "prices": [
        {
            "productPriceId": "100000"
        }
    ],
    "identifications": [
        {

        }
    ]
}

I am assuming they don't cause their entities (mantle.product.ProductIdentification and mantle.product.ProductParty) have more than one Primary Key.

jonesde commented 8 years ago

The general idea of the entity-auto services it to return any PK values that it fills in, such as fromDate and such. If no values are filled in it should return nothing, as generally the caller has all the data it needs to identify whatever was just updated. Right now that is correct, for a store entity-auto operation it returns single value PK fields regardless of whether they were passed in or generated/sequenced.

What it is that you propose be changed, or is this more of a question?

jonesde commented 8 years ago

Closing for now, feel free to reopen if you have any thoughts on changes to be made.