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

Taxonomies: Fix error when a taxonomy has no attached post type #9744

Merged
merged 1 commit into from Sep 11, 2018

Conversation

Projects
None yet
4 participants
@youknowriad
Contributor

youknowriad commented Sep 10, 2018

Just a small tweak to ensure we don't thrown an error when the "types" assigned for a given taxonomy is null (instead of an empty array)

closes #9693

The problem here though is while I'm certain this will fix the issue in #9693 I was not able to reproduce on my own and I'm still not certain if it's valid to have a taxonomy without types.

@jorgefilipecosta

I'm also not able to reproduce the error referred in the issue. But these changes test well and they should fix the error 👍

@youknowriad youknowriad merged commit 8129584 into master Sep 11, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@youknowriad youknowriad deleted the fix/no-types-per-taxonomy branch Sep 11, 2018

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Sep 11, 2018

Member

the "types" assigned for a given taxonomy is null (instead of an empty array)

Should this be expected, vs. being an empty array? It's documented as always returning an array.

https://developer.wordpress.org/rest-api/reference/taxonomies/#schema

Member

aduth commented Sep 11, 2018

the "types" assigned for a given taxonomy is null (instead of an empty array)

Should this be expected, vs. being an empty array? It's documented as always returning an array.

https://developer.wordpress.org/rest-api/reference/taxonomies/#schema

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Sep 11, 2018

Contributor

@aduth Agreed ideally it should be an array but here's my thinking here:

  • I can't reproduce the error, which means I can't debug why it's not an array
  • This has been confirmed to fix a bug at least two people are seeing
  • On its own, it seems like a reasonable thing to do (use lodash) regardless of whether if fixes the issue or not (it could be considered a performance improvement).
Contributor

youknowriad commented Sep 11, 2018

@aduth Agreed ideally it should be an array but here's my thinking here:

  • I can't reproduce the error, which means I can't debug why it's not an array
  • This has been confirmed to fix a bug at least two people are seeing
  • On its own, it seems like a reasonable thing to do (use lodash) regardless of whether if fixes the issue or not (it could be considered a performance improvement).
@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Sep 11, 2018

Member

Sure, but how can we plan to follow up about the REST API issue?

Member

aduth commented Sep 11, 2018

Sure, but how can we plan to follow up about the REST API issue?

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Sep 11, 2018

Contributor

The thing is I don't really know if it's a REST API issue or a Core Data issue? Would be good to find a way to reproduce.

Contributor

youknowriad commented Sep 11, 2018

The thing is I don't really know if it's a REST API issue or a Core Data issue? Would be good to find a way to reproduce.

@mtias mtias added this to the 3.9 milestone Sep 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment