Joomla! Issue Tracker - CMS

Download
Launch
Feeling Lucky

[#39435] - [4][com_actionlog] - log event reset password

? ? Pending

User tests: Successful: Unsuccessful:

avatar alikon
alikon
16 Dec 2022

Pull Request for Issue #39062.

Summary of Changes

generate proper event

Testing Instructions

  • Set up sending mail from the site,
  • Create a test user with a valid email address,
  • Use the item "Forgot your passwor" in the authorization form in the frontend for a test user,
  • Enter your login / confirmation code or follow the link in the letter received by mail,
  • Change the user's password to a new one.

Actual result BEFORE applying this Pull Request

see #39435

Expected result AFTER applying this Pull Request

the events are correctly logged in Action Logged

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

avatar joomla-cms-bot joomla-cms-bot - change - 16 Dec 2022
Category ⇒ Front End Plugins
avatar alikon alikon - open - 16 Dec 2022
avatar alikon alikon - change - 16 Dec 2022
Status New ⇒ Pending
avatar N6REJ
N6REJ - comment - 16 Dec 2022

please fill out PR info properly.

avatar joomla-cms-bot joomla-cms-bot - change - 16 Dec 2022
Category Front End Plugins ⇒ Front End com_users Plugins
93e3038 16 Dec 2022 avatar alikon event
avatar alikon alikon - change - 16 Dec 2022
Labels Added: ?
avatar alikon alikon - change - 16 Dec 2022
The description was changed
avatar alikon alikon - edited - 16 Dec 2022
avatar alikon alikon - change - 16 Dec 2022
The description was changed
avatar alikon alikon - edited - 16 Dec 2022
avatar alikon alikon - change - 16 Dec 2022
Title
…
Update Joomla.php
[4][com_actionlog] - log event reset password
avatar alikon alikon - edited - 16 Dec 2022
fa80696 16 Dec 2022 avatar alikon cs
avatar sandewt
sandewt - comment - 16 Dec 2022

Pull Request for Issue #39435 .
see #39435

Should be #39062

avatar SharkyKZ
SharkyKZ - comment - 17 Dec 2022

Surely, adding new events is not necessary to fix the reported issue?

avatar alikon alikon - change - 17 Dec 2022
The description was changed
avatar alikon alikon - edited - 17 Dec 2022
avatar Kostelano Kostelano - test_item - 17 Dec 2022 - Tested successfully
avatar Kostelano
Kostelano - comment - 17 Dec 2022

I have tested this item ✅ successfully on fa80696

Thanks, now it works as it should.

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

avatar viocassel viocassel - test_item - 17 Dec 2022 - Tested successfully
avatar viocassel
viocassel - comment - 17 Dec 2022

I have tested this item ✅ successfully on fa80696

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

avatar richard67
richard67 - comment - 17 Dec 2022

Hmm there is a comment with a valid concern: #39435 (comment) . We should not add new events when it‘s not needed.

avatar alikon
alikon - comment - 19 Dec 2022

What's the scope of the Events then ?

avatar alikon alikon - change - 19 Dec 2022
Labels Added: ?
avatar richard67
richard67 - comment - 19 Dec 2022

What's the scope of the Events then ?

@alikon Have you checked if there is an existing event which can be used? If you say yes and there was none, I am ok with it.

avatar alikon
alikon - comment - 19 Dec 2022

in the previous version 3 it was used onUserAfterSave but 1st is not logical 2nd no separation of concerns, and 3rd not readable code

avatar rdeutz
rdeutz - comment - 19 Dec 2022

I think it is fine to create new events for thinks like this, but it should be consisted overt the whole process

avatar Kostelano Kostelano - test_item - 29 Dec 2022 - Tested successfully
avatar Kostelano
Kostelano - comment - 29 Dec 2022

I have tested this item ✅ successfully on ba1d350

I re-tested after the changes - everything works. Thank you.
I hope this doesn't get stuck and gets merged.

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

avatar viocassel viocassel - test_item - 29 Dec 2022 - Tested successfully
avatar viocassel
viocassel - comment - 29 Dec 2022

I have tested this item ✅ successfully on ba1d350

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

avatar richard67
richard67 - comment - 29 Dec 2022

I think it is fine to create new events for thinks like this, but it should be consisted overt the whole process

@alikon If I understand @rdeutz 's comment right, he means that if we add events onUserAfterResetRequest and onUserAfterResetComplete, there should also be events onUserBeforeResetRequest and onUserBeforeResetComplete added, even if currently not used. @rdeutz Is my understanding right, and should this be done with this PR here?

avatar rdeutz
rdeutz - comment - 23 Feb 2023

@richard67 yes and yes.

avatar richard67
richard67 - comment - 23 Feb 2023

@alikon Could you check the previous two comments and if possible implement the requested changes? It would be good to bring this useful PR forward. Let us know if you need some help or other that or further advice. Thanks in advance.

avatar alikon
alikon - comment - 25 Feb 2023

added the Before events

avatar Kostelano
Kostelano - comment - 25 Feb 2023

I tested again. At the moment, the PR is broken, something needs to be fixed (problem after the last changes). A request to remind the login comes to the mail, a request to change the password does not. I tried several times.

avatar Kostelano Kostelano - test_item - 25 Feb 2023 - Tested successfully
avatar Kostelano
Kostelano - comment - 25 Feb 2023

I have tested this item ✅ successfully on afc06a4

Sorry, it works. Sending a successful test.

Why did it happen that at first it did not work: I created a user and, in addition to the "Registered" group, added him to the Superusers group. The system does NOT send a password change request for a group of Superusers (which is strange because I have seen error lines for superusers who want to change the password). In this case, the system proceeds to the next stage - entering a login / confirmation code, but nothing comes to the mail. Well, that's a bug for a separate fix.

As soon as I removed the user from the superuser group, everything began to work. Everything is displayed correctly in the action log.

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

avatar viocassel viocassel - test_item - 2 Mar 2023 - Tested successfully
avatar viocassel
viocassel - comment - 2 Mar 2023

I have tested this item ✅ successfully on afc06a4

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

avatar alikon alikon - test_item - 2 Mar 2023 - Tested successfully
avatar alikon alikon - alter_testresult - 2 Mar 2023 - alikon: Not tested
avatar alikon alikon - change - 2 Mar 2023
Status Pending ⇒ Ready to Commit
avatar alikon
alikon - comment - 2 Mar 2023

RTC

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

avatar roland-d roland-d - change - 6 Mar 2023
Labels Added: ?
Removed: ?
avatar sandewt
sandewt - comment - 7 Mar 2023

Why don't you use [ ] (brackets) instead of array() ?
See also line 1179.

@alikon Did I miss something, because as far as I know brackets are preferred in J4? You probably have a reason for it.

avatar roland-d roland-d - change - 7 Mar 2023
Status Ready to Commit ⇒ Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 ⇒ 2023-03-07 10:46:51
Closed_By ⇒ roland-d
avatar roland-d roland-d - close - 7 Mar 2023
avatar roland-d roland-d - merge - 7 Mar 2023
avatar roland-d
roland-d - comment - 7 Mar 2023

Thank you

avatar alikon
alikon - comment - 7 Mar 2023

@sandewt i'll change it to [] on a next pr

Add a Comment

Login with GitHub to post a comment