Problem/Motivation

drupal-check results on commit hash:
source : [git] https://git.drupal.org/project/r4032login a10444360f0d738e7517616fa30d60b5b1da0871
source : http://cgit.drupalcode.org/r4032login


 ------ --------------------------------------------------- 
  Line   src/EventSubscriber/R4032LoginSubscriber.php       
 ------ --------------------------------------------------- 
  169    Call to deprecated function drupal_set_message().  
 ------ --------------------------------------------------- 

 ------ ---------------------------------------------- 
  Line   tests/src/Unit/R4032LoginSubscriberTest.php   
 ------ ---------------------------------------------- 
  83     Call to deprecated method getMock() of class  
         Drupal\Tests\UnitTestCase.                    
  95     Call to deprecated method getMock() of class  
         Drupal\Tests\UnitTestCase.                    
  96     Call to deprecated method getMock() of class  
         Drupal\Tests\UnitTestCase.                    
  97     Call to deprecated method getMock() of class  
         Drupal\Tests\UnitTestCase.                    
  98     Call to deprecated method getMock() of class  
         Drupal\Tests\UnitTestCase.                    
  100    Call to deprecated method getMock() of class  
         Drupal\Tests\UnitTestCase.                    
  102    Call to deprecated method getMock() of class  
         Drupal\Tests\UnitTestCase.                    
  105    Call to deprecated method getMock() of class  
         Drupal\Tests\UnitTestCase.                    
  253    Call to deprecated method getMock() of class  
         Drupal\Tests\UnitTestCase.                    
 ------ ---------------------------------------------- 

 [ERROR] Found 10 errors                                                    

 

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#35 3042799-35.patch15.6 KBNixou
#33 3042799-33.patch8.87 KBNixou
#23 3042799-22.patch9.27 KBedysmp
#22 3042799-22.patch9.83 KBedysmp
#22 interdiff-18-22.txt408 bytesedysmp
#18 3042799-r4032login-fix-deprecation-interdiff-17-18.txt785 byteskatzilla
#18 3042799-r4032login-fix-deprecation-18.patch8.87 KBkatzilla
#17 3042799-r4032login-fix-deprecation-interdiff-14-17.txt1.03 KBkatzilla
#17 3042799-r4032login-fix-deprecation-17.patch8.87 KBkatzilla
#14 3042799-r4032login-fix-deprecation-interdiff-12-14.txt6.48 KBpminf
#14 3042799-r4032login-fix-deprecation-14.patch8.86 KBpminf
#12 3042799-r4032login-fix-deprecation-12.patch4.02 KBLilit_Ghazaryan
#11 3042799-r4032login-fix-deprecation-11.patch3.59 KBLilit_Ghazaryan
#10 3042799-r4032login-fix-deprecation-10.patch0 bytesLilit_Ghazaryan
#8 interdiff.txt6.51 KBjkswoods
#8 3042799-08-drupal-9-deprecations.patch8.81 KBjkswoods
#4 r4032login-fix-deprecation-3042799-4-d8.patch4.24 KBgun2dru
#2 drupal_9_deprecated_code-3042799-2.patch6.44 KBSergiu Stici
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mcdwayne created an issue. See original summary.

Sergiu Stici’s picture

Status: Active » Needs review
FileSize
6.44 KB

Here is the patch, please review.

Status: Needs review » Needs work

The last submitted patch, 2: drupal_9_deprecated_code-3042799-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

gun2dru’s picture

gun2dru’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 4: r4032login-fix-deprecation-3042799-4-d8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

anavarre’s picture

+++ b/src/EventSubscriber/R4032LoginSubscriber.php
@@ -166,7 +166,7 @@ class R4032LoginSubscriber extends HttpExceptionSubscriberBase {
+          \Drupal::messenger()->addMessage(Xss::filterAdmin($message), $messageType);

This should use dependency injection instead.

Also, there are still failures on the testbot:

1) Drupal\Tests\r4032login\Functional\RedirectTest::testSkipRedirect with data set #0 ('/admin/config/development', array(), 403, '/admin/config/development')
Behat\Mink\Exception\ExpectationException: Current response status code is 200, but 403 expected.

