Revert "Full Site Editing Framework: Fix template resolution priorities." #27322
Conversation
This reverts commit 61b9345.
|
Do you why the test failed? Is the logic wrong? |
I have no idea why the test fails... |
|
Size Change: -10 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
This reverts commit dfe78fb.
|
I tried different approaches and I can confirm that this PR is indeed the culprit, it causes the test to fail. I think we should revert #27303 by merging this one, then reopen #27177 and figure out a better way to handle it. |
|
so if I'm understanding properly. The fix is correct but the test fails and we don't really know why. It could suggest an issue in the test itself in which case maybe we'd prefer to keep the fix while disabling the test temporarily. (though both approaches are decent, we just need to follow-up). I'm also pinging @noahtallen and @david-szabo97 as they worked on that particular test it seems. |
|
Taking a look at the test now |
|
The test is failing because when you create a custom template with the slug |
|
I didn't fix the root bug, but I did fix the test here: #27347. The test shouldn't break by these bugs, since it makes the test seem unreliable. (The test is not trying to test template resolution anyways.) Entity editing always seems different from template resolution, so people tend to merge PRs which break the test without realizing it! |
Why there is both a theme front-page template and a customized one, I thought once you customized a template, it just takes the theme one (auto-draft) and publish it? |
|
I think the root problem lies in the fact that Then, when we query gutenberg/lib/template-loader.php Line 133 in 57bf987 we add a second orderby param it will make the tests pass. I'm not sure if there is a reason for not having unique slugs, but it makes sense to me that we should. |
|
Here's how I'm thinking of it:
If number 2 exists, then we need to show your custom copy of the I'm not exactly sure what makes most sense in terms of storing the template. But in my mind, there is a difference between "I've created a custom template for this slug myself" and "I've modified the template which the theme provides".
Nik is definitely right, and to expand further, for the broken e2e test, we basically just need any published template to test dirty states with. It just so happens that the test creates and publishes a custom front-page template and tests with that (at the time, I don't know if the FSE theme used for e2e tests provided a front-page template, so it may not have been a concern at the time the test was written). I updated the test in #27347 to create an arbitrary template that should not cause any conflicts with whatever the theme is providing. IMO we can move forward with that to fix the broken test and continue to figure out how exactly templates should be stored and resolved in this situation! Again, the broken test isn't trying to cover any of the template resolution behavior, it just made a bad assumption about how template resolution would work in the way it sets up the test |
|
Thank you @noahtallen and @ntsekouras for debugging the tests! We should definitely figure out what to do with templates that have the same slugs though. |
Probably not! @noahtallen 's change in tests to decouple template resolution and multi entities editing were great and we're now missing some tests for template resolution. |
#27303 caused some tests to fail.
This PR reverts the changes. If merged then we should reopen #27177 and find a better solution