Provide an API for listing available pages

amangrover90 on my team is picking this up, assigning to myself since I can't assign to him

#2 fixed that :)

amangrover90 → changed the visibility of the branch 3500052-provide-an-api to hidden.

amangrover90 → changed the visibility of the branch 3500052-provide-an-api to active.

FYI @amangrover90: once you want me (or somebody else) to review this, please change the d.o issue status to — I check that multiple times per day! :)

Sure @wim-leers.

After rechecking the designs, the navigation shows the page title and its path. Can we have the API also include the path for the page.

- The route must have a requirement for entity access to update pages, xb_page.update

This bit actually means that it's not just a list, it's a subset of the total list!

✨ Provide a way to create a new page Active is in!

Marked because:

• Some of the same problems present in #3500046's MR are present here too.
• This MR doesn't yet do what this issue's title and summary say WRT "updatable entities"
• I'm confused by #3500046 had the ambition to be as xb_page-agnostic as possible but not this one? 🤔
• Concerns about the way pagination is currently implemented.

FYI, personally I don't think pagination must be implemented in this issue — it could be implemented in a follow-up. This issue is meant to get things off the ground, and IMHO it's fine to crash hard when you have e.g. 100 xb_page entities during these early stages. We do the same thing for \Drupal\experience_builder\Controller\ApiConfigControllers::list()!

IOW: pagination is important in production, not in early development.

I'm confused by #3500046 had the ambition to be as xb_page-agnostic as possible but not this one?

Because creating an entity is more easily made generic than listing entities. Because pages do not have a bundle. But XB will support nodes, taxonomy, etc. And per the design there is a grouping for Pages and then other CMS content. So the assumption has been multiple endpoints.

Fixed a bunch of things. Notably, the OpenAPI schema was wrong: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5....

🏁 in sight!

3 bits remain:

1. missing list cache contexts — https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...
2. unclear why this response structure is different from other "list" responses — https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...
3. missing cacheability test coverage — https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...

wim leers → credited larowlan → .

I hear @larowlan and @mglaman's concerns about the introduction of the routes+logic in this MR instead of "just" using JSON:API. @larowlan:

I think we're reinventing the wheel a fair bit here and setting ourselves up for future performance and scalability issues.
I think we should consider using JSON:API for this because it already supports things like filtering, includes and pagination.
If we don't want to do that because we don't want the dependency, I think we at least need to allow for pagination.

@mglaman +1'd this.

Here's why I'm okay with it for now, while still sharing those concerns:

I don't disagree with this at all … but that comes with significant security caveats etc. This is literally the first "internal HTTP" issue/MR where this awkward position becomes truly painful:

1. All config entity-related HTTP API routes are basically _not possible_ with JSON:API, because ✨ [PP-2] POST/PATCH config entities via REST for config entity types that support validation Needs work is not fixed. (Note that I modeled XB's logic closely after that!)
2. The "create Page content entity" HTTP API route that ✨ Provide a way to create a new page Active added is doing something quite odd and special case-y, with it setting 2 required base fields' values. Which BTW only works for that single content entity type!

So far the amount of overlap/duplication is minimal. I think that if it grows significantly, we may need to consider changing that.

If we manage to keep XB's "content entity HTTP API responses" scoped to only high-level metadata, as this MR currently does, then the "damage" remains limited. If we'd start to expose all field data too, well … then I'd agree we SHOULD use JSON:API instead, and then figure out all the security consequences that come with it.

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

• The MR now looks fantastic compared to a week ago! 🤩👏
• Fixed the incorrect handling of subdirectory-installed Drupal/XB: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...
• Related: added missing handling of cacheability of generated URLs: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...
• Addressed #24 to the best of my ability within the current constraints: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...

⚠️ Note: I suspect that what y'all actually wanted, is the stored path alias for a Page content entity. But using $entity->toUrl() inevitably results in prop URL generation, which means that you may get /subdir/page-1, not /page-1.

@mglaman, my hunch is right, can you please open a new issue to ensure that the HTTP API for listing Page content entities always returns stored aliases, and does not perform full URL generation using \Drupal\Core\Routing\UrlGenerator::generateFromRoute()? 🙏

Ugh, it looks like there is a consistent failure in 0.x now, which is affecting this MR too: drag-and-drop.cy.js publish-button.cy.js 😞

This is happening since ✨ HTTP API for code component config entities Active .

But there’s literally zero changes to API responses being tested in there!

Possible explanations AFAICT:

• Since that MR, API responses are now slightly faster or slower
• GitLab CI infra hardware or load has changed
• d.o GitLab template updated Chrome

Today, I feel like I have to prioritize landing remaining MRs, and then tomorrow I can spend time digging into this — unless somebody with more experience beats me to it. (See last comment on #34999931 for research so far.)

@mglaman assigning to you for the bits at the bottom of #25.

We want the full URL generation. But subdirectory wasn't accounted for. This falls on laurii to decide.

So … that means that my hunch was right, and we essentially want to be able to do:

diff --git a/tests/src/Functional/XbContentEntityHttpApiTest.php b/tests/src/Functional/XbContentEntityHttpApiTest.php
index 2e71d19e..7638f169 100644
--- a/tests/src/Functional/XbContentEntityHttpApiTest.php
+++ b/tests/src/Functional/XbContentEntityHttpApiTest.php
@@ -119,19 +119,19 @@ final class XbContentEntityHttpApiTest extends HttpApiTestBase {
           'id' => 1,
           'title' => 'Page 1',
           'status' => TRUE,
-          'path' => base_path() . 'page-1',
+          'path' => '/page-1',
         ],
         '2' => [
           'id' => 2,
           'title' => 'Page 2',
           'status' => FALSE,
-          'path' => base_path() . 'page-2',
+          'path' => '/page-2',
         ],
         '3' => [
           'id' => 3,
           'title' => 'Page 3',
           'status' => TRUE,
-          'path' => base_path() . 'page-3',
+          'path' => '/page-3',
         ],
       ],
       $body

?

To _me_ we'd want to show the base path because that's the site and it's a super edge case to use Drupal in a subdirectory. But laurrii would like it removed.

Yeah, I know it's edge case to use Drupal in subdirectory. However, we're not showing the base path when we show aliases elsewhere (e.g., on the right sidebar) and we should remain consistent with that.

Opened 📌 Define xb_path entity key for identifying entity path field Active to tackle this

Great! Thanks for thoroughly following through, all! 👍

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