#1971384: [META] Convert page callbacks to controllers covers all controller-based forms, but there are plenty of others that need converting. Here's one.

CommentFileSizeAuthor
#9 interdiff.txt17.21 KBsun
#7 authorize-2112557-7.patch27.98 KBtim.plunkett
#6 interdiff.txt1.27 KBtim.plunkett
#6 authorize-2112557-6.patch28.13 KBtim.plunkett
#5 authorize-2112557-5.patch28.12 KBtim.plunkett
#2 interdiff.txt5.99 KBtim.plunkett
#2 authorize-2112557-2.patch28.1 KBtim.plunkett
#1 authorize-2112557-1.patch28.23 KBtim.plunkett

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Needs review
StatusFileSize
new authorize-2112557-1.patch28.23 KB
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue summary: View changes
StatusFileSize
new authorize-2112557-2.patch28.1 KB
new interdiff.txt5.99 KB

Rerolled.

Status: Needs review » Needs work

The last submitted patch, 2: authorize-2112557-2.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Needs review

2: authorize-2112557-2.patch queued for re-testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
StatusFileSize
new authorize-2112557-5.patch28.12 KB

Rerolled. Will get better with #2145007: Convert form_set_error() in FormBase classes to use FormErrorInterface

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
StatusFileSize
new authorize-2112557-6.patch28.13 KB
new interdiff.txt1.27 KB

Rerolled

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
StatusFileSize
new authorize-2112557-7.patch27.98 KB

Rerolled

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

7: authorize-2112557-7.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new interdiff.txt17.21 KB

Attaching an interdiff generated via

git diff -b 8.x:core/includes/authorize.inc core/lib/Drupal/Core/FileTransfer/Form/FileTransferAuthorizeForm.php
sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me :-)

alexpott’s picture

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

Committed 4a6f248 and pushed to 8.x. Thanks!

+++ b/core/lib/Drupal/Core/FileTransfer/Form/FileTransferAuthorizeForm.php
@@ -0,0 +1,323 @@
+        $filetransfer->connect();

This can never work! the connect method is an abstract protected method! Looking at the implementations maybe they are somehow making it public... who knows? http://3v4l.org/08n7b interesting :) Opening a new issue to fix this.

  • Commit 4a6f248 on 8.x by alexpott: 
    Issue #2112557 by tim.plunkett: Convert authorize_filetransfer_form to...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Created #2242405: FileTransfer::connect is protected yet the implementations are public and this is relied upon. to address the comment in #11

Status: Fixed » Closed (fixed)

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