Skip to content

Set an empty string value to the legacy otpKey and otep columns if empty#38549

Merged
zero-24 merged 2 commits intojoomla:4.2-devfrom
nikosdion:fix/38543-otpkey-otep-cli
Dec 23, 2022
Merged

Set an empty string value to the legacy otpKey and otep columns if empty#38549
zero-24 merged 2 commits intojoomla:4.2-devfrom
nikosdion:fix/38543-otpkey-otep-cli

Conversation

@nikosdion
Copy link
Copy Markdown
Contributor

Pull Request for Issue #38543 .

Summary of Changes

Modified the User table to add empty string values to the otpKey and otep columns if they are empty.

NORMALLY THIS SHOULD NOT BE NECESSARY. The table definition gives a default value of an empty string to both columns and the reported issue cannot be reproduced.

However, if your site came from a series of upgrades from older versions and/or transferred between servers by hand it might end up having these columns as NOT NULL but without a default value.

Testing Instructions

From the CLI folder run

php joomla.php user:add --username=foobar --name="Foo Bar" --email=foobar@example.com --password="P@s5w0rD" --usergroup=Registered

Actual result BEFORE applying this Pull Request

It works on all sites except the issue reporter's site.

Expected result AFTER applying this Pull Request

It works on all sites.

Documentation Changes Required

None.

@alikon
Copy link
Copy Markdown
Contributor

alikon commented Aug 24, 2022

I have tested this item ✅ successfully on 12f665c

code review


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38549.

@nikosdion
Copy link
Copy Markdown
Contributor Author

OK, I guess people don't care if a site is kinda broken from successive updates. Can't really blame them, this only ever happened to one site I am aware of. So, I am closing this PR.

@Webdongle
Copy link
Copy Markdown
Contributor

Webdongle commented Dec 22, 2022

https://forum.joomla.org/viewtopic.php?f=808&t=998623 appears to be the same problem


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38549.

@nikosdion
Copy link
Copy Markdown
Contributor Author

Reopening as this is a recurring issue, per #38549 (comment)

Kind reminder @fancyFranci @roland-d that this needs to be fixed in 4.2, ideally four months ago.

BTW, this fixes a simple mistake I made in an earlier PR that none of you caught (because y'all were too busy bikeshedding instead of actually testing during the entire month you were bombarding me with hundreds of mostly nonsense comments). Also note that I came here with a fix for it as soon as 4.2.1 was out which is when people actually started upgrading to 4.2. I don't understand why this trivial fix of an issue with a real world impact is still languishing in the PR purgatory.

@alikon
Copy link
Copy Markdown
Contributor

alikon commented Dec 23, 2022

I have tested this item ✅ successfully on 177be86


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38549.

@zero-24 zero-24 added this to the Joomla! 4.2.7 milestone Dec 23, 2022
@zero-24 zero-24 merged commit e9b9263 into joomla:4.2-dev Dec 23, 2022
@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Dec 23, 2022

Merging thanks @nikosdion

@nikosdion
Copy link
Copy Markdown
Contributor Author

Thank you! This will help folks work with their legacy template overrides without undue frustration 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants