send a notification on crawler success/failure #421
Conversation
| ) { | ||
| parent::__construct($projectRepository, $logger); | ||
| $this->repoCrawlerRegistry = $repoCrawlerFactory; | ||
| $this->entityManager = $entityManager; | ||
| $this->notificationCreator = $notificationCreatorService; |
There was a problem hiding this comment.
Use consistent naming: call the member variable notificationCreatorService as well.
| @@ -90,6 +117,21 @@ protected function processProject(Project $project) : bool | |||
| // Ignore -- we're already in the error handling path. | |||
| // The project will most likely remain in the "in processing" | |||
| // state. | |||
| // Send a notification that the project cannot be processed | |||
| $users = new ArrayCollection(); | |||
| ->setType("crawler_success") | ||
| ->setRecipient($user); | ||
| $this->notificationCreator->createNotification($notification); | ||
| } |
There was a problem hiding this comment.
Only send a notification in case of a failure.
| } else { | ||
| $users->add($project->getParentUser()); | ||
| } | ||
| foreach ($users as $user) { |
There was a problem hiding this comment.
How about adding a helper function to the NotificationCreatorService class, e.g. notifyProjectOwners($project, $notification), that wraps this functionality?
| foreach ($users as $user) { | ||
| $notification = new Notification(); | ||
| $notification->setSubject("Crawler Failure") | ||
| ->setMessage("Our RepoCrawler has failed to fetch metrics for your project") |
There was a problem hiding this comment.
Users will get this message without knowing how exactly LibreCores processes their data. They also want to have information on how to fix the problem. To do so, (a) give as much information as possible; include potential error messages we get, (b) include a way to fix the problem (e.g. fix repository permissions in GitHub), and (c) don't use any jargon. "Our RepoCrawler" for example isn't helpful, "On LibreCores, we regularly update your project listing with the latest data from your GitHub repository at github.com/asdf/asdf. Unfortunately, the last time we tried, we encountered this error: xxx. To fix this error, you can xzy"
imphil
force-pushed
the
master
branch
2 times, most recently
from
4ea38ec to
68e81af
Compare
For #419