Error on publish-all API

How is this a critical bug, if CI is showing 0.x passing just fine? 😅

You first tried to make this change in ✨ [PP-1] Create api slice for auto-save data (pending changes) and publishing all data Postponed . I pointed out at https://git.drupalcode.org/project/experience_builder/-/merge_requests/4... how that's out-of-scope and irrelevant.

That MR then landed without this change, which quite literally proves my point in #4: that this cannot be a critical bug — a minor task to make this consistent, at most.

Then in ✨ [PP-1] Implement the "Publish All" button Postponed , you're again trying to make this change — that's how you interpreted my original remark 🙃 So I had to again point the same thing out: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...

So: again: this cannot be a bug, let alone a critical one — at least not until steps to reproduce are provided, or better yet: a test failing on CI proves it.

I don't see how this relates to 📌 Create an endpoint to publish all auto-saved entities Active at all — because multiple other route definitions use the exact same notation.

Are you sure this is not simply something you hacked in your development environment?

@soaratul and I synced up on this just now.
In standard Drupal routing, having a slash or not having a slash is no issue, and it's just preference (for context, Drupal Core precedes all of its routes with the slash).

However, when you use Symfony's autowiring, it has to match 1:1 with the definition in the services.yml file.
And those definitions must NOT contain a slash!

As I mentioned before, core's routes are prefixed with a slash, EXCEPT for the ones that are autowired (e.g. WorkspacesHtmlEntityFormController).

Additionally, all of the existing routes in XB are all correctly not using a prefixed slash.

If you add a slash to any of the existing XB autowired routes, you will get the same error from ClassResolver.

The slash was added in 📌 Create an endpoint to publish all auto-saved entities Active
The only reason that issue worked is because the test coverage instantiated ApiPublishAllController as a service, not using the routing system.

So as soon as ✨ [PP-1] Implement the "Publish All" button Postponed attempted to use it as a route, this error was hit.

My recommendation would be to merge this fix into the MR for the preview issue and close this one.

Great write-up, thanks!

Then I’m +1 to changing it, of course 😄

But -1 to doing it in that other issue: we should make it consistent, especially after the insight you just provided, to avoid running into this again.

It’s a trivial MR, so let’s just make nobody have to rediscover your excellent research 🤓🙏

(In fact, that’s all I’ve been asking all along, but before your research it was a minor clean-up task, now it’s a major maintainability bug!)

Targeted MR already exists here, is more needed?

Oh wow, this is a trivial DX improvement we should make in core to avoid others running into this issue, there's no reason why we should accept the prefixed controller and then fail with a mysterious error - it should either work, or error immediately.

#9++

Retitling to reflect what I have been asking @soaratul since the very start to make it consistent:

Can you please extract this into a separate MR where you make all these consistent then? 🙏

— https://git.drupalcode.org/project/experience_builder/-/merge_requests/4...

That's also what I was getting at in #7.

(Even though I now see there's only one other place 😅)

Automatically closed - issue fixed for 2 weeks with no activity.