Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

run cleanup command after upload and ask to cleanup terms in poeditor #24

Merged
merged 4 commits into from
Feb 9, 2024

Conversation

yinx
Copy link
Contributor

@yinx yinx commented Feb 5, 2024

No description provided.

@yinx yinx self-assigned this Feb 5, 2024
Comment on lines 42 to 59
$diff = collect(app(Poeditor::class)->download($this->getPoeditorLocale()))->dot()
->diff(collect($translations)->dot());

if ($diff->isEmpty()) {
$this->info('The translations match the ones on POEditor');
} else {
$this->error('The following translations do not match the ones on POEditor:');

$this->table(['Key', 'Value'], $diff->map(function ($value, $key) {
return [$key, $value];
}));

if ($this->ask('Do you want to clean up the translations on POEditor? (y/n)')) {
$response = app(Poeditor::class)->upload($this->getPoeditorLocale(), $translations, true, true);

$this->info("Deleted {$response->getDeletedTermsCount()} terms");
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$diff = collect(app(Poeditor::class)->download($this->getPoeditorLocale()))->dot()
->diff(collect($translations)->dot());
if ($diff->isEmpty()) {
$this->info('The translations match the ones on POEditor');
} else {
$this->error('The following translations do not match the ones on POEditor:');
$this->table(['Key', 'Value'], $diff->map(function ($value, $key) {
return [$key, $value];
}));
if ($this->ask('Do you want to clean up the translations on POEditor? (y/n)')) {
$response = app(Poeditor::class)->upload($this->getPoeditorLocale(), $translations, true, true);
$this->info("Deleted {$response->getDeletedTermsCount()} terms");
}
}
collect(app(Poeditor::class)->download($this->getPoeditorLocale()))
->dot()
->keys()
->diff(collect($translations)->dot()->keys())
->whenNotEmpty(function ($locallyDeletedTranslationsKeys) use ($translations) {
$this->error('The following translation keys do not exist locally but do exist in POEditor:');
$this->table(
['Translation Key'],
$locallyDeletedTranslationsKeys->map(fn ($key) => [$key])->all()
);
if (! $this->confirm('Do you want to delete those translation keys in POEditor? (y/n)')) {
return;
}
$response = app(Poeditor::class)->upload($this->getPoeditorLocale(), $translations, true, true);
$this->info("Deleted {$response->getDeletedTermsCount()} terms");
});

Copy link
Collaborator

Choose a reason for hiding this comment

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

@yinx Maybe we can make it even simpler.

Now we first upload, then download and compare the download with local, and finally we upload to delete terms

But what if we first download, then compare the download with local, and then upload with correct "sync" parameter 🤔 Then only one upload as necessary and only upload response needs to be shown

$this->artisan('poeditor:upload --force')
->assertExitCode(0);
}

/** @test */
public function it_asks_for_confirmation_when_translations_do_not_match_with_cleanup_enabled()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public function it_asks_for_confirmation_when_translations_do_not_match_with_cleanup_enabled()
public function it_asks_for_confirmation_when_local_translations_keys_do_not_match_translations_keys_in_poeditor()

$this->artisan('poeditor:upload nl_BE')->assertExitCode(0);
}

/** @test */
public function it_checks_the_local_translation_keys_with_the_downloaded_keys()
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this not covered by all the other tests 🤔

['php-file.baz', 'bar'],
]
)
->expectsOutput('Deleted ' . 1 . ' terms')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
->expectsOutput('Deleted ' . 1 . ' terms')
->expectsOutput("{$response->getDeletedTermsCount()} terms deleted")

Comment on lines 232 to 237
], true, true, new UploadResponse([
'result' => [
'terms' => ['added' => $this->faker->randomNumber(), 'deleted' => 1],
'translations' => ['added' => $this->faker->randomNumber(), 'updated' => $this->faker->randomNumber()],
],
]));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
], true, true, new UploadResponse([
'result' => [
'terms' => ['added' => $this->faker->randomNumber(), 'deleted' => 1],
'translations' => ['added' => $this->faker->randomNumber(), 'updated' => $this->faker->randomNumber()],
],
]));
], true, true, $response = $this->getPoeditorUploadResponse());

@yinx yinx requested a review from gdebrauwer February 6, 2024 09:14
@yinx yinx force-pushed the feature/add_check_translations_command branch from 1a2a473 to decc228 Compare February 7, 2024 08:11
Base automatically changed from feature/add_check_translations_command to master February 7, 2024 08:13
@yinx yinx force-pushed the feature/sync_translations_in_poeditor branch from f241b56 to 24c28ac Compare February 7, 2024 08:43
Comment on lines 27 to 44
$cleanup = false;

collect(app(Poeditor::class)->download($this->getPoeditorLocale()))
->dot()
->keys()
->diff(collect($translations)->dot()->keys())
->whenNotEmpty(function ($locallyDeletedTranslationsKeys) use (&$cleanup) {
$this->error('The following translation keys do not exist locally but do exist in POEditor:');

$this->table(
['Translation Key'],
$locallyDeletedTranslationsKeys->map(fn ($key) => [$key])->all()
);

if ($this->confirm('Do you want to delete those translation keys in POEditor? (y/n)')) {
$cleanup = true;
}
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$cleanup = false;
collect(app(Poeditor::class)->download($this->getPoeditorLocale()))
->dot()
->keys()
->diff(collect($translations)->dot()->keys())
->whenNotEmpty(function ($locallyDeletedTranslationsKeys) use (&$cleanup) {
$this->error('The following translation keys do not exist locally but do exist in POEditor:');
$this->table(
['Translation Key'],
$locallyDeletedTranslationsKeys->map(fn ($key) => [$key])->all()
);
if ($this->confirm('Do you want to delete those translation keys in POEditor? (y/n)')) {
$cleanup = true;
}
});
$cleanup = collect(app(Poeditor::class)->download($this->getPoeditorLocale()))
->dot()
->keys()
->diff(collect($translations)->dot()->keys())
->whenNotEmpty(function ($locallyDeletedTranslationsKeys) use (&$cleanup) {
$this->error('The following translation keys do not exist locally but do exist in POEditor:');
$this->table(
['Translation Key'],
$locallyDeletedTranslationsKeys->map(fn ($key) => [$key])->all()
);
return $this->confirm('Do you want to delete those translation keys in POEditor? (y/n)');
}, fn () => false);

Comment on lines 215 to 220
$this->mockPoeditorUpload('en', [
'php-file' => [
'bar' => 'foo',
],
]);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$this->mockPoeditorUpload('en', [
'php-file' => [
'bar' => 'foo',
],
]);

can be removed as this upload is never executed

@yinx yinx merged commit 6b16dfd into master Feb 9, 2024
14 checks passed
@yinx yinx deleted the feature/sync_translations_in_poeditor branch February 9, 2024 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants