In principle yes this is how I see it being done

@brianteeman Could you please try it quickly to see what is missing or still need to be addressed?

From my quick test with both a manager and super administrator account this PR works correctly and is how it should have been done from the beginning without all the duplication of code in com_admin

@joomdonation Event if it’s draft status you should fix the PHPCS (code style) errors reported by drone, because only if these pass the system tests will be run, and those would show if there was a mistake.

The main change here is it allows user to edit his own account ...

@joomdonation Should the title of the PR be changed from "[4.0] Allow Managers to edit account via com_users" to "[4.0] Allow Managers to edit own account via com_users"?

@richard67 Yes. I changed it to a better title (describe exactly what PR does)

@joomdonation Your last commit changed the since tag at the wrong place. the old function should remain 1.6 and the new one should get deploy version, but you did it vice versa ;-)

Thanks @richard67. Hope it is all OK now. On top of my head, this PR is now ready for reviewing and testing. Hopefully we can close the release blocker issue #30217

@joomdonation

Hope it is all OK now.

Yes, code looks ok and clean to me.

Hopefully we can close the release blocker issue #30217

You mean this PR solves it? If you confirm, I will close it.

Added the release blocker label as inherited from the issue.

@richard67 Yes, I think we can close that issue. Even if this PR is not accepted, I think most users can agree that it is not release blocker anymore (the problem right now is just duplication code which is solved if this PR is accepted)

Even if this PR is not accepted, I think most users can agree that it is not release blocker anymore (the problem right now is just duplication code which is solved if this PR is accepted)

@joomdonation You mean I should remove the release blocker label from this PR?

@joomdonation You mean I should remove the release blocker label from this PR?

For now, we can leave it as how it is.

To me the thing I'd like to do before 4.0 ships is remove com_admin. Whilst we don't guarantee full b/c on extensions - I would rather just remove com_admin now before 4.0 ships and not have any issues.

Removing com_admin completely? It does a lot more than what this PR is correctly removing

Sorry to be clear the profile view that we added back in #29251 - I don't want to remove the rest of it

We can leave that removal for a separate PR - happy with that - this is fine as it is as it solves an "issue" in it's own right.

OK - we're all on the same page then

Will you be removing the now useless code in com_admin in this PR

Will you be removing the now useless code in com_admin in this PR

Let's get that in, removal can be done in another PR.

I just asked so I knew if to mark this as good test or not

I have tested this item ✅ successfully on 9b3a9b8

I tied to reproduce the error with current dev and it seems to be fixed magically. Can anyone confirm?

I tied to reproduce the error with current dev and it seems to be fixed magically. Can anyone confirm?

You mean you logged in using a manager account and can still edit the account? Maybe you still use the old link (which is linked to com_admin which should be removed if this PR is accepted)

I followed the instructions you gave and yes it is a link to com_admin. When I apply your patch then it still works but com_user is used.

Please change the testing instructions

Ah, Yes, sorry. The testing instructions need to be updated. I will do that.

I have tested this item ✅ successfully on 9b3a9b8

RTC

Thanks Tobias. Committed the CS changes.

Up to now it was code style only changed after the tests, but if the above suggestions (type safe comparison, cast to int) will committed, too, then it will need new human tests since these changes might change "behaviour" of the code.

Back to pending. Please test again. Thanks in advance.

I have tested this item ✅ successfully on 2e92be8

Previous test by @rdeutz is still valid because the commit after that was just a small code style change. I've restored the result in the issue tracker so it's correctly counted there and here.

I have tested this item ✅ successfully on 688f66a

RTC

I have tested this item ? unsuccessfully on 688f66a

Doesn't work with PDO.

Back to pending

setting back to pending

got this on pdo with 8.0.23

So the problem is that PDO return string data type for User ID but MySQLi return int data type . I was not sure about it and that was the reason I used weak comparison at the begininging

I made modification as suggested. I tested it myself with both PDO and MySQLi, it is working well now. Please test it again. Thanks all for reviewing and testing and sorry for taking so much time from you for this PR.

I have tested this item ✅ successfully on f47df46

@brianteeman Could you please re-test this PR?

I have tested this item ✅ successfully on f47df46

RTC

Thankyou so much guys. Much appreciated!