- Issue created by @bnjmnm
- πΊπΈUnited States bnjmnm Ann Arbor, MI
(I set the Component to "Miscellaneous" because I'm unsure what this actually is. Closest seems to be autosave but this is about publishing - a separate process that is predicated on autosave, but is not autosave)
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Thanks for the bug report!
It's
ApiAutoSaveController::post()
, which was originally calledApiPublishAllController
by @tedbow and @larowlan in π Create an endpoint to publish all auto-saved entities Active , so that seems the appropriate place. They wrote the logic + tests, so let's get their input. - First commit to issue fork.
- Merge request !975Issue #3521759 Bubble form errors when publishing β (Merged) created by larowlan
- π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Added a new issue this surfaces π Publishing the currently edit page causes 'another user has changed this content' error Active
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Test-only CI job running: https://git.drupalcode.org/project/experience_builder/-/jobs/5130397
This is , so over to @tedbow for review.
- First commit to issue fork.
- πΊπΈUnited States tedbow Ithaca, NY, USA
Looks good to be. Needs /ui approval
- πΊπΈUnited States bnjmnm Ann Arbor, MI
Spotted a few little things mentioned in the MR
- π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Addressed review and reinstated the alt text change needed for tests to pass
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
@bnjmnm's feedback has been addressed.
Also, Ben wrote:
Nice, happy to have this trojan horsed in here.
β¦ about the dreaded all of us have been seeing increasingly often lately.
So β¦ really wanted to merge this, but alas
entity-form-field-types-test.cy.js
is failing π - πΊπΈUnited States bnjmnm Ann Arbor, MI
Test fail appears due to the change that removed the The content has either been modified by another user..." issue see comment
- πΊπΈUnited States tedbow Ithaca, NY, USA
chatted with @wim leers myself or others(or wim) could review. I will look at this tomorrow if it still needs review
- πΊπΈUnited States tedbow Ithaca, NY, USA
This looks good to me but
publish-validation.cy.js
has a merge conflict I tried merge locally but then this test fails. Not sure if was merge conflict resolution or something we the actual test. I am not pushing my merge because it would probably just make thing worse -
bnjmnm β
committed 89865430 on 0.x authored by
larowlan β
Issue #3521759 by bnjmnm, larowlan, tedbow, wim leers: Publish checks...
-
bnjmnm β
committed 89865430 on 0.x authored by
larowlan β
- πΊπΈUnited States bnjmnm Ann Arbor, MI
Took care of the merge conflict and it looks like all other reviews were good with the changes so I'll go ahead and commit.