#116 - integration with slack #129

Merged
Baakoma merged 19 commits from #116-integration-with-slack into main 2022-04-27 09:57:13 +02:00
12 changed files with 10 additions and 16 deletions
Showing only changes of commit 54faa3460c - Show all commits

View File

@@ -9,7 +9,7 @@ use Illuminate\Notifications\ChannelManager;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Notification;
use Illuminate\Support\ServiceProvider;
use Toby\Domain\Slack\SlackApiChannel;
use Toby\Domain\Slack\Channels\SlackApiChannel;
class AppServiceProvider extends ServiceProvider
{

View File

@@ -2,7 +2,7 @@
krzysztofrewak commented 2022-04-26 10:09:19 +02:00 (Migrated from github.com)
Review

Can you type $notifiable?

Can you type `$notifiable`?
krzysztofrewak commented 2022-04-26 10:09:19 +02:00 (Migrated from github.com)
Review

Can you type $notifiable?

Can you type `$notifiable`?
krzysztofrewak commented 2022-04-26 10:09:51 +02:00 (Migrated from github.com)
Review

Hm, I don't if I wouldn't put his to Infrastucture directory.

Hm, I don't if I wouldn't put his to Infrastucture directory.
krzysztofrewak commented 2022-04-26 10:09:51 +02:00 (Migrated from github.com)
Review

Hm, I don't if I wouldn't put his to Infrastucture directory.

Hm, I don't if I wouldn't put his to Infrastucture directory.
krzysztofrewak commented 2022-04-26 10:11:01 +02:00 (Migrated from github.com)
Review

Could you extract this config to separate method? It would make further testing easier, I think.

Could you extract this config to separate method? It would make further testing easier, I think.
krzysztofrewak commented 2022-04-26 10:11:01 +02:00 (Migrated from github.com)
Review

Could you extract this config to separate method? It would make further testing easier, I think.

Could you extract this config to separate method? It would make further testing easier, I think.
Baakoma commented 2022-04-26 11:00:45 +02:00 (Migrated from github.com)
Review

In general, it can be anything that uses trait Notifiable

In general, it can be anything that uses trait `Notifiable`
Baakoma commented 2022-04-26 11:00:45 +02:00 (Migrated from github.com)
Review

In general, it can be anything that uses trait Notifiable

In general, it can be anything that uses trait `Notifiable`
krzysztofrewak commented 2022-04-26 11:17:21 +02:00 (Migrated from github.com)
Review

Oh no, maybe you could add some interface fot that?

Oh no, maybe you could add some interface fot that?
krzysztofrewak commented 2022-04-26 11:17:21 +02:00 (Migrated from github.com)
Review

Oh no, maybe you could add some interface fot that?

Oh no, maybe you could add some interface fot that?
declare(strict_types=1);
namespace Toby\Domain\Slack;
krzysztofrewak commented 2022-04-26 10:09:19 +02:00 (Migrated from github.com)
Review

Can you type $notifiable?

Can you type `$notifiable`?
krzysztofrewak commented 2022-04-26 10:09:51 +02:00 (Migrated from github.com)
Review

Hm, I don't if I wouldn't put his to Infrastucture directory.

Hm, I don't if I wouldn't put his to Infrastucture directory.
krzysztofrewak commented 2022-04-26 10:11:01 +02:00 (Migrated from github.com)
Review

Could you extract this config to separate method? It would make further testing easier, I think.

Could you extract this config to separate method? It would make further testing easier, I think.
Baakoma commented 2022-04-26 11:00:45 +02:00 (Migrated from github.com)
Review

In general, it can be anything that uses trait Notifiable

In general, it can be anything that uses trait `Notifiable`
krzysztofrewak commented 2022-04-26 11:17:21 +02:00 (Migrated from github.com)
Review

Oh no, maybe you could add some interface fot that?

Oh no, maybe you could add some interface fot that?
namespace Toby\Domain\Slack\Channels;
krzysztofrewak commented 2022-04-26 10:09:19 +02:00 (Migrated from github.com)
Review

Can you type $notifiable?

Can you type `$notifiable`?
krzysztofrewak commented 2022-04-26 10:09:51 +02:00 (Migrated from github.com)
Review

Hm, I don't if I wouldn't put his to Infrastucture directory.

Hm, I don't if I wouldn't put his to Infrastucture directory.
krzysztofrewak commented 2022-04-26 10:11:01 +02:00 (Migrated from github.com)
Review

Could you extract this config to separate method? It would make further testing easier, I think.

Could you extract this config to separate method? It would make further testing easier, I think.
Baakoma commented 2022-04-26 11:00:45 +02:00 (Migrated from github.com)
Review

In general, it can be anything that uses trait Notifiable

In general, it can be anything that uses trait `Notifiable`
krzysztofrewak commented 2022-04-26 11:17:21 +02:00 (Migrated from github.com)
Review

Oh no, maybe you could add some interface fot that?

Oh no, maybe you could add some interface fot that?
use Illuminate\Http\Client\Response;
use Illuminate\Notifications\Notification;

View File

@@ -2,7 +2,7 @@
declare(strict_types=1);
namespace Toby\Domain\Slack;
namespace Toby\Domain\Slack\Exceptions;
use Spatie\SlashCommand\Exceptions\SlackSlashCommandException;

View File

@@ -9,7 +9,6 @@ use Spatie\SlashCommand\Attachment;
krzysztofrewak commented 2022-04-26 10:13:22 +02:00 (Migrated from github.com)
Review
        $absences = $dailySummaryRetriever->getAbsences($now)
            ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name);

        $remoteDays = $dailySummaryRetriever->getRemoteDays($now)
            ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name);

        $birthdays = $dailySummaryRetriever->getBirthdays($now)
            ->map(fn(User $user): string => $user->profile->full_name);
