feat(cts): add requestOptions tests #501

shortcuts · 2022-05-12T16:20:51Z

🧭 What and Why

🎟 JIRA Ticket: https://algolia.atlassian.net/browse/APIC-470

Changes included:

This PR adds tests for the requestOptions parameter, available on every methods of every clients:

The requestOptions parameter is available at the same level as the parameters in a CTS file
- It was the easiest way to implement it and make it work for me, but we can definitely change how it's done, please provide feedbacks!
It only consider values under the queryParameters or headers option

I've leveraged the common tests added for the customRequest, so we can have tests generated on every clients under the same method.

What's next

Docs will be updated in https://algolia.atlassian.net/browse/APIC-484

🧪 Test

CI :D

netlify · 2022-05-12T16:20:57Z

✅ Deploy Preview for api-clients-automation ready!

Name	Link
🔨 Latest commit	`76c778a`
🔍 Latest deploy log	https://app.netlify.com/sites/api-clients-automation/deploys/6283898e7be0340008c8a478
😎 Deploy Preview	https://deploy-preview-501--api-clients-automation.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

algolia-bot · 2022-05-12T16:21:54Z

✗ The generated branch has been deleted.

If the PR has been merged, you can check the generated code on the main branch.

millotp

Nice ! Hope this will be enough to catch all the bugs :)

.../algoliasearch-client-javascript/packages/client-common/src/transporter/createTransporter.ts

generators/src/main/java/com/algolia/codegen/cts/Request.java

generators/src/main/java/com/algolia/codegen/cts/ParametersWithDataType.java

tests/CTS/methods/requests/templates/php/requests.mustache

millotp

Just minor comments, GG !

generators/src/main/java/com/algolia/codegen/cts/ParametersWithDataType.java

shortcuts · 2022-05-17T11:38:21Z

clients/algoliasearch-client-php/lib/RequestOptions/RequestOptionsFactory.php

@@ -63,39 +63,19 @@ private function normalize($options)
        ];

        foreach ($options as $optionName => $value) {


@damcou @millotp

After adding boolean tests, I realized PHP formatted booleans array like [true, true, false] to [1,1,], which would have been wrong in our case

I also realized that we were handling query parameters twice (here, and in buildQuery), so I've moved all the logic there

Could you please confirm it's correct to you? Tests pass correctly but better safe than sorry

This looks fine ... But I'm pretty sure at some point I had the issue with the attributesToRetrieve param not being formatted correctly in the CTS so now I'm quite lost why it's working :D .

It's now done directly here in the buildQuery method, looking at the CTS it's still formatted correctly. Do you have any other stuff in mind we should check?

No, this is the only issue of that type, I think we're good.

shortcuts · 2022-05-17T11:38:57Z

clients/algoliasearch-client-php/lib/Support/Helpers.php

        $args = array_map(function ($value) {
            if (is_array($value)) {
-                return json_encode($value);
+                // PHP converts `true,false` in arrays to `1,`, so we create strings instead
+                // to avoid sending wrong values
+                $values = array_map(function ($v) {
+                    if (is_bool($v)) {
+                        return $v ? 'true' : 'false';
+                    }
+
+                    return $v;
+                }, $value);
+
+                // We then return the array as a string comma separated
+                return implode(',', $values);


This is what was duplicated in the RequestOptionFactory that I commented above

shortcuts requested a review from a team May 12, 2022 16:20

shortcuts self-assigned this May 12, 2022

shortcuts requested review from eunjae-lee and millotp and removed request for a team May 12, 2022 16:20

millotp reviewed May 13, 2022

View reviewed changes

shortcuts mentioned this pull request May 16, 2022

fix(templates): ensure requestOptions follow same format #506

Merged

shortcuts force-pushed the feat/cts-requestOptions branch from 2e76ad4 to fd515f2 Compare May 16, 2022 16:26

shortcuts changed the base branch from main to fix/serialize-query-parameters May 16, 2022 16:26

shortcuts requested a review from millotp May 16, 2022 16:27

shortcuts force-pushed the feat/cts-requestOptions branch from fd515f2 to 55f1ead Compare May 16, 2022 16:29

millotp reviewed May 17, 2022

View reviewed changes

generators/src/main/java/com/algolia/codegen/cts/ParametersWithDataType.java Outdated Show resolved Hide resolved

generators/src/main/java/com/algolia/codegen/cts/ParametersWithDataType.java Outdated Show resolved Hide resolved

Base automatically changed from fix/serialize-query-parameters to main May 17, 2022 09:54

shortcuts added 3 commits May 17, 2022 12:07

feat(cts): add requestOptions tests

e07f218

format json

e49a8a2

add boolean tests, handle them in PHP

81c74e9

shortcuts force-pushed the feat/cts-requestOptions branch from 311ea83 to 81c74e9 Compare May 17, 2022 11:36

restore playground

76c778a

shortcuts commented May 17, 2022

View reviewed changes

shortcuts requested a review from millotp May 17, 2022 12:56

millotp approved these changes May 17, 2022

View reviewed changes

damcou approved these changes May 17, 2022

View reviewed changes

shortcuts merged commit ba2689a into main May 17, 2022

shortcuts deleted the feat/cts-requestOptions branch May 17, 2022 13:47

This was referenced May 19, 2022

chore: release 2022-05-19 #518

Closed

chore: release 2022-05-19 #520

Closed

chore: release 2022-05-23 #533

Closed

chore: prepare release 2022-05-24 #541

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(cts): add requestOptions tests #501

feat(cts): add requestOptions tests #501

Uh oh!

shortcuts commented May 12, 2022 •

edited

Loading

Uh oh!

netlify bot commented May 12, 2022 •

edited

Loading

Uh oh!

algolia-bot commented May 12, 2022 •

edited

Loading

Uh oh!

millotp left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

millotp left a comment

Uh oh!

Uh oh!

Uh oh!

shortcuts May 17, 2022

Uh oh!

damcou May 17, 2022

Uh oh!

shortcuts May 17, 2022

Uh oh!

damcou May 17, 2022

Uh oh!

shortcuts May 17, 2022

Uh oh!

Uh oh!

		@@ -63,39 +63,19 @@ private function normalize($options)
		];

		foreach ($options as $optionName => $value) {

feat(cts): add requestOptions tests #501

feat(cts): add requestOptions tests #501

Uh oh!

Conversation

shortcuts commented May 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧭 What and Why

Changes included:

What's next

🧪 Test

Uh oh!

netlify bot commented May 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for api-clients-automation ready!

Uh oh!

algolia-bot commented May 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✗ The generated branch has been deleted.

Uh oh!

millotp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

millotp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shortcuts May 17, 2022

Choose a reason for hiding this comment

Uh oh!

damcou May 17, 2022

Choose a reason for hiding this comment

Uh oh!

shortcuts May 17, 2022

Choose a reason for hiding this comment

Uh oh!

damcou May 17, 2022

Choose a reason for hiding this comment

Uh oh!

shortcuts May 17, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shortcuts commented May 12, 2022 •

edited

Loading

netlify bot commented May 12, 2022 •

edited

Loading

algolia-bot commented May 12, 2022 •

edited

Loading