Skip to content

Conversation

@jamescdavis
Copy link
Member

Purpose

Resolve loadAll() / queryHasMany() compatibility.

Summary of Changes

  • Get headers for queryHasMany ajax call from adapter instance
  • In loadAll(), get pagination info directly from meta

Side Effects / Testing Notes

loadAll() works again?

Ticket

N/A

Reviewer Checklist

  • meets requirements
  • easy to understand
  • DRY
  • testable and includes test(s)
  • changes described in CHANGELOG.md

@jamescdavis jamescdavis force-pushed the misc/resolve-loadAll-queryHasMany-compatibility branch from 8a566a5 to cd2660c Compare January 22, 2018 17:14
Copy link
Contributor

@aaxelb aaxelb left a comment

Choose a reason for hiding this comment

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

Looks good! I suppose the fact this slipped through means we should probably have better tests around this... but in the interest of speedily fixing all the broken PRs, I think this is good.

@jamescdavis
Copy link
Member Author

@jamescdavis
Copy link
Member Author

Indeed! More tests!

@jamescdavis jamescdavis merged commit 790bee4 into CenterForOpenScience:develop Jan 22, 2018
@jamescdavis jamescdavis deleted the misc/resolve-loadAll-queryHasMany-compatibility branch January 22, 2018 20:04
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