Problem/Motivation

Error
TypeError: Drupal\tfa\Form\TfaDisableForm::deleteUserData(): Argument #2 ($key) must be of type string, null given, called in /var/www/html/web/modules/custom/tfa/src/Form/TfaDisableForm.php

Steps to reproduce

As admin navigate to user/USER_ID_HERE/security/tfa/disable for a user with TFA enabled, attempt to disable TFA and receive an error.

This is suspected to be related to some of our recent typehint changes.

Proposed resolution

Remaining tasks

User interface changes

None

API changes

None expected

Data model changes

None

Issue fork tfa-3393468

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

cmlara created an issue. See original summary.

sarwan_verma’s picture

sarwan_verma commented
Assigned: Unassigned » sarwan_verma

sarwan_verma opened merge request !41

sarwan_verma’s picture

sarwan_verma commented
Status: Active » Needs review

Hi @cmlara,

I have fixed the issue in MR, please review and verify.

cmlara’s picture

cmlara
he/him
commented
Status: Needs review » Needs work

Added comments in the MR, setting back to NW for their resolution.

sarwan_verma’s picture

sarwan_verma commented
Assigned: sarwan_verma » Unassigned
sarwan_verma’s picture

sarwan_verma commented

Hi @cmlara,

Please share the reproduce steps, i'm not able to clone this module on my local.

cmlara’s picture

cmlara
he/him
commented

I'm wondering if I'm not understanding your response, are you attempting to resolve issues without having the module installed?

If you mean reproducing the MR comments, part of reproducing is from Human Review, and the other is from PHPStan which can be tested with phpstan analyze ./ from the modules directory (this does require downloading the module into a development lab), we also include the results in our GitLab CI tests.

If you mean reproducing the bug, the (high level) steps are included in the Issue Summary, reproducing does require having the module installed to test.

bhanu951’s picture

bhanu951 commented

Got hit by this issue, when trying to fix #3395254: Wrong domain name for Google Authenticator on TOTP and HOTP setup page on dev branch.

Can confirm, patch resolves the issue.

But seems PHPDoc Block type hint is not updated to match the variable type ?

bhanu951’s picture

bhanu951 commented

Seems @cmlara already commented on the issue, I updated the PHPDoc block.

bhanu951’s picture

bhanu951 commented
Status: Needs work » Needs review

Addressed review comments, please re review.

cmlara’s picture

cmlara
he/him
commented
Status: Needs review » Fixed

LGTM, merged to dev. Thank you!

Status: Fixed » Closed (fixed)

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