Skip to content
This repository was archived by the owner on Jul 8, 2022. It is now read-only.

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Sep 17, 2021

Relates to the changes in:

By looking at the changes in the PRs listed above, I'm guessing organization_id has been in the response from the live API for a while, but we never updated this particular mock response because we hadn't needed the value till now.

In fact, I noticed that the Jetpack /me/sites version does include organization_id, so we can say that the value had been in the response at least since May 2021. See caa9d47

To Test

See wordpress-mobile/WordPress-iOS#17177 and notice that the UI tests pass in both Buildkite and CircleCI. The reason for the failures we've seen before this was that the tests crashed when trying to assign the nil value stored in the organizationId property serialized from the stub response that didn't include the key.

Relates to the changes in:

- wordpress-mobile/WordPressKit-iOS#445
- wordpress-mobile/WordPress-iOS#17154

By looking at the changes in the PRs listed above, I'm guessing the key
has been in the response from the live API for a while, but we never
updated this particular mock response because we hadn't needed the value
till now.

I noticed that the Jetpack version of me/sites does include
`organization_id`, so we can say that the value had been in the response
at least since May 2021.
See
caa9d47
Copy link

@diegoreymendez diegoreymendez left a comment

Choose a reason for hiding this comment

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

The changes looks good on my end.

Thanks @mokagio !

Copy link
Contributor

@loremattei loremattei left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mokagio mokagio merged commit 12b8aa5 into develop Sep 20, 2021
@mokagio mokagio deleted the add-organization-id-to-me-sites branch September 20, 2021 01:55
mokagio added a commit that referenced this pull request Sep 20, 2021
Note that in this instance we are not going through a beta because today
is also code freeze for WordPress iOS and I just merged the PR, #35,
that required this new version.

Basically, I'm taking a shortcut and not releasing a beta because I
would override that beta with a stable version immediately.
mokagio added a commit to wordpress-mobile/WordPress-iOS that referenced this pull request Sep 20, 2021
Now that wordpress-mobile/WordPressMocks#35 has
been merged and a new version shipped, we can point away from the WIP
branch and back to a stable version.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants