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

rowStreamToOData(): reject unmatched repeatIds #1279

Merged

Conversation

alxndrsn
Copy link
Contributor

@alxndrsn alxndrsn commented Nov 8, 2024

@alxndrsn alxndrsn marked this pull request as ready for review November 26, 2024 05:45
@alxndrsn alxndrsn requested review from sadiqkhoja and removed request for sadiqkhoja November 26, 2024 05:47
@sadiqkhoja
Copy link
Contributor

Can we add an integration test as well

@alxndrsn
Copy link
Contributor Author

alxndrsn commented Dec 2, 2024

Can we add an integration test as well

I found it hard to find existing tests involving repeatId, so I've added a PR to address that at #1312.

branch: master

$ git grep repeatId -- test/integration/
[ no results ]

branch: #1312

$ git grep repeatId -- test/integration/
test/integration/api/odata.js:            body['@odata.nextLink'].should.have.skiptoken({ repeatId: 'b6e93a81a53eed0566e65e472d4a4b9ae383ee6d' });
test/integration/api/odata.js:            body['@odata.nextLink'].should.have.skiptoken({ repeatId: '7ac5f4d4facbaa9657c21ff221b885241c284b6c' });
test/integration/api/odata.js:            body['@odata.nextLink'].should.have.skiptoken({ repeatId: '52eff9ea82550183880b9d64c20487642fa6e60c' });
test/integration/api/odata.js:            body['@odata.nextLink'].should.have.skiptoken({ repeatId: '52eff9ea82550183880b9d64c20487642fa6e60c' });
test/integration/api/odata.js:            body['@odata.nextLink'].should.have.skiptoken({ repeatId: '52eff9ea82550183880b9d64c20487642fa6e60c' });

@alxndrsn
Copy link
Contributor Author

alxndrsn commented Dec 2, 2024

Can we add an integration test as well

@sadiqkhoja there don't appear to be any integration tests from the original implementation of odata skiptokens that involve repeatId values. Is that expected?

Edit: I think there is actually 1 integration test with a repeatId:

it('should limit subtable results', testService(async (service) => {
const asAlice = await withSubmissions(service, identity);
const nextlink = await asAlice.get('/v1/projects/1/forms/withrepeat.svc/Submissions.children.child?$top=2')
.expect(200)
.then(({ body }) => {
body.value[0].name.should.be.eql('Candace');
body.value[1].name.should.be.eql('Billy');
body['@odata.nextLink'].should.eql('http://localhost:8989/v1/projects/1/forms/withrepeat.svc/Submissions.children.child?%24top=2&%24skiptoken=01eyJyZXBlYXRJZCI6IjUyZWZmOWVhODI1NTAxODM4ODBiOWQ2NGMyMDQ4NzY0MmZhNmU2MGMifQ%3D%3D');
return body['@odata.nextLink'];
});
await asAlice.get(nextlink.replace('http://localhost:8989', ''))
.expect(200)
.then(({ body }) => {
body.value[0].name.should.be.eql('Blaine');
should.not.exist(body['@odata.nextLink']);
});
}));

@alxndrsn alxndrsn merged commit 54817a3 into getodk:master Dec 3, 2024
1 check passed
@alxndrsn alxndrsn deleted the odata-rowstream-reject-unmatched-repeat-id branch December 3, 2024 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants