Testing Instructions

Code review - verify null-safe operators (?-> and ??) are used for user property access.

@Shauryan0207 I'd prefer to have instructions for a real test, not just code review.

Providing reproducible test instructions would also show that you have tested the changes from your PR yourself before submitting it, which is a mandatory requirement for contributing here.

Could you provide instructions? Thanks in advance.

@Shauryan0207 - I am trying to test your PR and I read the Testing Instructions of "PHP warnings when ActionLog runs without authenticated user" but I must admit not understanding what you meant as I can't think of anything that triggers an ActionLog without a user being authenticated (but that could just be ME not thinking through all of the cases)...

I could imagine maybe this happening if a Session timeout happened during an action, but that's not so easy to trigger.

Looking forward to specific use case that I can test for you.

Hey @richard67
Create a test file test_actionlog_null.php

<?php
// Simulate scenario where getIdentity() returns null (guest user or logout scenario)
$user = null;

echo "TEST 1: Before fix (shows PHP warning)\n";
echo "Attempting to access \$user->id when \$user is null:\n";
try {
    $id = $user->id; // Old code behavior
    echo "User ID: " . $id . "\n";
} catch (\Error $e) {
    echo "❌ PHP Warning: " . $e->getMessage() . "\n\n";
}

echo "TEST 2: After fix (no warning)\n";
echo "Using null-safe operator:\n";
$id = $user?->id; // Fixed code behavior
$username = $user?->username ?? '';
echo "User ID: " . ($id ?? 'null') . "\n";
echo "Username: " . ($username ?: 'empty') . "\n";
echo "✅ No warnings - fix works correctly\n";

php test_actionlog_null.php

Expected output:
TEST 1 shows a PHP error when accessing properties on null
TEST 2 shows no warnings and handles null safely

Hey @exlemor ,Thanks for reviewing this while it’s rare, this can happen during things like command-line scripts, scheduled tasks, system maintenance, or guest API requests, where actions are logged even though no user is logged in.

I have tested this item ✅ successfully on 998963d

Review

@Shauryan0207 Could you adjust the testing instructions in the description (initial post) of this PR to the latest code changes? Thanks in advance.

@Shauryan0207 Could you adjust the testing instructions in the description (initial post) of this PR to the latest code changes? Thanks in advance.

@richard67 Done ! Thanks

@Shauryan0207 I'm probably doing something incorrectly but I'm not matching the BEFORE condition of a PHP error.

I created the php file: test_actionlog_null.php in the root of the sub-folder where Joomla 5.4 is installed, /_j540/
I connected to an SSH terminal and ran from the /_j540/ root, php test_actionlog_null.php and received this result:

TEST 1: Before fix (shows PHP warning)
Attempting to access $user->id when $user is null:
User ID: 
TEST 2: After fix (no warning)
Using null-safe operator:
User ID: null
Username: empty
✅ No warnings - fix works correctly

What did I screw up? ;(

I looked at this PR and I am not happy with it. The quoted test would just show the desired behaviour. It does not test Joomla. Various CLI commands such as php cli/joomla.php site:up don't trigger the plugin. It seems to me it may be better to see the PHP warning so that the site owner can see that something unusual is going on. Can you provide any sort of screenshot showing the warning?

I have tested this item ✅ successfully on 998963d

Tested with JBT on current 5.4-dev

• Enabled Xdebug and in file administrator/components/com_actionlogs/src/Plugin/ActionLogPlugin.php
  • Set break point in line 70 and logged in from site
  • After line 71 $user = $app->getIdentity(); set $user = null
  • After line 73 foreach ($messages as $index => $message) { set $message = array(1)
  • Seen PHP warning on frontend:

    Warning: Attempt to read property "id" on null in /var/www/html/administrator/components/com_actionlogs/src/Plugin/ActionLogPlugin.php on line 75
    
    Warning: Attempt to read property "username" on null in /var/www/html/administrator/components/com_actionlogs/src/Plugin/ActionLogPlugin.php on line 79
    

• Applied PR with Patch Tester
  • with brakpoint and overwriting variables:
    • No warning anymore on frontend
    • Actions log entry is User {username} logged in to {app}
  • Without breakpoint login/logout actions are logged

I looked at this PR and I am not happy with it.

@ceford The code review shows that the object $user is no longer used if it does not exist. As a PHP novice, this seems safe to me.

RTC

@muhme Haven't looked at the details, but some questions raise:

• Do we really need to want to log the action without knowing the user who is performing the action? Is that kind of log useful ?
• And with the change, when the user is not available, I guess the data such as userid is not available, so it won't be replaced?

@muhme Haven't looked at the details, but some questions raise:

Do we really need to want to log the action without knowing the user who is performing the action? Is that kind of log useful ?

And with the change, when the user is not available, I guess the data such as userid is not available, so it won't 

@joomdonation These are valid questions, but I cannot answer them. Perhaps you or someone else can answer them? thx

The fix is valid despite the questions.

About the questions.

The CLI app does not have an User.
Extension may trigger some Content import from CLI or anything else that works with content.
We can skip logs without User of course, but I would keep it because it still helpful to track changes in the system.

Thank you @Fedik for stepping in and answering the questions.

Thanks @Shauryan0207 for this PR, and thanks @Fedik and @muhme for testing, and again @Fedik for advice and support.