We get a fatal error after running the UserIntegrity sensor when there is no role assigned to users.
Fatal error: Recoverable fatal error: Argument 1 passed to Drupal\monitoring\Plugin\monitoring\SensorPlugin\UserIntegritySensorPlugin::getUserChanges() must be of the type array, null given
This is something that is very strange to happen but could be accidental. So we should avoid this.
Steps to reproduce:
- Run the sensor.
- Remove the administrator rol to admin.
- Run the sensor again.
This is happening because $current_users = $this->processUsers($this->loadCurrentUsers($role_ids)); is empty and then we trying to assert some values so we should verify if the variable is empty or not.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | 2581775-15.patch | 4.9 KB | giancarlosotelo |
| #13 | interdiff-2-13.txt | 2.42 KB | giancarlosotelo |
| #13 | 2581775-13.patch | 3.83 KB | giancarlosotelo |
| #13 | Screenshot from 2015-10-20 11:41:04.png | 49.88 KB | giancarlosotelo |
| #2 | 2581775-2.patch | 2.52 KB | giancarlosotelo |
Comments
Comment #2
giancarlosotelo commentedI am adding a test only patch with just deleting the user and running the sensor, then the exception appears.
But I dont know if is the best solution just checking if we have current users with the id.
Comment #10
berdirDeleting a user is a valid use case, that we need to account for.
Patch looks good. I'm just wondering if we should explicitly list that user as deleted in the table, without any other data.
Comment #11
giancarlosotelo commentedWe are creating new tables here #2581701: Add a second table to UserIntegrity sensor : "Users with privileged access" so what about creating a new table for deleted users?
Comment #12
miro_dietikerNew table is one thing... But things fall apart with too many tables... I also thought about putting both "new" and "deleted" data into the first table with an extra column...
But i'm not sure how much data we have from that deleted user and if it fits the first table well...
Possibly a separate table is best indeed.
Comment #13
giancarlosotelo commentedRebase and I am attaching a screenshot with the new table. I am not sure if there is another way to list the deleted users. So some feedback needed then I will add tests and probably a linkeable wid(?).
It would be better if I create a followup for the table? because this issue is about the fatal error, but somehow is related to the new table.
Comment #14
miro_dietikerAs discussed the deleted users should be identified by comparing our sensor array containing the expected situation... not just list all the deleted users. :-)
Comment #15
giancarlosotelo commentedChanged the approach using the sensor array and tests.
Comment #16
giancarlosotelo commentedComment #17
miro_dietikerThx, works for me. Committed. Retitling.
Fixing of the date for Drupal's API will happen here #2603272: Use Drupal's format date API in all the SensorPlugins