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

Improve warning message if article author no longer exists #11786

Closed
C-Lodder opened this issue Aug 25, 2016 · 4 comments
Closed

Improve warning message if article author no longer exists #11786

C-Lodder opened this issue Aug 25, 2016 · 4 comments

Comments

@C-Lodder
Copy link
Member

C-Lodder commented Aug 25, 2016

Steps to reproduce the issue

  • Create a new user
  • Create an article and set the Author as the newly created user
  • Delete the user
  • Open the Article in the Article Manager

Expected result

I would suggest displaying a more meaningful message, such as:

The author of this article no longer exist.

Or something along those lines. @brianteeman perhaps you could come up with a better warning message

Actual result

JUser: :_load: Unable to load user with ID: 371

@C-Lodder C-Lodder changed the title Improve warning if article author (user) no longer exists Improve warning message if article author no longer exists Aug 25, 2016
@brianteeman
Copy link
Contributor

brianteeman commented Aug 25, 2016 via email

@brianteeman brianteeman changed the title Improve warning message if article author no longer exists Improve warning message if article author no longer exists Aug 25, 2016
@C-Lodder
Copy link
Member Author

Not really sure how to approach this. The message is coming from here:

https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/user/user.php#L869

Only thing I can think of is using JInput and a conditional statement:

$input = JFactory::getApplication()->input;

if ($input->get('layout') == 'edit')
{
    JLog::add(JText::_('The author of this article is no longer a user on this site'), JLog::WARNING, 'jerror');
}
else
{
    JLog::add(JText::sprintf('JLIB_USER_ERROR_UNABLE_TO_LOAD_USER', $id), JLog::WARNING, 'jerror');
}

which I'm not sure is the best method.

@mbabker - any thoughts on how to approach this?

@mbabker
Copy link
Contributor

mbabker commented Aug 25, 2016

I would not be putting component specific stuff in JUser for starters. This is why arbitrarily logging things with the "jerror" category isn't a good mechanism of error handling. It'd be better if it threw an Exception but that apparently has B/C implications. So the only way around that would be to stop logging to the "jerror" category and force downstream uses of JUser::load() to check for boolean false conditions and implement their own error handling mechanisms at a lower level (to include rendering a "friendly" message).

As vague as it is, the error message generated at the spot it is now is correct and even if it were throwing an Exception I wouldn't change it. Annoying because it's vague and generally not friendly, but this is a case of an error being rendered where it shouldn't be and it does actually state what the error condition is (can't load a user with the given ID) but it's not really pertinent to every site visitor.

@C-Lodder
Copy link
Member Author

Ok thanks @mbabker . Closing

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

No branches or pull requests

3 participants