Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

declare(strict_types=1);

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class extends Migration
{
public function up(): void
{
Schema::table('github_repositories', static function (Blueprint $table): void {
$table->string('purpose')->default('contributions');
});
}
Comment on lines +12 to +19

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following the idea of implementing an enum for this value, I would use a nullable string rather than a default value. Then, I would add an extra step to fill it with the default values.

Note

Do not use enums in migrations.
This could be impacted by a change in the future.
It will not be implemented if migration has already occurred; it will only be implemented on new databases.

Suggested change
public function up(): void
{
Schema::table('github_repositories', static function (Blueprint $table): void {
$table->string('purpose')->default('contributions');
});
}
public function up(): void
{
Schema::table('github_repositories', static function (Blueprint $table): void {
$table->string('purpose')->nullable();
});
DB::table('github_repositories')
->update(['purpose' => 'contributions']);
}

Before applying/implementing this suggestion, I would like to hear from @danielhe4rt and @Clintonrocha98. Default values aren't a problem, but they could be in the future.

};
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
* @property string $tenant_id
* @property string $full_name
* @property bool $enabled
* @property string $purpose

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't 'purpose' be an enum rather than a string here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this but i dunno if the "purpose" will grow more or if will remain only this two (contributions | challenge)

So is it better to create a enum?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is it better to create a enum?

IMO yes, it will prevent the spread of a possible incorrect string in the system, and at the same time, we will keep it as a strong type, specifying that it will always be only one of these values.

* @property CarbonInterface|null $last_backfilled_at
* @property CarbonInterface|null $created_at
* @property CarbonInterface|null $updated_at
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ private function tenantsTracking(string $repo): array
return array_values(GithubRepository::query()
->enabled()
->where('full_name', $repo)
->where('purpose', 'contributions')
->pluck('tenant_id')
->map(static fn (mixed $tenantId): string => (string) $tenantId)
->all());
Expand Down
38 changes: 38 additions & 0 deletions app-modules/integration-github/tests/Feature/GithubWebhookTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,41 @@ function prWebhookPayload(string $repo = 'he4rt/heartdevs.com', int $number = 1,
expect(GithubContribution::query()->where('external_ref', 'issue:10')->exists())->toBeTrue()
->and(GithubContribution::query()->where('external_ref', 'commit:sha1')->exists())->toBeTrue();
});

it('grava no lake mas NÃO projeta contribuição para repo de challenge', function (): void {
Event::fake([GithubContributionRecorded::class]);

GithubRepository::factory()->create([
'full_name' => 'he4rt/heartdevs.com',
'purpose' => 'challenge',
]);

postGithubWebhook('pull_request', prWebhookPayload())->assertSuccessful();

expect(GithubEventLog::query()->count())->toBe(1)
->and(GithubContribution::query()->count())->toBe(0);

Event::assertNotDispatched(GithubContributionRecorded::class);
});

it('grava no lake e projeta a contribuição para repo de contributions, emitindo o evento', function (): void {
Event::fake([GithubContributionRecorded::class]);

$repo = GithubRepository::factory()->create([
'full_name' => 'he4rt/heartdevs.com',
'purpose' => 'contributions',
]);

postGithubWebhook('pull_request', prWebhookPayload())->assertSuccessful();

expect(GithubEventLog::query()->count())->toBe(1);

$contribution = GithubContribution::query()->where('external_ref', 'pr:1')->sole();

expect($contribution->type)->toBe(ContributionType::Pr)
->and($contribution->tenant_id)->toBe($repo->tenant_id)
->and($contribution->actor_login)->toBe('maria')
->and($contribution->metadata['additions'])->toBe(5);

Event::assertDispatched(GithubContributionRecorded::class);
});