feat(userEventListeners): add UserEventListener and UserStatsJob - #17
Conversation
1e3bad4 to
4bc604c
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds user lifecycle telemetry to the NCW Tools app by registering a new UserEventListener for Nextcloud user create/delete events and a UserStatsJob background job that logs aggregate user-count data.
Changes:
- Added
UserEventListenerto react toUserCreatedEventandUserDeletedEvent. - Added
UserStatsJobto collect total user count and log the stats payload asynchronously. - Updated application registration, Psalm config, and unit tests for the new listener/job wiring.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/Listeners/UserEventListenerTest.php | Adds unit coverage for create/delete/unsupported event handling. |
| tests/unit/BackgroundJob/UserStatsJobTest.php | Adds unit coverage for success and failure paths of the new background job. |
| tests/unit/AppInfo/ApplicationTest.php | Extends bootstrap registration tests for the new user event listener. |
| psalm.xml | Suppresses possibly-unused constructor warnings for new DI-managed classes. |
| lib/Listeners/UserEventListener.php | Implements the new listener that logs user events and enqueues the stats job. |
| lib/BackgroundJob/UserStatsJob.php | Implements the queued job that logs the user-count payload. |
| lib/AppInfo/Application.php | Registers the new listener for user created/deleted events. |
4bc604c to
b88779b
Compare
bromiesTM
left a comment
bromiesTM
left a comment
There was a problem hiding this comment.
As Copilot said, the eventlistener does to much. It should just start the background job which handles the stat collection.
b88779b to
f08c24c
Compare
| public function testRegisterMethodExists(): void { | ||
| // Test that register method registers the InstallationCompletedEvent listener | ||
| $this->registrationContext | ||
| ->expects($this->once()) | ||
| ->expects($this->exactly(3)) | ||
| ->method('registerEventListener') | ||
| ->with( | ||
| InstallationCompletedEvent::class, | ||
| InstallationCompletedEventListener::class | ||
| ); | ||
| ->willReturnCallback(function (string $event, string $listener): void { | ||
| // no assertions needed; just allow the three calls | ||
| }); | ||
|
|
||
| $this->application->register($this->registrationContext); |
|
@copilot what happens if there are multiple jobs at the same time? They would report the same number of users and therefore it makes no sense to have more at one job at the same time? Is that correct? |
Fixed in eccdac3. The listener now only logs the event and enqueues the job — and with the deduplication added, it skips adding a new
Yes, exactly right. Multiple queued jobs would all read the same user count when they execute, producing redundant identical log entries. Fixed in eccdac3 by checking |
eccdac3 to
6078837
Compare
6078837 to
cfad0e8
Compare
| $this->logger->info('user added: ' . $event->getUid()); | ||
| } else { | ||
| $this->logger->info('user deleted: ' . $event->getUid()); |
| public function testRegisterMethodExists(): void { | ||
| // Test that register method registers the InstallationCompletedEvent listener | ||
| $this->registrationContext | ||
| ->expects($this->once()) | ||
| ->expects($this->exactly(3)) | ||
| ->method('registerEventListener') | ||
| ->with( | ||
| InstallationCompletedEvent::class, | ||
| InstallationCompletedEventListener::class | ||
| ); | ||
| ->willReturnCallback(function (string $event, string $listener): void { | ||
| // no assertions needed; just allow the three calls | ||
| }); | ||
|
|
||
| $this->application->register($this->registrationContext); | ||
| } |
|
@copilot apply changes based on the comments in this thread fix with autosquashable fixup commits |
Done. Two autosquashable fixup commits added targeting
|
ecf511e to
442ccb4
Compare
Listens to UserCreatedEvent and UserDeletedEvent to log synchronous info entries and schedule a one-shot UserStatsJob that collects the total user count and logs the NSW-842 stats payload. Signed-off-by: Arsalan Ul Haq Sohni <arsalan-ul-haq.sohni@strato.de>
442ccb4 to
9434d93
Compare
Listens to UserCreatedEvent and UserDeletedEvent to log synchronous info entries and schedule a one-shot UserStatsJob that collects the total user count and logs the NSW-842 stats payload.