Terms: Include only known terms in rendered selector

[aduth]

This pull request seeks to resolve an issue where posts with multiple non-hierarchical terms assigned will display empty term values in the term selector. Previously, we would render a term even if its name could not be determined (because it had not yet been fetched), resulting in an issue of conflicting key assignments, since the key is derived from the value, an empty string when not yet known.

With these changes, we only include the term in the selector if it's known. Since we explicitly request terms assigned to the post, this should work well enough, though by default this request would only return 10 terms. Therefore, it is possible that a post with more than 10 terms assigned would not display all terms in the selector. With these changes, I have increased this to 100, though this is still not a perfect solution since more than 100 terms for a post is technically possible. A more thorough solution would paginate results as necessary.

Since this component needs to be refactored with consideration of data module improvements (related: #5826), I am inclined to defer this to a future improvement.

Testing instructions:

Verify that with multiple tags assigned to a post, no empty tags appear in tag selector.

[jorgefilipecosta]

I confirmed this change fixes the bug of showing an empty tag in the beginning and the code looks good 👍

[bobbingwide]

Please can you explain what you mean by

This pull request seeks to resolve an issue where posts with multiple non-hierarchical terms assigned will display empty term values in the term selector.

With 2.5.0 the problem you appear to be referring to occurs when a tag or custom taxonomy contains a blank. e.g. "two words".

The problem occurs when only one multi word tag is assigned to the post.

I also noted that the request to fetch the 'missing' terms appears like this:

https://qw/oikcom/wp-json/wp/v2/tags?per_page=100&orderby=count&order=desc&_fields=id%2Cname&include=449&include=590&include=38

But the response only contains information for the last term.

[{"id":38,"name":"WordPress"}]

This is a backwards compatibility issue.

[bobbingwide]

This is a backwards compatibility issue.
It's not limited to Chrome.

[jorgefilipecosta]

Hi @bobbingwide thank you for your debugging and feedback,

Please can you explain what you mean by

This pull request seeks to resolve an issue where posts with multiple non-hierarchical terms assigned will display empty term values in the term selector.

What I tested that this PR solves is:
If we add 3 or more tags e.g: "aaa", "bbb" and "ccc" to a post then we save the post and open it again, without this PR we have a bug where the editor adds empty tags:

After this PR the problem does not happen.

[bobbingwide]

@jorgefilipecosta. I can confirm that I got the same problem that you tested using three one word tags.

And now I don't appear to be able to reproduce the problem that I thought I'd found. The chances are very high that I stumbled across the same problem and attributed it incorrectly. I first noted the problem with this taxonomy block, associated with content last updated 3 years ago.

4 multi word tags... but I had no explanation for the three word tag :-)

[jorgefilipecosta]

@jorgefilipecosta OK. Well that's just one of the problems then.

Yes, this is one of the problems not directly related to the requests so we were able to separate the improvement as it was needed no matter what changes happen in the request logic. The request logic will need to suffer a refactoring because of the paginated requests (dealing with +100 tags) during this time we will try to check the other bugs in the request system. Thank you for your inputs.

[aduth]

I also noted that the request to fetch the 'missing' terms appears like this:

But the response only contains information for the last term.

Nice find! It's likely it's only occasionally problematic because we fire requests both for a set of all terms, and those assigned to the post, and between the two, even with only one returned from "assigned to the post", most terms details are likely available for a majority of sites (except those with many tags).

The solution is an easy one, instead of:

&include=449&include=590&include=38

It should be:

include=449,590,38

I'll include a fix for this specific problem here. It highlights a DevEx concern over being an easy oversight, hopefully one which is more easily overcome with abstractions of the core data module planned to be introduced to these components.

[jorgefilipecosta]

Hi @aduth, thank you for including the fix to the requests already in this PR, I confirmed it works as expected. So the changes are still good to 👍

[bobbingwide]

Just to confirm I was able to produce the problem with only 2 terms attached to my post.