SdcController cleanup tasks

Created on 5 June 2024, 24 days ago

Problem/Motivation

📌 [MR Only] provide UI foundation with Drag and Drop + panels Fixed aka https://git.drupalcode.org/project/experience_builder/-/merge_requests/33 added SdcController to provide a skeleton of backend listings.
In a retrospective review a few issues were flagged

Steps to reproduce

Proposed resolution

* Add typehinting to the closure in getComponentsList
* Make ::components return a CacheableJsonResponse
* Make ::component make use of ComponentPluginManager::find instead of building the full list only to throw it away
* Make ::component return a CacheableJsonResponse
* Make ::renderComponent use ComponentPluginManager::find
* Make ::renderComponent return a CacheableJsonResponse
* Make ::populatePropValues make use of early returns
* Move ::populatePropValues out of the controller into a service or singleton
* Add test coverage
* Use new permissions, not 'access administration pages'

Remaining tasks

User interface changes

API changes

Data model changes

📌 Task
Status

Active

Component

Page builder

Created by

🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

Live updates comments and jobs are added and updated live.
  • Novice

    It would make a good project for someone who is new to the Drupal contribution process. It's preferred over Newbie.

Sign in to follow issues

Comments & Activities

  • Issue created by @larowlan
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
  • Assigned to larowlan
  • 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺

    Thanks for the review!

    • I think everybody would agree all of the things you listed should happen eventually.
    • But they should not be required to happen now.

    We need to be able to iterate quickly, without accumulating enormous MRs like we tend to do in Drupal core. In Drupal core this makes sense because it's already a consistent whole, but that's not yet true for Experience Builder.
    In other words: for XB, we intentionally want to avoid the "every commit must be perfect" strategy that Drupal core uses.

    I acknowledge this means outside participation is more difficult: what is in a "ready for detailed review" state vs "slapped together, don't bother to review" state? So I think that is the challenge we should solve rather than disallowing imperfect/incomplete MRs to be reviewed.

    So I propose to introduce a way to convey this. Ideas:

    1. an impossible to miss
      // ⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️
      // ⚠️ 🔨🧹  This file is an early iteration. Do not review in detail yet.   🧹🔨 ⚠️
      // ⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️
      

      at the top of the file

    2. Issues like the one you created here make for excellent Novice tasks! 😊 So it's not like this is a wasted effort at all. Perhaps we should even intentionally call this out, by adding comments like // @todo Novice: inject service, // @todo Novice: introduce new permission, et cetera?
    3. introduce src-rough (back end) and ui/src-rough (front end) directories
    4. … something else?
Production build 0.69.0 2024