chore(samples): interactive tutorials code samples for CRUD products by nmykhailets · Pull Request #149 · googleapis/nodejs-retail

nmykhailets · 2022-02-18T18:12:23Z

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

generated-files-bot · 2022-02-18T18:12:28Z

Warning: This pull request is touching the following templated files:

.kokoro/pre-samples-test.sh - .kokoro files are templated and should be updated in synthtool

snippet-bot · 2022-02-18T18:12:32Z

Here is the summary of changes.

You are about to add 9 region tags.

samples/interactive-tutorials/product/add-fulfillment-places.js:18, tag retail_add_remove_fulfillment_places
samples/interactive-tutorials/product/create-product.js:18, tag retail_create_product
samples/interactive-tutorials/product/crud-product.js:18, tag retail_crud_product
samples/interactive-tutorials/product/delete-product.js:18, tag retail_delete_product
samples/interactive-tutorials/product/get-product.js:18, tag retail_get_product
samples/interactive-tutorials/product/get-products-list.js:18, tag retail_get_products_list
samples/interactive-tutorials/product/remove-fulfillment-places.js:18, tag retail_remove_fulfillment_places
samples/interactive-tutorials/product/set-inventory.js:18, tag retail_set_inventory
samples/interactive-tutorials/product/update-product.js:18, tag retail_update_product

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

Refresh this comment

sofisl

Looks good to me except for a few things:

Environment variables during testing are pretty confusing if you're new to the repo. Unless the sample requires the use of env variables, can we move them to just be variables in the test files? And otherwise, can we include it in the test file itself (i.e., process.env.VARIABLE = 'value')?
You need to run npm run fix to get rid of the linting errors.

chenlei1216 · 2022-02-25T01:20:23Z

+  );
+
+  // Add fulfillment places with outdated time
+  placeIds = ['store4'];


As explained offline, the timestamp check is per-store/place. If we use a new store id here, this update will still be allowed.

chenlei1216 · 2022-02-25T01:20:54Z

+  );
+
+  // Remove fulfillment places with outdated time
+  placeIds = ['store2'];


Same comment here to use the same place id.

We use here correct place id, because created product has placeIds: ['store1', 'store2', 'store3']

In such case we will not be able to demo this case - as if we send the same store_id the second time there store Id was already deleted and the flow of checking the timestamps will probably not executed. So I suggest to remove this stem from the tutorials and from this sample

kweinmeister

Please address the issues flagged at the bottom of this page, e.g. creating variables that aren't used. Other than that, looks good!

t-karasova · 2022-03-07T17:59:42Z

In the interactive tutorials we ask our users to set the environment variables, so I assume that in the tests we need reproduce the user flow closely. from this point of view we cannot change how the variables are stored, and if we want to use process.env.VARIABLE to store the variables for tests, we will get them duplicated

t-karasova · 2022-03-08T09:56:36Z

Looks good to me except for a few things:

Environment variables during testing are pretty confusing if you're new to the repo. Unless the sample requires the use of env variables, can we move them to just be variables in the test files? And otherwise, can we include it in the test file itself (i.e., process.env.VARIABLE = 'value')?
You need to run npm run fix to get rid of the linting errors.

@sofisl I don't know why I cannot see my earlier comment on this review, so I will answer one more time here, sorry if it is a duplication.
We ask our user who are going through the tutorials to set environment variables and those values are used in the code samples code. We could use the process.env.VARIABLE = 'value' in tests, but I think that the tests should simulate the user flow, and if these variables will be added they will duplicate the env variables

nmykhailets requested a review from a team February 18, 2022 18:12

product-auto-label Bot added api: retail Issues related to the googleapis/nodejs-retail API. samples Issues that are directly related to samples. labels Feb 18, 2022

nmykhailets force-pushed the products-crud branch from f76b2dc to d30d858 Compare February 18, 2022 18:41

sofisl reviewed Feb 18, 2022

View reviewed changes

sofisl added the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 18, 2022

gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 18, 2022

kweinmeister added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 18, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 18, 2022

nmykhailets force-pushed the products-crud branch from d30d858 to c23cad1 Compare February 18, 2022 19:09