/var/www/html/vendor/behat/mink/src/WebAssert.php:770
/var/www/html/vendor/behat/mink/src/WebAssert.php:130
/var/www/html/modules/contrib/r4032login/tests/src/Functional/RedirectTest.php:46

2) Drupal\Tests\r4032login\Functional\RedirectTest::testSkipRedirect with data set #2 ('/admin/modules', array(array('bar')), 403, '/admin/modules?foo=bar')
Behat\Mink\Exception\ExpectationException: Current response status code is 200, but 403 expected.

/var/www/html/vendor/behat/mink/src/WebAssert.php:770
/var/www/html/vendor/behat/mink/src/WebAssert.php:130
/var/www/html/modules/contrib/r4032login/tests/src/Functional/RedirectTest.php:46
jkswoods’s picture

Updated patch to use dependency injection. Tests are passing locally, so we'll see what testbot says...

jkswoods’s picture

Status: Needs work » Needs review
Lilit_Ghazaryan’s picture

Lilit_Ghazaryan’s picture

Lilit_Ghazaryan’s picture

pminf’s picture

Status: Needs review » Needs work

#7 wasn't addressed, working on this now.

pminf’s picture

Oh, patch from #10 (#11 and #12) did not contain changes from #8, which already addressed #7 with dependency injection. I have combined patch #10 and #12 and finally removed the obsolete function "drupal_set_message()" from the unit test.

pminf’s picture

Gogowitsch’s picture

@pminf: there are 2 things to improve from my perspective:

  1. Remove the space after the arrow:

    $kernel = $this-> createMock('Symfony\Component\HttpKernel\HttpKernelInterface');
  2. @param MessengerInterface $messenger;: the trailing ; needs to be removed
katzilla’s picture

Changed last recommendations from @gogowisch and applied a new patch. Tested and reviewed - works fine, all errors from drupal-check are gone after applying.

katzilla’s picture

pminf’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! #17 and #18 addressed #16. Everything should be fine now.

DamienMcKenna’s picture

Title: Drupal 9 Deprecated Code Report » Drupal 9 Deprecated Code Report for R4032Login module
jkswoods’s picture

edysmp’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
408 bytes
9.83 KB

Get info file compatilble with Drupal 9

See: https://www.drupal.org/node/3070687

smrutha’s picture

Status: Needs review » Reviewed & tested by the community

Patch #23 works for me.

drupal-check -d web/modules/contrib/r4032login
15/15 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

[OK] No errors

fabianderijk’s picture

Works like a charm!

dbielke1986’s picture

Works for me, too.
I would welcome an official release.

:-)

Martijn de Wit’s picture

A new release for D9 would be great indeed :)

grasmash’s picture

Or even just a commit to a dev branch! :)

grasmash’s picture

For those who need to use this module with Drupal 9 now, feel free to use my fork (which has the patch committed):

{
    "repositories": {
        "r4032login": {
            "type": "vcs",
            "url": "git@github.com:grasmash/drupal-r4032login.git"
        }
    },
    "require": {
        "drupal/r4032login": "8.x-1.x-dev"
    }
}
Grimreaper’s picture

Hello,

Thanks everyone for working on this.

From what I see, only maintainers action is needed now. Or am I missing something?

Is a new maintainer needed for the module? Because none has replied to this issue or #3148547: Automated Drupal 9 compatibility fixes.

Nixou’s picture

Thanks you everyone.

I take the point this morning :

  • New branch 2.x for the new version compatible with drupal 8 and drupal 9
  • Check if test pass on drupal 8 and drupal 9 with this patch
  • If all tests are ok, I'll create a new release 2.0.0
Nixou’s picture

Version: 8.x-1.x-dev » 2.x-dev
Nixou’s picture

I committed directly to the 2.x branch the .info modification so I hope I can run test without composer error then.

I attach the reroll patch without the .info modification part.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 33: 3042799-33.patch, failed testing. View results

Nixou’s picture

Nixou’s picture

Status: Needs work » Needs review

  • Nixou committed d89bcb4 on 2.x
    Issue #3042799 by Lilit_Ghazaryan, katzilla, edysmp, Nixou, pminf,...
Nixou’s picture

Status: Needs review » Fixed
Grimreaper’s picture

Thanks!

martinhansen’s picture

Excellent!

Status: Fixed » Closed (fixed)

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