Part of #1945406: [meta] Convert all of confirm_form() to ConfirmFormBase

CommentFileSizeAuthor
#5 openid-1946438-5.patch5.26 KBtim.plunkett
#5 interdiff.txt535 bytestim.plunkett
#3 openid-1946438-3.patch5.22 KBtim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Assigned: Unassigned » tim.plunkett

Working on this.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Active » Postponed

This needs #1947432: Add a generic EntityAccessCheck to replace entity_page_access()

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Postponed » Needs review
FileSize
openid-1946438-3.patch5.22 KB

Okay, that's all in now.

Status: Needs review » Needs work

The last submitted patch, openid-1946438-3.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review
FileSize
interdiff.txt535 bytes
openid-1946438-5.patch5.26 KB
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Perfect!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 08a5842 and pushed to 8.x. Thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Fixed » Needs work

Reverted due to the database connection serialisation issue see #1953800: Make the database connection serializable

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Needs work » Needs review

#5: openid-1946438-5.patch queued for re-testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Does that mean that there is no test for this page yet?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

I have the same error as well, but just to be sure, that's an issue unrelated with this issue.

The reason why this db connection is not an issue is, because the confirm form submit redirects the user, so the form state is never cached.
In general though, I think we should wait for the other issue, just to be 100% sure.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Status: Needs review » Postponed

correct status

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Status: Postponed » Needs review

#5: openid-1946438-5.patch queued for re-testing.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Status: Needs review » Reviewed & tested by the community

if bot disagrees it ll let us know

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 3a8e8c6 and pushed to 8.x. Thanks!

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