terminal42 / contao-mailusername

MIT License
7 stars 5 forks source link

2.0.2 breaks compatibility with core ModuleRegistration #27

Closed philvdb closed 2 years ago

philvdb commented 2 years ago

Hey @aschempp ,

I am not sure why you have added this check: https://github.com/terminal42/contao-mailusername/commit/426aa78d23f36cf1d57c49a4f1c5cabb18171669#diff-b953e0792101741546ab26af5d86a713ba7d0d49b2a384517378ac73d4e9e103R60

It is breaking compatibility with the core registration module for me. See here: https://github.com/contao/contao/blob/87b3a7cf676461608c08146678c046bd5debf4d8/core-bundle/contao/modules/ModuleRegistration.php#L253

The save callback will pass null as second argument which invokes your line ($dc being null). $strValue is then set to null as well and later returned. ModuleRegistration will use this new value of null to create the user and throw a database exception, as the email field cannot be null.

philvdb commented 2 years ago

Ah, I see it was in response to #24 . Well I'm afraid the fix has unintended consequences :)

ameotoko commented 2 years ago

I can confirm the issue. I'm afraid 2.0.2 breaks the front end registration completely now.

Uncaught PHP Exception Doctrine\DBAL\Exception\NotNullConstraintViolationException: 
   "An exception occurred while executing a query: SQLSTATE[23000]: Integrity constraint violation:
   1048 Column 'email' cannot be null"
philvdb commented 2 years ago

An interim fix until @aschempp gets around to it, in case a user doesn't know: you can edit your main composer.json and add the following line to your "conflict" section:

"terminal42/contao-mailusername": "2.0.2"

So in total it might look similar to this:

    "conflict": {
        "contao/core": "*",
        "contao/manager-plugin": "<2.0 || >=3.0",
        "terminal42/contao-mailusername": "2.0.2"
    },

This way, whenever @aschempp tags a new version, you will be able to upgrade to it.

ameotoko commented 2 years ago

Actually, front end registration does not work for me in the entire v2 branch.

This should be fixed by #26.

aschempp commented 2 years ago

Fixed in 57fd76ca27d80f41651bdca3edd65df032ab9095