Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for error handling saveUser function #6275

Merged
merged 1 commit into from Apr 2, 2019

Conversation

@Ruud68
Copy link
Contributor

@Ruud68 Ruud68 commented Apr 2, 2019

Pull Request for Issue # new

Summary of Changes

the current implementation of saving the (joomla) user is not handling any errors correct.
It tries a try / catch to catch all errors generated by the User object bind and save or by user plugins (onUserbeforeSave) but the Joomla User object catches any errors itself and sets these in the User object. It then returns with a false. Therefore the catch will never be executed.

this PR corrects the error handling by testing for a false (both bind and save) and if one of these returned false, it enqueues the error message that is set in the Joomla User object.

When the user saving now fails due to Joomla or a plugin, the correct error message is displayed instead of the profile saved message.

Testing Instructions

In a (existing) system plugin, create the folowing function:

	public function onUserBeforeSave($user, $isNew, $data)
	{
		throw new InvalidArgumentException('Oepsie...');
		return false;
	}

Now when saving your user profile in the front-end you should see the correct error 'Oepsie...' preventing the saving of the profile.

@810 810 added this to the 5.1.11 milestone Apr 2, 2019
@810 810 merged commit adaac3a into Kunena:K5.1 Apr 2, 2019
1 check failed
@Ruud68
Copy link
Contributor Author

@Ruud68 Ruud68 commented Apr 2, 2019

the merge was faster then it took me to fix LOL.
Thanks!

@810
Copy link
Member

@810 810 commented Apr 2, 2019

How better the instructions and the issue are explained how faster i can check/review it. 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants