Auto JSON.stringify for POST Requests

[darseen]

Description

This PR fixes #202 where users were required to manually convert their data to a string using JSON.stringify for some POST requests. With this fix, data will now be automatically stringified before being sent in POST requests.

I discovered this issue when I was rewriting the codebase in TypeScript for the unofficial package amadeus-ts, and decided to fix it for the official package as well.

Changes Made

• Added functionality to automatically apply JSON.stringify to objects in POST requests.
• Updated tests to expect automatically stringified objects for POST requests.
• Ensured all existing tests are passing.
• Manually tested all endpoints to confirm functionality.

Testing

• Unit Tests: All relevant tests have been updated and are passing successfully.
• Manual Testing: Each endpoint was manually tested to ensure correct behavior after the update.

[tsolakoua]

Thanks @darseen so much for the PR! We will review it shortly!

[tsolakoua]

Hello @darseen, I reviewed your code, thanks again for the PR!

What worries me is the fact that these changes will break the code of existing users of the library who will install the latest version. Do you think we could add a check inside the POST to determine if the body is already a string or not and then handle the conversion accordingly? What are your thoughts on that?

[darseen]

Hi @tsolakoua, Thank you for taking the time to review the PR!

You are absolutely correct, the current implementation would break backwards compatibility. Your suggestion for checking the type of the params before passing them to the client seems great to handle this issue. It could be as simple as:

const stringifiedParams = typeof params === "string" ? params : JSON.stringify(params);

and then pass the stringifiedParams to the client.

Would you like me to implement the currently suggested approach? If so, should the README be changed in this case? Or if you have any other ideas, I'm looking forward to hearing them.

[tsolakoua]

Thanks for your quick response @darseen.

If you want and you have time, feel free to go ahead and make the changes. Otherwise we will create a new PR whenever we have some time after we merge yours to master. Let me know what works the best for you. If we agree on merging this PR, I would just ask you to solve the conflicts.

Maybe for the README we could keep it as it is in any case, to encourage the new users to use the method you've implemented.

Thanks again for the PR, we really appreciate it :)

[sonarqubecloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[darseen]

Hi @tsolakoua, I have added the following changes:

Changes

• Resolved merge conflict
• Implemented stringifying params while maintaining backwards compatibility.
• Updated code examples both in README and in JSDocs.
• Used const instead of var in README, since var is not the recommended keyword for declaring variables and let or const should be used instead.

As for the current implementation of the stringifying of the params, I decided to add the check in the client.post method itself, since all post methods require their params to be a string, thus avoiding duplicating the same check for various endpoints across the codebase.

Tests

• I modified tests that are related to the client.post method, and all of them passed successfully.
• I tested some of the endpoints manually to ensure that the implemented functionality works. Although I do advise more manual testing on your part, just in case anything was overlooked.

Suggestions

Currently the codebase can be improved in many ways, since many parts of it is still using ES5 syntax. Here are some recommendations

• Using async/await instead of .then.
• Using Arrow functions instead of bind method
• Some code refactors that would make the code cleaner and easier to read and maintain, such as:

From this:

// request.js
body() {
    if (this.verb !== 'POST') { return ''; }
    else {
      if (!this.bearerToken) {
        return qs.stringify(this.params);
      }
      return this.params;
    }
  }

To this:

// request.js
body() {
    if (this.verb !== 'POST') return '';
    else if (!this.bearerToken) return qs.stringify(this.params);
    return this.params;
  }

I would be glad to make such changes if you would like to implement these suggestions!

[tsolakoua]

The PR is merged, thanks so much @darseen! We checked your suggestions to improve the codebase and definitely are valid!