Skip to content
New issue

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

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

Already on GitHub? # to your account

salesforce: autofetch is broken #644

Closed
josephjclark opened this issue Jun 24, 2024 · 4 comments · Fixed by #699
Closed

salesforce: autofetch is broken #644

josephjclark opened this issue Jun 24, 2024 · 4 comments · Fixed by #699
Assignees

Comments

@josephjclark
Copy link
Collaborator

josephjclark commented Jun 24, 2024

The autofetch feature on the salesforce query sort of works but also not really.

This PR #478 introduced a few regressions:

  • Referneces is nested, so you have to do [0][0]
  • The adaptor pushes responses into references, not recerds, So the data isn't really even accurate

Basically autofetch needs to push the RECORDS into a single flat results array

Not to mention that the code is a bit quirky, to say the least.

I'd also ask whether callback should be triggered per batch, or at the end result. I think that when you're talking about a batch processing API it make sense to chunkup the callback or else you're likely to hit a problem downstream. Maybe this is an option. This is something that comes up on many APIs so I'd want to think carefully about what we do here.

@github-project-automation github-project-automation bot moved this to New Issues in v2 Jun 24, 2024
@josephjclark
Copy link
Collaborator Author

And there are no unit tests on any of this functionality

@mtuchi
Copy link
Collaborator

mtuchi commented Jul 23, 2024

EOD update

I have started working on this but i need @josephjclark input. I think we shouldn't combine records into a single array because the other metadata will be lost (See screenshot attached below 👇🏽 ).
If we can rule out that the other metadata Eg: done, nextRecordsUrl, totalSize are not important when we use autoFetch then yeah we can merge the records into a single array

Screenshot 2024-07-23 at 5 42 07 PM

@josephjclark
Copy link
Collaborator Author

The other metadata is not useful when using autofetch. We'll use that stuff internally but there's no point feeding it back to the user.

@aleksa-krolls
Copy link
Member

Agreed with Joe's comment.

@mtuchi mtuchi linked a pull request Jul 29, 2024 that will close this issue
3 tasks
@github-project-automation github-project-automation bot moved this from Ready to Done in v2 Jul 31, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants