Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds isMobileSegment property to VerticalSegmentModel #1089

Merged
merged 2 commits into from Jan 22, 2019

Conversation

oguzkocer
Copy link
Contributor

This PR implements the FluxC side of wordpress-mobile/WordPress-Android#8999 by adding isMobileSegment property to VerticalSegmentModel. The endpoint returns this property as mobile, but I think this name makes it clear. I am open to suggestions for an even better name.

I've also re-enabled the limit check for the verticals connected test. This basically tests that the limit parameter for the /verticals endpoint is respected which has been recently fixed on the server-side.

To test:

  1. Run all the connected tests in ReleaseStack_VerticalTest
  2. Debug the ReleaseStack_VerticalTest.testFetchSegments by putting a breakpoint in ReleaseStack_VerticalTest.onSegmentsFetched and observe that every segment except for Online Store has its isMobileSegment set to true.

@kwonye
Copy link
Contributor

kwonye commented Jan 22, 2019

Tests pass and I've verified Online Store is the only segment where isMobileSegment set to false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants