Locale module uses an array to store data of translatable projects and available translations during its translation update batch processes. Also different (helper) function are used to process the project related data. Readability and maintainability of the code could be improved by combining functions and data storage in a TranslatableProject class and by using a typed class to store the translation source file data. This was suggested by Berdir in #1804688-14: Download and import interface translations. This gives the opportunity to simplify the way the $source data is handled within the batch operations.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sutharsan’s picture

Assigned: Unassigned » Sutharsan
Issue tags: +sprint

Tagging for extra attention ;)

Gábor Hojtsy’s picture

Issue tags: +language-ui

Should this not be based on Update module representing projects with a class instance first and foremost?

Gábor Hojtsy’s picture

Status: Active » Postponed
Issue tags: -sprint

I think this should be postponed on introducing project classes themselves, no? That is #1832946: Runtime translation download fallback works different from installer translation download fallback.

Sutharsan’s picture

Agree

Sutharsan’s picture

Status: Postponed » Needs review
FileSize
44.15 KB

Sharing my ideas of how to improve the readability and maintainability of the code by aggregating the project translation state and helper functions into one class. The patch is build on top of #1998056: Automatically update interface translations using cron #25 and therefore quires that patch to be applied first.

Status: Needs review » Needs work

The last submitted patch, locale-project-translation-object-1842380-5.patch, failed testing.

jair’s picture

Issue tags: +Needs reroll
jair’s picture

Issue summary: View changes

Updated issue summary.

aks22’s picture

Assigned: Sutharsan » aks22
Issue summary: View changes
aks22’s picture

The patch file which i was applying is not there.
File path:
/core/modules/locale/lib/Drupal/locale/ProjectTranslationState.php
also checked on git.

aks22’s picture

The patch file is for core/modules/locale/lib/Drupal/locale/ProjectTranslationState.php
which does not exist in core module.

Sutharsan’s picture

The patch needs to be rerolled first. See https://drupal.org/patch/reroll‎ for instructions.

akozma’s picture

FileSize
44.49 KB

Re-roll.

akozma’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 12: locale-project-translation-object-1842380-12.patch, failed testing.

akozma’s picture

Status: Needs work » Needs review
FileSize
44.51 KB

re-roll

Status: Needs review » Needs work

The last submitted patch, 15: locale-project-translation-object-1842380-15.patch, failed testing.

Sutharsan’s picture

Unassigning @akshay.swnt2. If you are still woking of the issue, please tell us here.

skipyT’s picture

After discussing with Sutharsan I'm proposing the next solution:

- we wait until #1842362: Replace locale_project table and improve caching will be committed
- we rewrite the interface for the project storage to return TranslatableProject instances
- we rewrite the interface for project storage to save TranslatableProjects inside the key value store. We can create a toArray() method on the translatable projects class.
- we'll store the translation status for each project inside the TranslatableProject data, this means it will be saved with the other project data in the key value store into the locale.project collection.

I propose also the following class names:
- project storage: LocaleProjectStorage
- translatable project class: LocaleTranslatableProject or just TranslatableProject

Sutharsan’s picture

Assigned: aks22 » Unassigned

#1842362: Replace locale_project table and improve caching was committed. Lets get this one rolling again ;)
Unassigning akshay.swnt22. This is not personal, but is has been a long time and I like to clear the path for anyone (incl. myself) to pickup the issue.

skipyT’s picture

@Sutharsan: I could help also. you can contact me on IRC to discuss about this. my nickname is: skipyT

Sutharsan’s picture

Assigned: Unassigned » Sutharsan

Working on rerolling the patch. Major changes in the status form...

Sutharsan’s picture

Rerolled the #15 patch, but did not make changes to TranslationStatusForm.php. The code of the form changed significantly and I doubt that the status form changes in #15 will survive. Because I don't want to spent a lot of time re-rolling code that will not be reused, I dropped it in this re-roll.

Sutharsan’s picture

Status: Needs work » Needs review
FileSize
73.13 KB
69.95 KB

This is the implementation of the ideas described in #18. Code is partly working, test not yet tested. The interdiff is perhaps too big to be useful.

Status: Needs review » Needs work

The last submitted patch, 23: locale-project-translation-object-1842380-23.patch, failed testing.

Sutharsan’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
79.27 KB
26.53 KB

More working code. Completed interface documentation. Can't get the tests working locally (even on 8.0.x, without patch), so no warranties on tests.

Status: Needs review » Needs work

The last submitted patch, 25: locale-project-translation-object-1842380-25.patch, failed testing.

Sutharsan’s picture

Assigned: Sutharsan » Unassigned
FileSize
86.57 KB
28.81 KB

Made test working. Some LocaleTranslationProject methods renamed.

Sutharsan’s picture

Status: Needs work » Needs review

Go botty

Status: Needs review » Needs work

The last submitted patch, 27: locale-project-translation-object-1842380-27.patch, failed testing.

Sutharsan’s picture

Status: Needs work » Needs review
FileSize
87.63 KB
1.21 KB
skipyT’s picture

Hi,

I read your patch and I have some ideas how could we improve the patch. But I need some time to reflect on it.

My concern is with that source object returned as a stdclass. Perhaps we should somehow split this class into 2 separate classes and try to have classes doing only one thing.

Also some methods are not unit testable, like the update method. I will try to push a patch until tomorrow morning to propose a new, cleaner class hierarchy here. I wanted to do it for yesterday, but it took me a while understanding the whole code.

Sutharsan’s picture

Last week I discussed in IRC with SkipyT some possible improvements:

  • To replace the stdClass source objects: Create an interface for translation sources and two implementations. One for the local and one for the remote translation source.
  • Instead of using
    $project->setLangcode($langcode);
    $project->getRemoteSource()

    use $project->getSourcebyLangcode($langcode)->getRemoteSource();

skipyT’s picture

FileSize
90.45 KB

I tried to rewrite the LocaleTranslatableProject to a new class hierarchy. Now I have:
- LocaleTranslatableProject
- ProjectState
- AbstractProjectSource
- LocalProjectSource
- RemoteProjectSource

This is a work in progress, I uploaded it only to have an early review. I will continue to work on this. I need to create the class interfaces, to clean the code, to have the right comments.

Status: Needs review » Needs work

The last submitted patch, 34: locale.patch, failed testing.

skipyT’s picture

Status: Needs work » Needs review
FileSize
92.33 KB
40.1 KB

I continued the work on the patch from #34.

Status: Needs review » Needs work

The last submitted patch, 36: locale-project-translation-object-1842380-36.patch, failed testing.

skipyT’s picture

rerolled the patch.

skipyT’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 38: locale-project-translation-object-1842380-38.patch, failed testing.

Sutharsan’s picture

I've discussed at DC Amsterdam, with skipyT how to proceed.

  • Keep the stateByLanguage array, it is convenient for data storage/retrieval.
  • Use a typed class(es) for ProjectSource. This will make the code more future proof, will document the properties and it is an obvious candidate for encapsulation.

Some code examples as we would like to retrieve the source data.

$translationProject->getName();
$translationProject->getVersion();
$translationProject->getStatusByLanguage('nl')->getRemote()->getTimestamp();
$translationProject->getStatusByLanguage('nl')->setTimestamp($now);
$translationProject->getStatusByLanguage('nl')->isUpToDate();
$translationProject->getStatusByLanguage('nl')->getLatest()->getTimestamp();
$translationProject->getStatusByLanguage('nl')->clear();
$translationProject->getStatusByLanguage('nl')->getRemote()->create($data);
$translationProject->getStatusByLanguage('nl')->hasUpdates();

$translationProject // TranslatableProject
$translationProject->getStatusByLanguage('nl') // Returns TranslatableProject
$translationProject->getStatusByLanguage('nl')->getRemote() // Returns RemoteProjectSource or ProjectSource
Sutharsan’s picture

Assigned: Unassigned » Sutharsan

Working ...

Sutharsan’s picture

For the record, #38 rerolled.

Sutharsan’s picture

Assigned: Sutharsan » Unassigned
Status: Needs work » Needs review
FileSize
87.65 KB
100.99 KB

This patch is partially working. The OO architecture is complete enough for a review. I'm not sure if I have the time to continue working on it the following days, therefore I share what I have now.

Summary of changes:

  • Renamed LocaleTranslatableProject(Interface) to TranslatableProject(Interface)
  • Using TranslatableProject::getStateByLanguage() and TranslatableProject::getState() to access translation state objects.
  • Removed LocaleProjectTranslationState classes.
  • Reworked ProjectSource classes:
    • - ProjectSourceInterface
    • - ProjectSourceBase (formerly AbstractProjectSource)
    • - LocalProjectSource
    • - RemoteProjectSource
  • Using \Serializable to (un)serialize project and source classes.

Still to do:

  • Get all existing tests working
  • Add unit tests

Status: Needs review » Needs work

The last submitted patch, 44: locale-project-translation-object-1842380-44.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

nterbogt’s picture

Is this something that is still on the cards? I'm having all sorts of performance issues with this specific part of locale.
I have 157 custom languages, only 7 of which exist on the Drupal translation servers.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.