Looks like the same ask in ✨ Changes to session breaks access to masquerading user. Needs review but titled better
balintbrews → credited mglaman → .
MR up but test needs to be finished
Delete all doesn't exist and was just an idea pitched on #3504970-3: Publish all changes within a database transaction to allow rollbacks →
Thanks, everyone.
Approved by all codeowners
I read this and was confused, thinking it was the Page entity title, adding block to issue title
Needs tests, but we have a basic code change that could use review and feedback
Pushed https://git.drupalcode.org/project/experience_builder/-/merge_requests/6..., I can update the @todo when it is added. This feels like a perfect case for content moderation and a specific workflow. If the state is archived, we will not make published. Or if we do not want content moderation, it's another use for a trinary state field: 0=TO_BE_PUBLISHED, 1=PUBLISHED, 2=ARCHIVE. On "Publish all changes" all items in 0=TO_BE_PUBLISHED are published.
Approved by @wim leers
There are some big assumptions here. That ApiPublishAllController should take all entities which are \Drupal\Core\Entity\EntityPublishedInterface and call \Drupal\Core\Entity\EntityPublishedInterface::setPublished.
How can something remain "Archived" then? Since new entities cannot be staged, we are creating them as unpublished to start.
Added test
Thanks @wim leers – enforced on config makes more sense. I'll update the test
Makes sense to me. Approved.
Tossed up quick, need to manually test
Whoops didn't realize I closed this with a pending question 😅
That makes sense, and it is being done there. Should we keep this open to have createContent invalidate the "Content" tag work being done over there?
So maybe it is accidentally being published by that other bug? Probably fix that first then double check here
You're experiencing this bug https://www.drupal.org/project/experience_builder/issues/3503199 🐛 "Published" checkbox is always checked even if the entity is not Active
It's not actually published.
I pushed to the MR with changes to use AutoSaveManager for handling simple config changes. It's not 100% done but enough to review and validate as an approach.
I need to update the issue summary with changes on the approach from the MR
MR approved
CI is failing, but I'd rather get reviews on the approach before fixing.
Why not just let the last save win?
Perfectly acceptable to me!
Drupal Core added the cookie_samesite to session.storage.options service parameter. This should be used over an override by autologout. Same with the cookie_lifetime value.
That's one reason we have this in the PageTrait for tests: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/path/...
protected static function assertSaveWithoutViolations(Page $page): void {
// Path field is always invalid for new entities.
// @see \Drupal\path\Plugin\Field\FieldWidget\PathWidget::validateFormElement().
From Core:
$element += [
'#element_validate' => [[static::class, 'validateFormElement']],
];
and in validateFormElement
foreach ($violations as $violation) {
// Newly created entities do not have a system path yet, so we need to
// disregard some violations.
if (!$path_alias->getPath() && $violation->getPropertyPath() === 'path') {
continue;
}
$form_state->setError($element['alias'], $violation->getMessage());
}
}
Okay, I think this jumps the shark a bit as we have no way to stage configuration changes.
Approach 1: base field on xb_page
- There is a new "hidden"/internal boolean flag on pages called homepage
- Mark as homepage actually toggles a base field, which goes to auto save
- On preSave we see if homepage === TRUE, change system.site and remove the value of homepage
problem: how to prevent marking multiple as homepage in autosave data?
Approach 2: make ApiPublishAllController aware of frontpage concept
- the concept of what would be the new homepage cannot live in the entity data
- needs to be part of staged config changes
create a one-off snowflake concept in AutoSaveManager to track changes for the homepage (system.site.path.front) only
Setting credits
MR approved 🫡
Opened 📌 Define xb_path entity key for identifying entity path field Active to tackle this
mglaman → created an issue. See original summary → .
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.
I thought we had an issue about this, sorry! It's documented in https://git.drupalcode.org/project/experience_builder/-/blob/0.x/tests/s... and we ran into it while working on ✨ Creating a page generates the URL path dynamically when editing. Active
// Path field is always invalid for new entities.
// @see \Drupal\path\Plugin\Field\FieldWidget\PathWidget::validateFormElement().
The widget excludes the violation if it's a new entity. I was hoping JSON:API had a fix for this
Kicking this off today
Correct. I think we can close this. I opened tickets with an idea of how things would be developed and merged. Then things went differently and I forgot about this issue 😅. Closing as outdated.
We want the full URL generation. But subdirectory wasn't accounted for. This falls on laurii to decide.
mglaman → created an issue. See original summary → .
We have some problems here:
- The code needs to move into the editor, how should we handle transliteration? Should we bubble the settings into the XB mount so it is accessible via configSlice?
- How would we access and use the transliteration library within React?
- I'm assuming this code needs to live within InputBehaviorsEntityForm for the path input, but can we know the value of the title from there? What is the best approach?
- We can't actually publish these entities due to the fact path fields are always invalid for new entities! See \Drupal\path\Plugin\Field\FieldWidget\PathWidget::validateFormElement().
foreach ($violations as $violation) {
// Newly created entities do not have a system path yet, so we need to
// disregard some violations.
if (!$path_alias->getPath() && $violation->getPropertyPath() === 'path') {
continue;
}
$form_state->setError($element['alias'], $violation->getMessage());
}
}
I honestly don't know how to move forward here
MR is ready for an initial review!
Approvals in 🫡
This ended up being worked on mostly in ✨ Create a navigation modal for changing pages in the editor Active , I think we can close. And use ✨ [PP-1] Connect "new page" with API to create new pages Postponed and ✨ [PP-2] List available pages in editor navigation Postponed
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.
Our team has an upcoming ticket for this.
mglaman → created an issue. See original summary → .
Needs e2e tests
Now that ✨ Show page information in top bar Active we can put the component in that popover
Woo! We're unblocked to build the in-editor page navigation.
MR approved! here's my attempt at issue credits, other maintainers fix as needed!
🫡 ready for reviews
Created 📌 Followup: tokens for page title in SEO settings Postponed as the follow up. Tests were added.
mglaman → created an issue. See original summary → .
Going to help push this through over the finish line
Leaving in review for the backend code. But @attilatilman can you add a button to the editor's topbar?
In ui/src/components/topbar/Topbar.tsx let's add a "New" button right before <DemoPublishButton />
You'll need a new mutation for the onclick handler, see something like ui/src/services/preview.ts to POST to your new endpoint,
I'm not 100% sure how to handle routing to the new page once it has been clicked.
Ready for review. I'm going to ping laurii about:
If ✨ Open navigation modal by clicking on page title in XB navigation Postponed isn't ready, place the "New" button in the XB top bar.
I discussed with @amangrover90 and it is brushing on the edge of "can be difficult" and adjusting the scope of this issue.