@snake14 opened this Pull Request on August 31st 2022 Contributor

Description:

Posting to new activity to track when a customer requests report invalidation via the console. This is part of DEV-2669. There's a PR in the ActivityLog plugin to add the new activity so that it can be tracked.

Review

@sgiehl commented on September 1st 2022 Member

@snake14 I'm not sure if adding this event is useful that way. It might help with the issue to see which invalidation were triggered when using this command. But imho for ActivityLog it might be enough to simply display that the command was run, together with the options that were set. There is already an event being triggered when a console command is executed: Console.doRun
This could be most likely used for that purpose I guess. Or maybe @tsteur has other thoughts on that.

@tsteur commented on September 1st 2022 Member

👍 would say as well be probably best to trigger only one activity for the command as otherwise it may result in a lot of entries. If there's an event we can already reuse that be good.

@snake14 commented on September 1st 2022 Contributor

@sgiehl @tsteur Thank you. I was unaware of the Console.doRun event. I searched the list of events, in an effort to avoid changing core, but I didn't see anything I thought I could use in this case. The closest I could find was Console.filterCommands, but that didn't seem to match what I needed. Was it intentional to leave out Console.doRun from that list or should it be added?

@justinvelluppillai commented on September 5th 2022 Member

Hey @snake14 can this be closed now?

@snake14 commented on September 5th 2022 Contributor

@justinvelluppillai Good call. I'll close it now. The only remaining question was whether the Console.doRun event was intentionally left out of the documentation or if it should be added. I also noticed that no event data was passed for that event, so I had to use $_SERVER['argv'] to see which command was used. Is there a better way to do that?

@justinvelluppillai commented on September 5th 2022 Member

Console.doRun has been purposely left out of docs.

I'm not sure the best way to get the args, maybe one of the Symfony classes would work but @sgiehl maybe would have suggestions there?

@snake14 commented on September 5th 2022 Contributor

@justinvelluppillai Thank you. That's what I suspected when I saw that it had an ignore annotation on it, but I wanted to make sure. I've got my hook for Console.doRun working using $_SERVER['argv'], but I'm always open to better options.

@justinvelluppillai commented on September 6th 2022 Member

@snake14 doesn't the event pass $input? That could be used to get what you need if I'm not mistaken.

@snake14 commented on September 6th 2022 Contributor

@justinvelluppillai Yeah. It looks like in core/Console.php, it passes the following array: [&$exitCode, $input, $output]. However, when I debugged the hook in the ActivityLog plugin, the $eventData was null instead of the array that I was expecting.

@snake14 commented on September 6th 2022 Contributor

@justinvelluppillai Altamash noticed that the issue was that I defined my hook method with too few arguments. I'll try adjusting that. I figured I was doing something wrong when I had to use the $_SERVER global, I just didn't know what I was missing.

@justinvelluppillai commented on September 6th 2022 Member

yep nice one, I saw that 👍🏽

This Pull Request was closed on September 5th 2022
Powered by GitHub Issue Mirror