Skip to content

[com_actionlogs] - fix non object notice #22526

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.

Sign up for GitHub

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

Merged
wilsonge merged 1 commit into from
Oct 8, 2018
Merged

[com_actionlogs] - fix non object notice #22526

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion administrator/components/com_actionlogs/libraries/actionlogplugin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ abstract class ActionLogPlugin extends JPlugin
*/
protected function addLog($messages, $messageLanguageKey, $context, $userId = null)
{
$user = $this->app->getIdentity();
$user = JFactory::getUser();
Copy link
Member

@laoneo laoneo Oct 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering if we have a bug here? Why is getIdentity null?

Copy link
Contributor

@mbabker mbabker Oct 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For 3.x? Because loadIdentity is not consistently called.

Copy link
Member

@laoneo laoneo Oct 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this needs to be fixed then. As far as I know in 4 it works reliably.

Copy link
Contributor

@wilsonge wilsonge Oct 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a specific fix for that in 4.x we always call it early in the stack to ensure it's loaded (#16398). It's not something I'd feel comfortable porting back into 3.x because it might well affect anything that extends CMSApplication

Copy link
Member

@laoneo laoneo Oct 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it got fixed in #19766. It would make sense to port it back to 3 or not?

Copy link
Contributor

@wilsonge wilsonge Oct 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well then is #16398 actually doing anything. Because that should have been enough on it's own

If we do backport it would have to be 3.10 and would need some heavy testing because it might well have affects on things that extends CMSApplication (i know when michael made it a final class some people complained and it ended up being reverted so there are definitely people doing that)

Copy link
Contributor

@mbabker mbabker Oct 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is more involved than those two patches. You're going to have to dig back even deeper to when the session API was refactored to use the Framework.


foreach ($messages as $index => $message)
{
Expand Down