fix(gatsby-source-contentful): fixed id collision in contentful entries #23514
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
If anyone has any ideas as to why those unit tests fail that'd be helpful thanks. Nothing appears to be wrong in the tests, and the actual code functions correctly. |
|
Failures do seem unrelated. Maybe try merging master to this branch? |
me4502
force-pushed
the
fix/contentful-id-collision
branch
from
08a28a5
to
3995cd0
Compare
|
Seems it still fails, with an unrelated linting issue now too |
|
If you enable us to commit to the branch we could try to run our formatter. Currently you have to do it manually by running |
me4502
force-pushed
the
fix/contentful-id-collision
branch
from
3995cd0
to
e80d55a
Compare
|
The checkbox to allow committing to the branch is missing so I can't enable it sorry - I could move this to a branch on the official Gatsby project if that'd make it easier. I've just made some changes that fix the test issues within this PR, and also just fix ones that are in master so that it passes |
pvdz
reviewed
Apr 29, 2020
packages/gatsby-source-contentful/src/__tests__/__snapshots__/gatsby-node.js.snap
Show resolved
Hide resolved
me4502
force-pushed
the
fix/contentful-id-collision
branch
from
00ab480
to
81a1957
Compare
vladar
reviewed
May 4, 2020
There was a problem hiding this comment.
Great job and thanks for the PR!
I've added a couple of comments inline. Also, I think we need tests for this use case because there are certainly edge cases (sys.linkType vs sys.type; deleted entries; also I see foreignReferenceMap uses sys.id as a key which can possibly break with identical ids but different types).
me4502
force-pushed
the
fix/contentful-id-collision
branch
from
81a1957
to
4a9b0d9
Compare
|
I've just fixed the issues noted |
ascorbic
added
topic: source-contentful
vladar
reviewed
May 19, 2020
|
@axe312ger Should we wait for #15084 to get this in? I feel this one is an easier merge than #15084 |
|
I think both PRs are potentially breaking so it might make sense to include them both in a new major version of the plugin? What do you think? |
|
@vladar yes! Totally down for this! |
axe312ger
added
the
breaking change
axe312ger
pushed a commit
that referenced
this pull request
Jun 23, 2020
…es (#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
4 tasks
axe312ger
pushed a commit
that referenced
this pull request
Jun 30, 2020
…es (#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
axe312ger
pushed a commit
that referenced
this pull request
Jul 8, 2020
…es (#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
axe312ger
pushed a commit
that referenced
this pull request
Jul 8, 2020
…es (#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
axe312ger
pushed a commit
that referenced
this pull request
Jul 17, 2020
…es (#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
axe312ger
pushed a commit
that referenced
this pull request
Jul 17, 2020
…es (#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
axe312ger
pushed a commit
that referenced
this pull request
Jul 29, 2020
…es (#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
axe312ger
pushed a commit
that referenced
this pull request
Jul 29, 2020
…es (#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
axe312ger
pushed a commit
that referenced
this pull request
Aug 5, 2020
…es (#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
axe312ger
pushed a commit
that referenced
this pull request
Aug 5, 2020
…es (#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
axe312ger
pushed a commit
to axe312ger/gatsby
that referenced
this pull request
Oct 2, 2020
…es (gatsbyjs#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
axe312ger
pushed a commit
to axe312ger/gatsby
that referenced
this pull request
Oct 2, 2020
…es (gatsbyjs#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
6 tasks
axe312ger
pushed a commit
to axe312ger/gatsby
that referenced
this pull request
Oct 2, 2020
…es (gatsbyjs#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
axe312ger
pushed a commit
to axe312ger/gatsby
that referenced
this pull request
Oct 2, 2020
…es (gatsbyjs#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries
pvdz
pushed a commit
that referenced
this pull request
Oct 5, 2020
…e performance (#27244) * fix(gatsby-source-contentful): fixed id collision in contentful entries (#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries * fix(gatsby-source-contentful): properly resolve subsequent sync calls (#15084) * fix(gatsby-source-contentful): fixed id collision in contentful entries (#23514) * fix(gatsby-source-contentful): fixed id collision in contentful entries * improve logging and clean up code (#26238) * improve comment about contentful-resolve-response usage * test(unit): remove cross test data pollution * further improve loggin by replacing console.time with GatsbyReporter * improve code style * missing file * remove logging verbosity * adjust mocked reporter for tests to pass * remove unused deep-map dependency * fix: use content type id by default to create reference gql types (#26102) * temporary console.time calls for performance measurements * perf(gatsby-source-contentful): use a map and less loops to merge old and latest sync API response * log time Contentful API processing and GraphQL node creation * upgrade dependencies and use faster node 10 versions * remove duplicate logging * allow multiple Contentful sources fixes #26875 * refactor(imports): fix dependency structure and make sharp optional fixes #23904 * align tests to match new multi source cache pattern Co-authored-by: Matthew Miller <mnmiller1@me.com> Co-authored-by: Khaled Garbaya <khaled@contentful.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This resolves an ID collision in the contentful source plugin, as noted here: #22004 (comment)
Related Issues
Related to #22004