Skip to content
This repository has been archived by the owner on Dec 31, 2023. It is now read-only.

docs: add Admin API samples for account management methods #65

Merged
merged 14 commits into from May 27, 2021

Conversation

ikuleshov
Copy link
Contributor

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> 🦕

@ikuleshov ikuleshov requested a review from a team as a code owner May 19, 2021 20:54
@ikuleshov ikuleshov requested a review from leahecole May 19, 2021 20:54
@product-auto-label product-auto-label bot added api: analyticsadmin Issues related to the googleapis/python-analytics-admin API. samples Issues that are directly related to samples. labels May 19, 2021
@snippet-bot
Copy link

snippet-bot bot commented May 19, 2021

Here is the summary of changes.

You are about to add 10 region tags.

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

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 19, 2021
@ikuleshov
Copy link
Contributor Author

Region tag product prefix check is failing due to the product set as non-public in https://devrel.corp.google.com/admin/products/401. This is something that we are currently working on, but I believe the region prefixes are fine.

Copy link

@leahecole leahecole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stopped about halfway through because it was going to be obnoxious to see me say "same as above" on the tests - the samples themselves look great as far as I can tell, and the tests aren't bad as is (they're independent of each other and valid! Woo!), but I think they could be hardened with the use of fixtures. If you need it, this cloud storage test has some examples of function scoped fixtures and crud operations

def test_accounts_user_links_audit(capsys):
accounts_user_links_audit.audit_account_user_links(TEST_ACCOUNT_ID)
out, _ = capsys.readouterr()
assert "Result" in out

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would probably make sense to add a few more asserts for your other print statements just to be super sure they happen

TEST_EMAIL_ADDRESS = os.getenv("GA_TEST_EMAIL_ADDRESS")


def test_accounts_user_links_batch_create():

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to test the happy path and delete it afterwards with a fixture? I do like the creative 403 as an additional test

"""Runs the sample."""

# !!! ATTENTION !!!
# Running this sample may change/delete your Google Analytics account

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love this very helpful comment A+ DevX it caught my attention immediately

FAKE_ACCOUNT_USER_LINK_ID = "1"


def test_accounts_user_links_batch_delete():

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to the create, would love to see a test that uses a fixture to create then actually deletes and checks to see that the value is no longer there

TEST_ACCOUNT_ID, TEST_USER_LINK_ID
)
out, _ = capsys.readouterr()
assert "Result" in out

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the value you're expecting known? Could you add more to this assert if it's the same test value you're looking for every time?

(Should you add a value in a fixture that you then "get" in the test and delete in the teardown?)

FAKE_ACCOUNT_USER_LINK_ID = "1"


def test_accounts_user_links_batch_update():

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as others - would love to see a fake account that gets created and deleted in a fixture and updated in the test

@ikuleshov
Copy link
Contributor Author

Leah, to address comments above. Problem is, it is currently impossible to create a new Google Analytics account programmatically using a public API due to the need for a user to accept Terms of Services. The feature to provision a new account automatically will likely be implemented by end of Q3, so we might be able to do more interesting tests. Right now, I would prefer to not mutate the state of the test GA account since there is no easy way to reset it to the original state in case something goes wrong. As a result, the tests mainly check for the validity of the request proto for now.

@ikuleshov ikuleshov requested a review from leahecole May 27, 2021 23:43
@ikuleshov ikuleshov merged commit a3fecc4 into googleapis:master May 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: analyticsadmin Issues related to the googleapis/python-analytics-admin API. cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants