Skip to content

Add machine translation service#6179

Merged
mrcasals merged 51 commits intodecidim:feat/machine-translationfrom
codegram:add_machine_translation_service
Aug 4, 2020
Merged

Add machine translation service#6179
mrcasals merged 51 commits intodecidim:feat/machine-translationfrom
codegram:add_machine_translation_service

Conversation

@anaghavl
Copy link
Copy Markdown
Contributor

@anaghavl anaghavl commented Jun 10, 2020

Related to #6127.
New Approach with machine translations, we add the translations to the same object. This way we don't need an extra table to store the translations.
ex. title = { :en => "title", :machine_translations{ :ca => "ca-title", :en => "en-title"}}

Why?
The old change made us change the translatable_attribute method. And so it forced us to change a lot of the existing code to make it compatible and some points can't use that (eg component settings).
The new approach allows us to keep human and machine translations identifiable, and lets us use the previous translatable_attribute helper as it is and only change the internals of the method.

Changes in the PR:

  • Created hooks after_create and after_update, that trigger jobs to start the machine translation flow.
  • Create: When a translated_field is changed, we call MachineTranslationResourceJob job.

MachineTranslationResourceJob

  • We call N x M MachineTranslationFieldsJob jobs where N translatable_fields and M locales available.
  • We translate only when the default locale is changed. Why?
  1. Default locale can never be empty, hence we have translations for the other locale at all times.
  2. Since they all mean the same it doesn't make sense to call translations when other locales have been updated.
    That being said, there are a few other cases that are taken care of:
  • Different cases that are covered here:
    Let's assume the default locale is en

Case 1:

  • Duplicate translations are removed
    ie. In case machine_translations exists for a particular locale,
    ex. title = { :en => "title", :machine_translations{ :ca => "ca-title", :en => "en-title"}}
    and we update the title with title = { :en => "title", :ca => "ca- title" }
    Then the duplicate ca translation is removed from machine_translations.
    title = { :en => "title", :machine_translations{:en => "en-title"}}

Case 2:

  • If the user removes the translation of a locale that isn't default locale, since we have removed duplicates the machine translation for the locale will be empty.
    We identify the locale that has been set to "" and trigger a job to get the machine translation for that.

You can see how it looks in each case,
Screenshot 2020-07-23 at 12 35 56 AM

MachineTranslationFieldsJob

  • It calls the Decidim.machine_translation_service which in this case is the DummyTranslator.

DummyTranslator: We have a dummy translator that updates the field with machine_translations => { translation_locale =>"#{translation_locale} - #{field_value}"}

This is just to test the flow of machine translation and hence the storing operations are done inside it.
Jobs can be used to achieve the same if a different translation service is integrated.

Finally, incase there isn't human translation available, we look for machine translations in the translated_attribute to use it in the views.

@virgile-dev
Copy link
Copy Markdown
Contributor

@anaghavl @mrcasals Awesome work ! Does this feature means we will have a saved in the db a translated version for each contribution ? Does the system targets a destination language everytime or does it translate to all the languages activated on the org ? Thanks in advance for your reply looking forward to testing this !

@mrcasals
Copy link
Copy Markdown
Contributor

@virgile-dev the idea is that, when a resource is created or updated, we automatically machine-translate the specified fields. These machine translations will be persisted in the DB, and we will translate them using the organization available locales 😄

There's a missing piece that is the actual integration with a translation service. We'll write an example dummy translator, but real integrations will most probably live outside the main Decidim repo (eg. Google Translate, DeepL, Bing...)

@anaghavl anaghavl requested a review from mrcasals July 1, 2020 04:31
@anaghavl anaghavl changed the title [WIP] Add machine translation service Add machine translation service Jul 1, 2020
@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented Jul 1, 2020

Nice job, @anaghavl!!

I don't understand why that test is failing, though 😕

Copy link
Copy Markdown
Contributor

@mrcasals mrcasals left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented Jul 3, 2020

@decidim/core can you review that? Check #6127 for more context

Copy link
Copy Markdown
Contributor

@tramuntanal tramuntanal left a comment

Choose a reason for hiding this comment

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

Good job @anaghavl , I see the code is clear and well tested 😄

I just have left some questions, I know this goes into another PR but would like to clarify some decisions you are making here, thanks!

@anaghavl anaghavl changed the title Add machine translation service [WIP] Add machine translation service Jul 14, 2020
@anaghavl anaghavl changed the title [WIP] Add machine translation service Add machine translation service Jul 20, 2020
@anaghavl anaghavl changed the title Add machine translation service [WIP]Add machine translation service Jul 22, 2020
@tramuntanal
Copy link
Copy Markdown
Contributor

@anaghavl so this PR is still in WIP?

@anaghavl anaghavl changed the title [WIP]Add machine translation service Add machine translation service Jul 22, 2020
@anaghavl
Copy link
Copy Markdown
Contributor Author

@anaghavl so this PR is still in WIP?

Hi @tramuntanal ,
We changed the approach to machine translations and hence it was WIP. I have tried to explain the new approach and the changes in the description. Please review and let me know if something isn't clear. 😄

@tramuntanal
Copy link
Copy Markdown
Contributor

