#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
14 changed files with 14 additions and 14 deletions
Showing only changes of commit 160eb5f94b - Show all commits

View File

@@ -22,4 +22,4 @@ class SlackApiChannel
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?
"text" => $notification->toSlack($notifiable), "text" => $notification->toSlack($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: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?
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?
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?

View File

@@ -52,4 +52,4 @@ class Controller extends SlackController
krzysztofrewak commented 2022-04-26 10:11:59 +02:00 (Migrated from github.com)
Review

Please add return type here.

Please add return type here.
krzysztofrewak commented 2022-04-26 10:11:59 +02:00 (Migrated from github.com)
Review

Please add return type here.

Please add return type here.
krzysztofrewak commented 2022-04-26 10:12:17 +02:00 (Migrated from github.com)
Review

@EwelinaLasowy , what do you think about word Komenda? :>

@EwelinaLasowy , what do you think about word Komenda? :>
krzysztofrewak commented 2022-04-26 10:12:17 +02:00 (Migrated from github.com)
Review

@EwelinaLasowy , what do you think about word Komenda? :>

@EwelinaLasowy , what do you think about word Komenda? :>
->withText(":x: Komenda `/{$this->request->command} {$this->request->text}` jest niepoprawna:") ->withText(":x: Komenda `/{$this->request->command} {$this->request->text}` jest niepoprawna:")
->withAttachments($errors->all()); ->withAttachments($errors->all());
} }
} }
krzysztofrewak commented 2022-04-26 10:11:59 +02:00 (Migrated from github.com)
Review

Please add return type here.

Please add return type here.
krzysztofrewak commented 2022-04-26 10:12:17 +02:00 (Migrated from github.com)
Review

@EwelinaLasowy , what do you think about word Komenda? :>

@EwelinaLasowy , what do you think about word Komenda? :>
krzysztofrewak commented 2022-04-26 10:11:59 +02:00 (Migrated from github.com)
Review

Please add return type here.

Please add return type here.
krzysztofrewak commented 2022-04-26 10:12:17 +02:00 (Migrated from github.com)
Review

@EwelinaLasowy , what do you think about word Komenda? :>

@EwelinaLasowy , what do you think about word Komenda? :>
krzysztofrewak commented 2022-04-26 10:11:59 +02:00 (Migrated from github.com)
Review

Please add return type here.

Please add return type here.
krzysztofrewak commented 2022-04-26 10:11:59 +02:00 (Migrated from github.com)
Review

Please add return type here.

Please add return type here.
krzysztofrewak commented 2022-04-26 10:12:17 +02:00 (Migrated from github.com)
Review

@EwelinaLasowy , what do you think about word Komenda? :>

@EwelinaLasowy , what do you think about word Komenda? :>
krzysztofrewak commented 2022-04-26 10:12:17 +02:00 (Migrated from github.com)
Review

@EwelinaLasowy , what do you think about word Komenda? :>

@EwelinaLasowy , what do you think about word Komenda? :>

View File

@@ -8,4 +8,4 @@ use Spatie\SlashCommand\Exceptions\SlackSlashCommandException;
class UserNotFoundException extends SlackSlashCommandException class UserNotFoundException extends SlackSlashCommandException
{ {
} }

View File

@@ -32,4 +32,4 @@ class CatchAll extends BaseHandler
->setFields($attachmentFields), ->setFields($attachmentFields),
); );
} }
} }

View File

@@ -52,4 +52,4 @@ class DailySummary extends 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: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?
->withAttachment($remoteAttachment) ->withAttachment($remoteAttachment)
->withAttachment($birthdayAttachment); ->withAttachment($birthdayAttachment);
} }
} }
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: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: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

@@ -68,4 +68,4 @@ class GiveKeysTo extends SignatureHandler
"user.required" => "Musisz podać użytkownika, któremu chcesz przekazać klucze", "user.required" => "Musisz podać użytkownika, któremu chcesz przekazać klucze",
]; ];
} }
} }

View File

@@ -30,4 +30,4 @@ class Help extends SignatureHandler
->setFields($attachmentFields), ->setFields($attachmentFields),
); );
} }
} }

View File

@@ -42,4 +42,4 @@ class HomeOffice extends SignatureHandler
"flow_skipped" => false, "flow_skipped" => false,
], $user); ], $user);
} }
} }

View File

@@ -28,4 +28,4 @@ class KeyList extends SignatureHandler
->setText($keys->isNotEmpty() ? $keys->implode("\n") : "Nie ma żadnych kluczy w tobym"), ->setText($keys->isNotEmpty() ? $keys->implode("\n") : "Nie ma żadnych kluczy w tobym"),
); );
} }
} }

View File

@@ -23,4 +23,4 @@ abstract class SignatureHandler extends BaseSignatureHandler
{ {
return []; return [];
} }
} }

View File

@@ -67,4 +67,4 @@ class TakeKeysFrom extends SignatureHandler
"user.required" => "Musisz podać użytkownika, któremu chcesz zabrać klucze", "user.required" => "Musisz podać użytkownika, któremu chcesz zabrać klucze",
]; ];
} }
} }

View File

@@ -21,4 +21,4 @@ class SlackUserExistsRule implements Rule
{ {
return "Użytkownik :input nie istnieje w tobym"; return "Użytkownik :input nie istnieje w tobym";
} }
} }

View File

@@ -40,4 +40,4 @@ 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 `
{ {
return Str::between($slackId, "<@", "|"); return Str::between($slackId, "<@", "|");
} }
} }
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 `
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 `
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 `

View File

@@ -42,4 +42,4 @@ trait ListsHandlers
) )
->all(); ->all();
} }
} }