```suggestion $absences = $dailySummaryRetriever->getAbsences($now) ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name); $remoteDays = $dailySummaryRetriever->getRemoteDays($now) ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name); $birthdays = $dailySummaryRetriever->getBirthdays($now) ->map(fn(User $user): string => $user->profile->full_name); ```
krzysztofrewak commented 2022-04-26 10:13:22 +02:00 (Migrated from github.com)
Review
        $absences = $dailySummaryRetriever->getAbsences($now)
            ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name);

        $remoteDays = $dailySummaryRetriever->getRemoteDays($now)
            ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name);

        $birthdays = $dailySummaryRetriever->getBirthdays($now)
            ->map(fn(User $user): string => $user->profile->full_name);
```suggestion $absences = $dailySummaryRetriever->getAbsences($now) ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name); $remoteDays = $dailySummaryRetriever->getRemoteDays($now) ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name); $birthdays = $dailySummaryRetriever->getBirthdays($now) ->map(fn(User $user): string => $user->profile->full_name); ```
krzysztofrewak commented 2022-04-26 10:14:04 +02:00 (Migrated from github.com)
Review

What if we would have five more activities? Maybe this could be encapsulated somehow?

What if we would have five more activities? Maybe this could be encapsulated somehow?
krzysztofrewak commented 2022-04-26 10:14:04 +02:00 (Migrated from github.com)
Review

What if we would have five more activities? Maybe this could be encapsulated somehow?

What if we would have five more activities? Maybe this could be encapsulated somehow?
use Spatie\SlashCommand\Request;
use Spatie\SlashCommand\Response;
use Toby\Domain\DailySummaryRetriever;
use Toby\Domain\Slack\SignatureHandler;
krzysztofrewak commented 2022-04-26 10:13:22 +02:00 (Migrated from github.com)
Review
        $absences = $dailySummaryRetriever->getAbsences($now)
            ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name);

        $remoteDays = $dailySummaryRetriever->getRemoteDays($now)
            ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name);

        $birthdays = $dailySummaryRetriever->getBirthdays($now)
            ->map(fn(User $user): string => $user->profile->full_name);
```suggestion $absences = $dailySummaryRetriever->getAbsences($now) ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name); $remoteDays = $dailySummaryRetriever->getRemoteDays($now) ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name); $birthdays = $dailySummaryRetriever->getBirthdays($now) ->map(fn(User $user): string => $user->profile->full_name); ```
krzysztofrewak commented 2022-04-26 10:14:04 +02:00 (Migrated from github.com)
Review

What if we would have five more activities? Maybe this could be encapsulated somehow?

What if we would have five more activities? Maybe this could be encapsulated somehow?
use Toby\Eloquent\Models\User;
use Toby\Eloquent\Models\Vacation;
krzysztofrewak commented 2022-04-26 10:13:22 +02:00 (Migrated from github.com)
Review
        $absences = $dailySummaryRetriever->getAbsences($now)
            ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name);

        $remoteDays = $dailySummaryRetriever->getRemoteDays($now)
            ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name);

        $birthdays = $dailySummaryRetriever->getBirthdays($now)
            ->map(fn(User $user): string => $user->profile->full_name);
```suggestion $absences = $dailySummaryRetriever->getAbsences($now) ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name); $remoteDays = $dailySummaryRetriever->getRemoteDays($now) ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name); $birthdays = $dailySummaryRetriever->getBirthdays($now) ->map(fn(User $user): string => $user->profile->full_name); ```
krzysztofrewak commented 2022-04-26 10:13:22 +02:00 (Migrated from github.com)
Review
        $absences = $dailySummaryRetriever->getAbsences($now)
            ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name);

        $remoteDays = $dailySummaryRetriever->getRemoteDays($now)
            ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name);

        $birthdays = $dailySummaryRetriever->getBirthdays($now)
            ->map(fn(User $user): string => $user->profile->full_name);
```suggestion $absences = $dailySummaryRetriever->getAbsences($now) ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name); $remoteDays = $dailySummaryRetriever->getRemoteDays($now) ->map(fn(Vacation $vacation): string => $vacation->user->profile->full_name); $birthdays = $dailySummaryRetriever->getBirthdays($now) ->map(fn(User $user): string => $user->profile->full_name); ```
krzysztofrewak commented 2022-04-26 10:14:04 +02:00 (Migrated from github.com)
Review

What if we would have five more activities? Maybe this could be encapsulated somehow?

What if we would have five more activities? Maybe this could be encapsulated somehow?
krzysztofrewak commented 2022-04-26 10:14:04 +02:00 (Migrated from github.com)
Review

What if we would have five more activities? Maybe this could be encapsulated somehow?

What if we would have five more activities? Maybe this could be encapsulated somehow?

View File

@@ -8,10 +8,9 @@ use Illuminate\Validation\ValidationException;
use Spatie\SlashCommand\Request;
use Spatie\SlashCommand\Response;
use Toby\Domain\Notifications\KeyHasBeenGivenNotification;
use Toby\Domain\Slack\SignatureHandler;
use Toby\Domain\Slack\SlackUserExistsRule;
use Toby\Domain\Slack\Exceptions\UserNotFoundException;
use Toby\Domain\Slack\Rules\SlackUserExistsRule;
use Toby\Domain\Slack\Traits\FindsUserBySlackId;
use Toby\Domain\Slack\UserNotFoundException;
use Toby\Eloquent\Models\Key;
class GiveKeysTo extends SignatureHandler

View File

@@ -7,7 +7,6 @@ namespace Toby\Domain\Slack\Handlers;
use Spatie\SlashCommand\Attachment;
use Spatie\SlashCommand\Request;
use Spatie\SlashCommand\Response;
use Toby\Domain\Slack\SignatureHandler;
use Toby\Domain\Slack\Traits\ListsHandlers;
class Help extends SignatureHandler

View File

@@ -9,7 +9,6 @@ use Spatie\SlashCommand\Request;
use Spatie\SlashCommand\Response;
use Toby\Domain\Actions\VacationRequest\CreateAction;
use Toby\Domain\Enums\VacationType;
use Toby\Domain\Slack\SignatureHandler;
use Toby\Domain\Slack\Traits\FindsUserBySlackId;
use Toby\Eloquent\Models\User;
use Toby\Eloquent\Models\YearPeriod;

View File

@@ -7,7 +7,6 @@ namespace Toby\Domain\Slack\Handlers;
use Spatie\SlashCommand\Attachment;
use Spatie\SlashCommand\Request;
use Spatie\SlashCommand\Response;
use Toby\Domain\Slack\SignatureHandler;
use Toby\Eloquent\Models\Key;
class KeyList extends SignatureHandler

View File

@@ -2,7 +2,7 @@
declare(strict_types=1);
namespace Toby\Domain\Slack;
namespace Toby\Domain\Slack\Handlers;
use Illuminate\Support\Facades\Validator;
use Spatie\SlashCommand\Handlers\SignatureHandler as BaseSignatureHandler;

View File

@@ -8,10 +8,9 @@ use Illuminate\Validation\ValidationException;
use Spatie\SlashCommand\Request;
use Spatie\SlashCommand\Response;
use Toby\Domain\Notifications\KeyHasBeenTakenNotification;
use Toby\Domain\Slack\SignatureHandler;
use Toby\Domain\Slack\SlackUserExistsRule;
use Toby\Domain\Slack\Exceptions\UserNotFoundException;
use Toby\Domain\Slack\Rules\SlackUserExistsRule;
use Toby\Domain\Slack\Traits\FindsUserBySlackId;
use Toby\Domain\Slack\UserNotFoundException;
use Toby\Eloquent\Models\Key;
class TakeKeysFrom extends SignatureHandler