Hi @anaghavl ,
I see you decided to introduce the machine translations into a new :machine_translations key into each translatable field. Why this change? wasn't it cleaner to have the translations into its own table?

@anaghavl
Copy link
Copy Markdown
Contributor Author

Hi @anaghavl ,
I see you decided to introduce the machine translations into a new :machine_translations key into each translatable field. Why this change? wasn't it cleaner to have the translations into its own table?

The old change made us change the translatable_attribute method. And so it forced us to change a lot of the existing code to make it compatible and some points can't use that (eg component settings).
The new approach allows us to keep human and machine translations identifiable, and lets us use the previous translatable_attribute helper as it is and only change the internals of the method.

Copy link
Copy Markdown
Contributor

@mrcasals mrcasals left a comment

Choose a reason for hiding this comment

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

Looks good to me!!

@anaghavl anaghavl force-pushed the feat/machine-translation branch 2 times, most recently from 4ae9f10 to 5cbf1c3 Compare July 27, 2020 12:09
@tramuntanal
Copy link
Copy Markdown
Contributor

@anaghavl you have conflicting files, can you check please 😃

@anaghavl anaghavl force-pushed the add_machine_translation_service branch from b01cf57 to de56e04 Compare July 28, 2020 05:18
@anaghavl
Copy link
Copy Markdown
Contributor Author

@anaghavl you have conflicting files, can you check please 😃

Sorry again, resolved the conflicts 😄

@anaghavl anaghavl force-pushed the feat/machine-translation branch from 5cbf1c3 to 6cd43aa Compare July 28, 2020 11:36
mrcasals and others added 2 commits July 28, 2020 17:37
* Require confirmation on exit

* Add specs

* Use path instead of url

* Fix changelog

* Trigger build

* Fix expected path on test

* Fix method call

* Take textareas and selects into account
@anaghavl anaghavl force-pushed the add_machine_translation_service branch from aa55ca8 to dc98640 Compare July 28, 2020 12:07
@mrcasals mrcasals requested a review from oriolgual July 29, 2020 13:12
@mrcasals
Copy link
Copy Markdown
Contributor

@decidim/core tests are green now! Can you review it, please? 😄

@mrcasals mrcasals requested a review from tramuntanal July 31, 2020 09:16
@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented Aug 3, 2020

@decidim/core sorry again, can you review this PR please?

@mrcasals mrcasals merged commit 9ed6d69 into decidim:feat/machine-translation Aug 4, 2020
@mrcasals mrcasals deleted the add_machine_translation_service branch August 4, 2020 13:15
mrcasals added a commit that referenced this pull request Aug 14, 2020
Co-authored-by: Marc Riera Casals <mrc2407@gmail.com>
Co-authored-by: Marc Riera <mrc2407@gmail.com>
mrcasals added a commit that referenced this pull request Aug 14, 2020
Co-authored-by: Marc Riera Casals <mrc2407@gmail.com>
Co-authored-by: Marc Riera <mrc2407@gmail.com>
tramuntanal pushed a commit that referenced this pull request Aug 17, 2020
* Base branch

* remove file

* Base branch

* remove file

* Identify translatable resources (#6145)

* Base branch

* remove file

* Require confirmation on exiting a survey mid-answering (#6118)

* Require confirmation on exit

* Add specs

* Use path instead of url

* Fix changelog

* Trigger build

* Fix expected path on test

* Fix method call

* Take textareas and selects into account

* WIP adding concern

* Adding concern in all the models which have translatable fields

* removed :extended_data as translatable field

* WIP adding concern

* Adding concern in all the models which have translatable fields

* Revert "Require confirmation on exiting a survey mid-answering (#6118)"

This reverts commit bdeb933.

* Revert "remove file"

This reverts commit 2565dbb.

* Revert "Base branch"

This reverts commit 2a09cc4.

Co-authored-by: Marc Riera Casals <mrc2407@gmail.com>

* Add Decidim global and organization config for machine translation (#6128)

* Adding setting to organizations table and creating global config

* Adding config accessor to core.rb

* Base branch

* Added a check to display machine translation settings and changed initializer value

* Fixing lint issue in migration file

* Adding test and removing test file

Co-authored-by: decidim-bot <decidim-bot@users.noreply.github.com>
Co-authored-by: anagha <anagha1996@gmail.com>

* Identifying translatable fields in meetings and comments (#6333)

* Base branch

* remove file

* Idenifying translatable fields in meetings and comments

Co-authored-by: Marc Riera Casals <mrc2407@gmail.com>

* Identifying translatable fields for proposals (#6346)

* Add machine translation service (#6179)

Co-authored-by: Marc Riera Casals <mrc2407@gmail.com>
Co-authored-by: Marc Riera <mrc2407@gmail.com>

* Make some fields non-translatable

* Improve spec

* Don't run job if class is not defined

* Improvee method naming

* Machine translation display priority (#6385)

* Add docs on how to enable the integration

* Add docs on how to write a machine translation service

* Improve code strength

* Fix specs

* Fix specs

Co-authored-by: anagha vl <44900292+anaghavl@users.noreply.github.com>
Co-authored-by: decidim-bot <decidim-bot@users.noreply.github.com>
Co-authored-by: anagha <anagha1996@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants