Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Facebook test client WIP, campaigns stream only #231
Facebook test client WIP, campaigns stream only #231
Changes from 13 commits
f9bb1ef
5d6ff16
05cfc4b
20d37ba
0356413
78004a6
8af6f4d
e497066
810f602
826b505
01ad519
6753872
668e165
5e8ef41
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more than enough for now if we have random pk values. For an all fields test we would need to expand this and it could get messy. Might need to think about that more in the future, but not necessary now.
Also, to think about for the future (and maybe for the pagination test) is if we should store and return the data that we get and create to compare to what the sync gets and to make sure it is getting the correct data. Not sure if our standard pagination test worries about that but probably not since the data is unknown ahead of time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would think for pagination to occur the number of records cannot be equal to the limit. If the limit is 100 and there are 100 then it won't go to a second page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not clear on what
response.get('paging', {}).get('next'):
does?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears as if the limit sent in the get request controls the number of records returned per page so I don't expect to get greater than 'limit' records back in a single response. I guess we should change this from ">=" to just "==". The "response.get('paging', {}).get('next'):" verifies that there is another page of data still left to get.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic should be in the
get_account_objects
. When the if statement is true we just continue to the next stream. Are we assuming because there is a next page, we have another record and are over the limit.