View File

@@ -2,7 +2,7 @@
declare(strict_types=1);
namespace Toby\Domain\Slack;
namespace Toby\Domain\Slack\Rules;
use Illuminate\Contracts\Validation\Rule;
use Illuminate\Support\Str;

View File

@@ -5,7 +5,7 @@ declare(strict_types=1);
krzysztofrewak commented 2022-04-26 10:16:44 +02:00 (Migrated from github.com)
Review
Is it required here? https://github.com/laravel/framework/blob/9.x/src/Illuminate/Collections/Collection.php#L387
krzysztofrewak commented 2022-04-26 10:16:44 +02:00 (Migrated from github.com)
Review
Is it required here? https://github.com/laravel/framework/blob/9.x/src/Illuminate/Collections/Collection.php#L387
Baakoma commented 2022-04-26 10:58:05 +02:00 (Migrated from github.com)
Review

Yes.
Return value is expected to be 'null|\Toby\Eloquent\Models\User', '\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Eloquent\Model|object' returned

Yes. `Return value is expected to be 'null|\Toby\Eloquent\Models\User', '\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Eloquent\Model|object' returned `
Baakoma commented 2022-04-26 10:58:05 +02:00 (Migrated from github.com)
Review

Yes.
Return value is expected to be 'null|\Toby\Eloquent\Models\User', '\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Eloquent\Model|object' returned

Yes. `Return value is expected to be 'null|\Toby\Eloquent\Models\User', '\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Eloquent\Model|object' returned `
namespace Toby\Domain\Slack\Traits;
use Illuminate\Support\Str;
use Toby\Domain\Slack\UserNotFoundException;
krzysztofrewak commented 2022-04-26 10:16:44 +02:00 (Migrated from github.com)
Review
Is it required here? https://github.com/laravel/framework/blob/9.x/src/Illuminate/Collections/Collection.php#L387
Baakoma commented 2022-04-26 10:58:05 +02:00 (Migrated from github.com)
Review

Yes.
Return value is expected to be 'null|\Toby\Eloquent\Models\User', '\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Eloquent\Model|object' returned

Yes. `Return value is expected to be 'null|\Toby\Eloquent\Models\User', '\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Eloquent\Model|object' returned `
use Toby\Domain\Slack\Exceptions\UserNotFoundException;
krzysztofrewak commented 2022-04-26 10:16:44 +02:00 (Migrated from github.com)
Review
Is it required here? https://github.com/laravel/framework/blob/9.x/src/Illuminate/Collections/Collection.php#L387
Baakoma commented 2022-04-26 10:58:05 +02:00 (Migrated from github.com)
Review

Yes.
Return value is expected to be 'null|\Toby\Eloquent\Models\User', '\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Eloquent\Model|object' returned

Yes. `Return value is expected to be 'null|\Toby\Eloquent\Models\User', '\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Eloquent\Model|object' returned `
use Toby\Eloquent\Models\User;
trait FindsUserBySlackId
krzysztofrewak commented 2022-04-26 10:16:44 +02:00 (Migrated from github.com)
Review
Is it required here? https://github.com/laravel/framework/blob/9.x/src/Illuminate/Collections/Collection.php#L387
krzysztofrewak commented 2022-04-26 10:16:44 +02:00 (Migrated from github.com)
Review
Is it required here? https://github.com/laravel/framework/blob/9.x/src/Illuminate/Collections/Collection.php#L387
Baakoma commented 2022-04-26 10:58:05 +02:00 (Migrated from github.com)
Review

Yes.
Return value is expected to be 'null|\Toby\Eloquent\Models\User', '\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Eloquent\Model|object' returned

Yes. `Return value is expected to be 'null|\Toby\Eloquent\Models\User', '\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Eloquent\Model|object' returned `
Baakoma commented 2022-04-26 10:58:05 +02:00 (Migrated from github.com)
Review

Yes.
Return value is expected to be 'null|\Toby\Eloquent\Models\User', '\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Eloquent\Model|object' returned

Yes. `Return value is expected to be 'null|\Toby\Eloquent\Models\User', '\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Eloquent\Model|object' returned `