kweinmeister added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Feb 18, 2022

gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 18, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 18, 2022

kweinmeister added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 18, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 18, 2022

nmykhailets force-pushed the products-crud branch from c23cad1 to 7e0505c Compare February 18, 2022 19:44

kweinmeister added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Feb 21, 2022

gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 21, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 21, 2022

nmykhailets force-pushed the products-crud branch 2 times, most recently from 62aec36 to 4b38384 Compare February 23, 2022 12:06

kweinmeister added the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 23, 2022

gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 23, 2022

kweinmeister added the kokoro:run Add this label to force Kokoro to re-run the tests. label Feb 23, 2022

yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Feb 23, 2022

nmykhailets force-pushed the products-crud branch from 4b38384 to e989ce9 Compare February 23, 2022 16:13

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 23, 2022

chenlei1216 approved these changes Feb 25, 2022

View reviewed changes

kweinmeister suggested changes Feb 25, 2022

View reviewed changes

nmykhailets force-pushed the products-crud branch from e989ce9 to b7e46b5 Compare February 28, 2022 23:17

kweinmeister added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Feb 28, 2022

gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 28, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 28, 2022

nmykhailets force-pushed the products-crud branch from b7e46b5 to 9abee6e Compare February 28, 2022 23:33

kweinmeister added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 28, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 1, 2022

nmykhailets force-pushed the products-crud branch from 9abee6e to 7d0e46c Compare March 1, 2022 00:44

kweinmeister added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Mar 1, 2022

gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 1, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 1, 2022

nmykhailets added 2 commits March 8, 2022 15:48

chore: crud products interactive samples

b2b503f

fix: lint issues

451ed32

nmykhailets force-pushed the products-crud branch from 9bfc622 to 451ed32 Compare March 8, 2022 13:51

kweinmeister added owlbot:run Add this label to trigger the Owlbot post processor. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Mar 8, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 8, 2022

gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 8, 2022

nmykhailets requested a review from kweinmeister March 8, 2022 14:21

kweinmeister approved these changes Mar 8, 2022

View reviewed changes

bcoe approved these changes Mar 8, 2022

View reviewed changes

bcoe merged commit fd95679 into googleapis:main Mar 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(samples): interactive tutorials code samples for CRUD products#149

chore(samples): interactive tutorials code samples for CRUD products#149
bcoe merged 2 commits intogoogleapis:mainfrom
nmykhailets:products-crud

nmykhailets commented Feb 18, 2022

Uh oh!

generated-files-bot Bot commented Feb 18, 2022

Uh oh!

snippet-bot Bot commented Feb 18, 2022 •

edited

Loading

Uh oh!

sofisl left a comment

Uh oh!

chenlei1216 Feb 25, 2022

Uh oh!

nmykhailets Feb 28, 2022

Uh oh!

chenlei1216 Feb 25, 2022

Uh oh!

nmykhailets Feb 28, 2022

Uh oh!

tetiana-karasova Mar 1, 2022

Uh oh!

kweinmeister left a comment

Uh oh!

t-karasova commented Mar 7, 2022

Uh oh!

t-karasova commented Mar 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

nmykhailets commented Feb 18, 2022

Uh oh!

generated-files-bot Bot commented Feb 18, 2022

Uh oh!

snippet-bot Bot commented Feb 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sofisl left a comment

Choose a reason for hiding this comment

Uh oh!

chenlei1216 Feb 25, 2022

Choose a reason for hiding this comment

Uh oh!

nmykhailets Feb 28, 2022

Choose a reason for hiding this comment

Uh oh!

chenlei1216 Feb 25, 2022

Choose a reason for hiding this comment

Uh oh!

nmykhailets Feb 28, 2022

Choose a reason for hiding this comment

Uh oh!

tetiana-karasova Mar 1, 2022

Choose a reason for hiding this comment

Uh oh!

kweinmeister left a comment

Choose a reason for hiding this comment

Uh oh!

t-karasova commented Mar 7, 2022

Uh oh!

t-karasova commented Mar 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

snippet-bot Bot commented Feb 18, 2022 •

edited

Loading