Skip to content

[4.0] Allow users edit account via com_users without core.manager permission#32771

Merged
wilsonge merged 18 commits intojoomla:4.0-devfrom
joomdonation:user_edit_own_account
Mar 26, 2021
Merged

[4.0] Allow users edit account via com_users without core.manager permission#32771
wilsonge merged 18 commits intojoomla:4.0-devfrom
joomdonation:user_edit_own_account

Conversation

@joomdonation
Copy link
Contributor

@joomdonation joomdonation commented Mar 21, 2021

Pull Request for Issue #30217.

Summary of Changes

This PR allows users to edit their account from administrator area of the site without requiring to have core.manager permission. If it is accepted, it will solve the release blocker issue #30217. Also, it allows us to removing the code which we have to add to com_admin to allow editing profile at the moment.

Testing instructions

  • Apply patch.
  • Create a user account, assign this account to Manager user group
  • Login to administrator account of the site using that manager account
  • Try to access to User Account section at the top right of administrator area, click on Edit Account and Accessibility Settings, make sure you can still access to the page and update the account data without any problem.

@brianteeman
Copy link
Contributor

In principle yes this is how I see it being done

@joomdonation
Copy link
Contributor Author

joomdonation commented Mar 21, 2021

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

@brianteeman
Copy link
Contributor

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

@richard67
Copy link
Member

@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.

@richard67
Copy link
Member

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"?

@joomdonation joomdonation changed the title [4.0] Allow Managers to edit account via com_users [4.0] Allow users edit account via com_users without core.manager permission Mar 21, 2021
@joomdonation
Copy link
Contributor Author

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

@richard67
Copy link
Member

@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 ;-)

@joomdonation joomdonation marked this pull request as ready for review March 21, 2021 14:07
@joomdonation
Copy link
Contributor Author

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

@richard67
Copy link
Member

@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.

@richard67
Copy link
Member

Added the release blocker label as inherited from the issue.

@joomdonation
Copy link
Contributor Author

@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)

@richard67
Copy link
Member

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?

@richard67
Copy link
Member

Back to pending


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

@alikon
Copy link
Contributor

alikon commented Mar 24, 2021

setting back to pending


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

@alikon
Copy link
Contributor

alikon commented Mar 24, 2021

got this on pdo with 8.0.23
Screenshot from 2021-03-24 19-01-11

@alikon alikon added Release Blocker RMDQ ReleaseManagerDecisionQueue labels Mar 24, 2021
@joomdonation
Copy link
Contributor Author

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.

@rdeutz
Copy link
Contributor

rdeutz commented Mar 25, 2021

I have tested this item ✅ successfully on f47df46


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

@rdeutz rdeutz removed the RMDQ ReleaseManagerDecisionQueue label Mar 25, 2021
@rdeutz rdeutz added this to the Joomla 4.0 milestone Mar 25, 2021
@joomdonation
Copy link
Contributor Author

@brianteeman Could you please re-test this PR?

@dgrammatiko
Copy link
Contributor

I have tested this item ✅ successfully on f47df46


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

@richard67 richard67 added RMDQ ReleaseManagerDecisionQueue and removed Release Blocker labels Mar 25, 2021
@joomla-cms-bot joomla-cms-bot removed this from the Joomla 4.0 milestone Mar 25, 2021
@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 25, 2021
@richard67 richard67 added Release Blocker and removed RMDQ ReleaseManagerDecisionQueue labels Mar 25, 2021
@richard67 richard67 added this to the Joomla 4.0 milestone Mar 25, 2021
@wilsonge
Copy link
Contributor

Thankyou so much guys. Much appreciated!

@wilsonge wilsonge merged commit 52ae0ee into joomla:4.0-dev Mar 26, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 26, 2021
@joomdonation joomdonation deleted the user_edit_own_account branch March 26, 2021 00:37
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.

Comments