fix: fallback to template filename if sanitized name is empty

[mecaota]

What does this implement/fix? Explain your changes.

When we use non-ascii characters in all of template name, non-ascii characters type generated.

This PR is fix the issue by using file name.

Does this close any currently open issues?

fixes #2807, fixes #2492

Any relevant logs, error output, GraphiQL screenshots, etc?

(If it’s long, please paste to https://ghostbin.com/ and insert the link here.)

Any other comments?

…

Where has this been tested?

Operating System: docker

WordPress Version: 6.2

[jasonbahl]

@mecaota thanks for the PR! Will review shortly

[jasonbahl]

@mecaota so if we change the Types that are registered, we also need to change how the Template resolves here: https://github.com/wp-graphql/wp-graphql/blob/develop/src/Model/Post.php#L580-L584

We need to make sure we add some tests around this too.

If you're not comfortable writing tests, it would be good to at least write detailed steps to reproduce the current issue + the behavior after the changes, then we can help get some tests written

[jasonbahl]

I think it would actually be nice if we could add support for something like a GraphQL Type Name field on templates.

like:

<?php
/**
 * Template Name: My Templaté
 * GraphQL Type Name: MyTemplate
 */

[mecaota]

@jasonbahl

I think it would actually be nice if we could add support for something like a GraphQL Type Name field on templates.

That sounds good! I agree with that solution.
I would like to close this PR and remake it with your proposal.

[mecaota]

But, I am busy at the moment and it will be a while before I can fix....
Would you like to make an issue or work on it instead?

[justlevine]

I think it would actually be nice if we could add support for something like a GraphQL Type Name field on templates.

That sounds good! I agree with that solution. I would like to close this PR and remake it with your proposal.

@mecaota @jasonbahl a custom template header for GraphQL Type Name is an additive enhancement , but we still need to address the referenced bug where non-ASCII characters lead to empty type names be it via this PR or a different one.

[mecaota]

@justlevine OK, I'll leave the corrections in this PR ready for merging.

[mecaota]

@jasonbahl

We need to make sure we add some tests around this too.
If you're not comfortable writing tests, it would be good to at least write detailed steps to reproduce the current issue + the behavior after the changes, then we can help get some tests written

I'm sorry, I'm not familiar with PHP and I'm not familiar with writing integration tests, so I'd appreciate any advice.

Here are the steps I would like to test: I think it will be some sort of test to check if the generated schema is correct. This is because the error currently mentioned in each issue comes from an invalid schema.

1. Make template php file with non-ascii template name.
   *1 If wordpress have a create template function, it use, but I can't find.
   *2 This example use カスタムテンプレート、 it means Custom Template in Japanese, so it can be set other language like 自定義模板, 맞춤 템플릿, and so on.

like this page-custom.php

<?php
/*
Template Name: カスタムテンプレート
*/

?>

<section>
</section>

2. Run WPGraphQL::get_schema().
3. Check this schema is valid.

[odrowonz]

Yes, this PR recipe worked for me ( #2881 ). Many thanks, guys.

[odrowonz]

It is worked for me (#2881).

[mecaota]

@jasonbahl

This is reproduce method→ #2808 (comment)

CodeClimate has reported an issue, but this is not my implementation.
If I need to add a test for this, I think I'll add a test for non-ASCII character slugs to sample posts, categories, taxonomies, etc. However, this will cause many errors. I think it would be better to add the non-ASCII slug test after merging this.

Additionally, multiple similar issues have cropped up in the last few months since this PR was created, so it's clear that some people are in trouble. Please respond as soon as possible.

[justlevine]

Rebased and linted to fix the phpcs issue

[coveralls]

coverage: 84.635% (+0.08%) from 84.558% when pulling d5c72b6 on mecaota:hotfix/empty-name-to-filename into 4050581 on wp-graphql:develop.

[codeclimate]

Code Climate has analyzed commit d5c72b6 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	1

View more on Code Climate.

[justlevine]

@jasonbahl I did a bit of cleanup, and then added the necessary tests in d5c72b6.

Should be good to merge once you review

[justlevine]

#2808 (comment)

(changing status so I dont forget)

[justlevine]

@jasonbahl I made the requested changes, this should be good to go 🤞

[imjlk]

Well, the issue is definitely fixed for WordPress site using the GraphiQL IDE, but when working with the integration using various GraphQL clients(in my case, graphql-codegen features), I still encounter issues when the site locale is in non-ASCII format.
People can find the issue(#2492) and know about it, but wouldn't it be nice if there was a mention of it in the official docs?

[jasonbahl]

@imjlk can you open another issue with steps to reproduce? If there's still a bug with locales, we should track it and work toward a fix (or at least document it)

I'd love to know more about what you're experiencing and how to reproduce consistently