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

D8AGE-291: The translate actions. #309

Merged
merged 56 commits into from
Jun 3, 2024
Merged

Conversation

intelektron
Copy link
Member

D8AGE-291

Description

This PR adds actions to the translate page. Thanks to it, you can work with translations and accept them.

@intelektron
Copy link
Member Author

Besides the minor comments on the ticket I encountered some errors while doing the functional testing, here are the steps to reproduce:

This was actually the problem with the example.com addresses, locally I did an override for them so I didn't notice the problem. It turns out that the http_request_mock defines a sample plugin that returns the Mocking example.com response text for all URLs in that domain. I had to adjust the weights of our plugin to fix the problem. They are now checked before the example plugin.

The drone issues have also been fixed, probably an update to phpcs as it started complaining about trailing commas in multiline function definitions. This has also been fixed. Now the drone fails in the composer install, I'll try to trigger it again later.

@@ -221,4 +304,24 @@ protected function assertLogMessagesTable(array $logs): void {
$this->assertEquals($logs, $actual);
}

Copy link
Member

Choose a reason for hiding this comment

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

I am missing the cases where a language is cancelled. From the top of my head:

  • One request with two languages: an accepted language and a cancelled language -> request finished
  • One request with one language: cancelled language -> request finished.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, I've also added a function that creates a new translation request. I'll move it to a trait when we have more functional tests.

foreach ($all_statuses as $status => $label) {
$operations[$status] = [
'title' => sprintf('Update status to %s (mock)', $label),
'url' => Url::fromRoute('oe_translation_cdt_mock.request_status', [
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why but when using the "In progress" update option, there is no information logged in the request. Afaik we do not have an ongoing status for our requests but we should still log the update in the request logs. Same thing happens for the job updates

Copy link
Member Author

Choose a reason for hiding this comment

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

We've got only these states of request in the CDT:

  • COMP - The request has been completed and delivered.
  • INPR - The request is in progress or request for quotation approved.
  • CANC - The request has been cancelled
  • PEND - The request quotation is ready to be approved by the client (Requests for Quotation only).
  • UNDE - The request quotation is under quotation.

So I have no way to distinguish "requested" from "in progress".

The log is only written when there is an actual change. When you start the translation, it is already in INPR state.

/**
* Implements hook_ENTITY_TYPE_presave().
*/
function oe_translation_cdt_oe_translation_request_presave(TranslationRequestCdtInterface $request) {
Copy link
Member

Choose a reason for hiding this comment

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

If both the cdt and poetry module are enabled, this will fail because poetry will save entities with a different interface. The paramer type should be TranslationRequestInterface and then we check whether it implements the cdt interface

@@ -140,49 +140,132 @@ public function testCdtSingleTranslationFlow(): void {
$translator->setProviderConfiguration($configuration);
$translator->save();

$node = $this->createBasicTestNode('oe_demo_translatable_page', "The translation's page");
Copy link
Member Author

Choose a reason for hiding this comment

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

I've just moved it to the separate function, but the diff looks weird here...

D8AGE-341: Create dashboard for CDT translations.
@intelektron intelektron merged commit 7f64091 into EPIC-D8AGE-281-cdt Jun 3, 2024
1 check passed
@intelektron intelektron deleted the D8AGE-291 branch June 3, 2024 10:26
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.

None yet

3 participants