ericblade / mws-advanced

Javascript (node) Amazon Merchant Web Services (MWS) code with mws-simple as it's underpinnings
Apache License 2.0
41 stars 11 forks source link

Why does getMyFeesEstimate return an object? #154

Open pwrnrd opened 4 years ago

pwrnrd commented 4 years ago

I'm working on a PR where we mock tests which use MWS-Simple (so pretty much all test in test-api.js) . I fixed some minor bugs which I'll include in the coming PR.

When working on mocked tests for getMyFeesEstimate, I was wondering why getMyFeesEstimate returns an object instead of an array? Is it OK if I return an array here? MWS Advanced returns arrays everywhere else (for as far is know), so this change would make the API's more consistent. In addition, the MWS API docs also state that the return value is a list (see: https://docs.developer.amazonservices.com/en_UK/products/Products_GetMyFeesEstimate.html).

ericblade commented 4 years ago

Hmm. I think I need to have a look, I use that function extensively, and I haven't noticed that it seems like weird results... Is it one of those situations where it becomes an object because it's always a length 1 array, so it gets flattened out?

I'm mobile right now so I can't really have a look.. can you paste in a sample result ?

pwrnrd commented 4 years ago

This is what happens:

function parseFeesEstimate(fees) {
    const res2 = forceArray(fees.FeesEstimateResultList.FeesEstimateResult);
    const feeList = res2.reduce((acc, e) => {
        const sellerIdentifier = getSellerInputIdentifier(e);
        const identifierData = { ...getIdentifierData(e) };

        identifierData.IsAmazonFulfilled = identifierData.IsAmazonFulfilled === 'true';

        acc[sellerIdentifier] = {
            totalFees: getTotalFeesEstimate(e),
            time: getTimeOfFeesEstimation(e),
            detail: getFeeDetailList(e),
            identifier: identifierData,
            status: e.Status,
            error: e.Error || undefined,
        };
        return acc;
    }, {});

    return transformObjectKeys(feeList);
}

Fees is forced to be an array, but is converted to an object ({}) using reduce.

ericblade commented 4 years ago

Right, so I think that what's happening there, is that I'm intentionally transforming it to an object that is indexed by the identifier, so that on the receiving end you can just do

    (await getMyFeesEstimate({...}))['WhateverIdentifierISpecified']

rather than

    (await getMyFeesEstimate({...})).find((x) => x.SellerInputIdentifier === 'WhateverIdentifierISpecified')

I'm pretty sure that this is one of the quality-of-life changes that makes it much easier to access the data this way, than having to loop an array to find the piece you're looking for.

My particular use case, is that I request both the FBA and MF fees for any given product at the same time, so I provide SellerIdentifier as 'FBA' and 'MF' with the same ASIN, and then I can refer to them individually with something like

    const x = await getMyFeesEstimate({ ... });
    setFBAFees(x.FBA);
    setMFFees(x.MF);

Do you have a different use case, where using an array would make more sense? (specifically ignoring that "MWS returns an array" as a reason to have it that way, because we do a lot of transformations in the implemented things, to make the XML conversion more Javascript friendly, than a straight translation of the XML)