- Issue created by @effulgentsia
- 🇬🇧United Kingdom catch
Does this mean that experience builder is guaranteeing an upgrade path for its config from January 13th?
Or if not what happens to Drupal CMS sites that leave experience builder installed and then update it later?
- 🇺🇸United States effulgentsia
They'll probably need to uninstall XB and reinstall it. Or perhaps XB could provide an upgrade path to do the equivalent. The XB demo_mode will disable the ability to publish and will only allow editing a single Page (not create new pages), so wiping all XB content and config won't result in the user losing much.
- 🇬🇧United Kingdom catch
They'll probably need to uninstall XB and reinstall it.
That's not enough - if they update the module prior to uninstalling, especially if this happens with other core or contrib updates, it could very easily result in update errors and an unrecoverable site.
Why not provide a demo when an upgrade path (if only for config) is possible, rather than rushing it in and potentially breaking people's sites?
- 🇺🇸United States effulgentsia
Would your concern be addressed if we did commit to providing an upgrade path that deletes all XB config and content and reinstalls the new default config?
- 🇬🇧United Kingdom catch
Would your concern be addressed if we did commit to providing an upgrade path that deletes all XB config and content and reinstalls the new default config?
If XB adds an update path to uninstall itself, that will happen against the new xb code base, not the old one.
So if for example a config entity type is renamed, then I don't think Drupal will be able to successfully delete the old config entities as part of the uninstall process. See ConfigManager::uninstall() which relies on loading the entities to delete in order to delete them.
It would also have to happen every time that xb releases a new version. If a site skips a couple of xb releases, then updates, then it will have to uninstall and reinstall xb multiple times.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Everything @catch is describing is why I've always been skeptical of including a demo-only version of XB in Drupal CMS when XB clearly isn't in a place yet where it can provide an upgrade path — too many things left to be figured out.
I was told "demo mode does not imply upgrade path, people will need to reinstall". Is that not true? 🤔
#7: TIL Drupal loads config entities prior to deleting them. Interesting for sure :D There's one work-around I could think of: nothing will depend on XB config entities, except XB config entities. So we could in XB do a one-off "update path" that bypasses the usual config entity deletion process.
- 🇫🇮Finland lauriii Finland
I was told "demo mode does not imply upgrade path, people will need to reinstall". Is that not true? 🤔
It's fine to require a reinstall of the Experience Builder module, but it would not be a whole site reinstall. It would be fine to require a step that removes everything related to XB and starts fresh before it could be used outside the demo mode once we release 1.0. However, we should make this really simple for user, i.e. users should not have to run CLI commands or go modify their database manually. 😅
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Uninstalling XB would uninstall all config (simple + entities) without any extra steps, just like any other module.
But @catch wrote in #3 + #5 + #7 is implying it's not acceptable for the module to be uninstalled; i.e. that
composer update
should work. Based on #9, @lauriii disagrees with that.@catch: thoughts? :)
- 🇬🇧United Kingdom catch
Unless someone has a way to prevent composer update from running when experience builder is installed, and then clearly inform a site owner that they need to uninstall experience builder before they can update their site, then it seems pretty clear to me that composer update needs to work with experience builder installed. (And even if that could be done, it would be very poor and confusing UX to require manual uninstall before updates). This is the big reason we have alpha vs. beta experimental modules in core, to avoid this exact situation and the incredible maintenance burden and complexity it adds to trying to build new features - you only want to support an upgrade path, when you actually want to support an upgrade path, and never before.
@effulgentsia's suggestion in #6 is that experience builder provides an upgrade path that deletes all of its config then reinstalls it, but I've already pointed out at least two issues with that in #7. The new code base would definitely have to support deleting the old config, and that can't be guaranteed, and updating from say 0.2.0 to 0.6.0 could result in the same logic running multiple times. xb will have to do the same thing in every version it releases, until it starts doing 'real' upgrade paths. So if there are 6 releases between now and then, it could be trying to delete and recreate config six times in one update.php/drush updb run.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Can we make XB an "alpha-experimental" feature of Drupal CMS?
- 🇬🇧United Kingdom catch
@Wim 'alpha experimental' means that the module is included in the development branch - at the moment that would be 11.x, but not in any release branches.
At the point where we branch for the next minor release, e.g. 11.2.x, if the module is still alpha, it would get removed from 11.2.x (either at that moment or before alpha/beta) so that it never finds its way into an 11.2.0 tagged release.
(We have a current exception in core, which is package_manager at alpha stability in 11.1, but we made it impossible to enable package_manager without manually setting something in settings.php, and also made it hidden etc., to make it clear it's only there for use on development environments. This is because automatic_updates needs tagged releases to update between, so to manually test package_manager updating itself in core, it needs to be in a tagged release (at least without a lot more messing about than a settings.php flag). Also package_manager doesn't really store any data as such, so the upgrade path issue is moot even if someone managed to install it in production.)
When an alpha stability module reaches beta, then we stop removing it from release branches and it gets released in e.g. 11.2.0 (as a beta stability module, with upgrade paths supported from that point onwards).
The only way this concept would help in this case is:
1. Follow the same pattern and wait until XB is beta.
2. Put XB in Drupal CMS 1.1.0-alpha1, 1.2.0-alpha1, 1.3.0-alpha1 etc., but never in a beta/rc/stable release until XB itself reaches beta.
For #2, that would allow someone to download the alpha release, play around with it, but crucially, not have the expectation that they can then continue to build a site on it without problems. Although to make that a reality would mean defining 📌 [policy] What do our stability levels mean? Active clearly.
- 🇺🇸United States tim.plunkett Philadelphia
See 📌 Create a specialized recipe to hold the XB demo Active , still need XB people to sign off on the deleting part (line 64 to 86 of https://git.drupalcode.org/project/drupal_cms/-/merge_requests/387/diffs...)
- 🇺🇸United States effulgentsia
I added 📌 Add no_ui=TRUE to XB's field type definition Active and 📌 Move the addition of the XB field on the Article bundle to a hidden submodule Active as child issues, and to the list in the issue summary. The combination of them will keep people from being able to use XB on anything without some other code enabling it somewhere. In the case of Drupal CMS, that enabling code is the creation of a single Page (not node) entity as part of its recipe. In the case of XB developers/testers, that enabling code would be the installing (via Drush) of either the
xb_test_node
orxb_test_page
(hidden) modules.In the case of Drupal CMS, this means that when we later change the field schema of the field (which we likely will), we can just delete the single page entity, and not worry about the XB field being present anywhere else.
- 🇬🇧United Kingdom longwave UK
📌 Add JavaScript build artifacts to tagged releases Active is a blocker assuming we don't want Drupal CMS users to be manually running
npm run build
before they can use XB. - 🇺🇸United States effulgentsia
Added 🐛 Empty global regions add unnecessary spacing to preview Active and 🐛 [Needs design] Previews of pages containing (dynamically) empty blocks are malformed Active as blockers, since we don't want the first tagged version since DrupalCon Barcelona and the first version in Drupal CMS to look immediately broken.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Did @phenaproxima in 📌 Create a specialized recipe to hold the XB demo Active single-handedly work around all concerns surfaced here? 😮🤞 @catch, thoughts?
@effulgentsia Are you basically working through the goals+tasks outlined in https://docs.google.com/document/d/1ld-VuwRt0IZ0cmCL75Zth02Rtf2Nd3hyZrFX...? (@lauriii just pointed me to that doc in a private Slack.)
- 🇬🇧United Kingdom catch
@wim leers to be entirely honest:
- I don't understand the motivation to add a read-only demo to a site building distribution that site builders can't actually do anything with. There could be an announcements feed post with a link to a tugboat demo or something that would achieve much the same thing, and not require Drupal CMS site owners to deal with any unexpected bugs.
- it seems like a lot of comple, potentially fragile and destructive, and untested, code to add a week before a stable release - the version of that MR before I reviewed it would have uninstalled stable versions of XB. If that hadn't been caught, it would have been a time bomb. I'm not even sure what the mechanism is to update the composer plugin if a bug is found.
- there are serious user-facing issues like 🐛 Hint missing for Allowed memory size of x bytes exhausted Active 📌 RecipeRunner::processInstall() installs modules one by one Active 🐛 Drupal\project_browser\Element\ProjectBrowser::getDrupalSettings() is uncached Active (multiple second waits and out of memory errors in project browser when you try to browser and install recipes, to give just one example from this week) that the multiple people and hours involved in this issue could have been spent on instead. Or for that matter on experience builder beta blockers.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
As described in #3497990-6: The XB canvas incorrectly renders blocks (and their corresponding margins as specified by the theme's CSS) that usually have empty content for most/all site visitors → , I believe the work-around proposed by @effgulgentsia over there is acceptable short-term but not sustainable. I propose to do 📌 PageTemplate: allow configuring which regions are exposed Active instead. If @effulgentsia and @lauriii disagree, then a sibling issue should be opened that literally does what @effulgentsia proposed at #3497990-2: The XB canvas incorrectly renders blocks (and their corresponding margins as specified by the theme's CSS) that usually have empty content for most/all site visitors → .
- 🇺🇸United States effulgentsia
I added these to the requirements:
- 📌 When demo_mode, remove the "Use Experience Builder for page templates in this theme" checkbox Active
- 📌 When demo_mode, limit component list to only SDCs provided by the active theme (and not subthemes) Active
- 📌 When demo_mode, limit component list to only SDCs, not blocks Active
I also added ✨ Consider whether to store Component entities outside of config when in demo_mode Active as a nice to have.
- 🇺🇸United States effulgentsia
I moved 🐛 Empty global regions add unnecessary spacing to preview Active and 🐛 [Needs design] Previews of pages containing (dynamically) empty blocks are malformed Active from required to nice-to-have, because they're not surfaced unless the "Use Experience Builder for page templates in this theme" checkbox is selected, and 📌 When demo_mode, remove the "Use Experience Builder for page templates in this theme" checkbox Active is in the required list.
I left them as nice to have, because some people might install 0.2.0 outside of Drupal CMS, and fixing those "things look broken right away" bugs would be great to do for those users.
- 🇺🇸United States effulgentsia
Added another nice to have: 📌 When demo_mode, add messaging in the UI explaining what it is, what its purpose is, and maybe a link to the issue queue Active .
We might come up with some other requirements or nice-to-haves in the next day or two, but I think the list is mostly complete at this point.
There might also be other tasks for Drupal CMS itself, but this issue is just tracking the XB-specific stuff.
- 🇺🇸United States effulgentsia
The following might be moot depending on what happened with #19 and #20, but in case it's not moot, here's a response to #7:
This doesn't get into any configuration which xb might affect but doesn't own, but if there is any, that's another massive can of worms.
With 📌 Add no_ui=TRUE to XB's field type definition Active and 📌 Move the addition of the XB field on the Article bundle to a hidden submodule Active , we won't have any. Those field definition ones would be the worst ones to deal with, but those issues should remove that concern. With demo_mode, the only XB field definition will be the one in
Page::baseFieldDefinitions()
, and that's code, not config.So if for example a config entity type is renamed, then I don't think Drupal will be able to successfully delete the old config entities as part of the uninstall process.
With the various other issues in the requirements list, I think we'll be down to no saved config entities (while in demo_mode) other than
Component
. ✨ Consider whether to store Component entities outside of config when in demo_mode Active might be a way to safeguard that one. - 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Responded to all new issues in #23 + #24 + #25.
#26 first half (in response to #7): agreed.
#26 second half (regarding renaming config entities): disagreed. That's a problem prevented/solved by #3497781. As of 📌 Create a specialized recipe to hold the XB demo Active ,
composer update drupal/experience_builder
within Drupal CMS will uninstall the XB module first, before updating the XB code.And uninstalling the XB module also uninstalls all XB config entities (thanks you, config dependencies!).
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Per #3497990-8: [later phase] [Needs design] Previews of pages containing (dynamically) empty blocks are malformed → , 📌 PageTemplate: allow configuring which regions are exposed Active supersedes #3497990 in implementing @effulgentsia's proposal at #3497990-2: [later phase] [Needs design] Previews of pages containing (dynamically) empty blocks are malformed → . Reflecting that here too.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Closed 📌 When demo_mode, limit component list to only SDCs provided by the active theme (and not subthemes) Active as working as designed, because @phenaproxima did 📌 Disable default XB components Postponed , which will gives Drupal CMS' recipe far greater control 👍
Related: should we disable the entire "XB Component admin UI"? i.e.
/admin/structure/component
? I bet that's something worth doing, otherwise enterprising users could simply go and re-enable additional SDCs there. Or do we want them to be able to do that, given that as described in #27, they can't do any real damage with that?I defer to @lauriii & @effulgentsia.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
The required 📌 Move the addition of the XB field on the Article bundle to a hidden submodule Active also just landed.
That leaves only:
- 📌 When demo_mode, remove the "Use Experience Builder for page templates in this theme" checkbox Active — which seems totally doable, even though I am proposing a slightly different scope at #3498334-5: When demo_mode, remove the "Use Experience Builder for page templates in this theme" checkbox → , which is actually simpler to implement while demoing more
- 📌 When demo_mode, limit component list to only SDCs, not blocks Active — has been ready for ~4 hours, should be trivial to land
… at which point we'll have achieved all the required ones, plus a nice-to-have ( 🐛 Empty global regions add unnecessary spacing to preview Active , which used to be required until last night) and a NR/RTBC'able nice-to-have ( 📌 PageTemplate: allow configuring which regions are exposed Active , which also was required until last night).
That means we are now in pretty good shape for Monday, assuming that the list doesn't grow further.
- 🇺🇸United States effulgentsia
should we disable the entire "XB Component admin UI"? i.e. /admin/structure/component?
Yes, I think we should remove/disable that route when in demo_mode. Nice catch, sorry I didn't catch that earlier.
- 🇺🇸United States effulgentsia
Another question: should we make experience_builder module itself a hidden module (add
hidden: true
to its info.yml)? Otherwise, a Drupal CMS user could enable it viaadmin/modules
without going through the recipe. Which wouldn't necessarily be terrible in that they wouldn't be able to actually use it that way (without a page entity getting created which there's no UI for creating it), but would that interfere with #27? - 🇬🇧United Kingdom longwave UK
@effulgentsia I think we should at least label it as
lifecycle: experimental
. - 🇺🇸United States effulgentsia
It was decided in 📌 Remove the XB demo from the 1.0.x branch Active to not ship an XB demo recipe in Drupal CMS 1.0, and instead to have a Drupal CMS 1.1 release in February. Therefore, updating the issue title and summary here to reflect that.
This gives us an opportunity to polish up some of the UI around recently landed XB features (like editing content in global regions, placing and configuring blocks, and others), so perhaps we'll be able to include those even with demo_mode=TRUE for XB 0.2.0 / Drupal CMS 1.1.
If we end up getting all of 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active done before Drupal CMS 1.1, then that will end up being 0.2.0 and the 0.3 number will become available for new scope. But for now, I'm keeping this issue and that one separate, since we might need or want a 0.2 release ahead of completing everything from that other issue.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
📌 PageTemplate: allow configuring which regions are exposed Active is also done, which makes my disagreement with #3498334 only stronger. See #3498334-5: When demo_mode, remove the "Use Experience Builder for page templates in this theme" checkbox → for a counter-proposal.