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.

CommentFileSizeAuthor
#44 locale-project-translation-object-1842380-44.patch100.99 KBsutharsan
#44 interdiff-1842380-43-44.txt87.65 KBsutharsan
#43 locale-project-translation-object-1842380-43.patch93.43 KBsutharsan
#38 interdiff.txt40.11 KBskipyT
#38 locale-project-translation-object-1842380-38.patch94.02 KBskipyT
#36 interdiff.txt40.1 KBskipyT
#36 locale-project-translation-object-1842380-36.patch92.33 KBskipyT
#34 locale.patch90.45 KBskipyT
#31 interdiff-1842380-27-31.txt1.21 KBsutharsan
#31 locale-project-translation-object-1842380-31.patch87.63 KBsutharsan
#27 interdiff-1842380-25-27.txt28.81 KBsutharsan
#27 locale-project-translation-object-1842380-27.patch86.57 KBsutharsan
#25 interdiff-1842380-23-25.txt26.53 KBsutharsan
#25 locale-project-translation-object-1842380-25.patch79.27 KBsutharsan
#23 interdiff-1842380-22-23.txt69.95 KBsutharsan
#23 locale-project-translation-object-1842380-23.patch73.13 KBsutharsan
#22 locale-project-translation-object-1842380-22.patch38.29 KBsutharsan
#15 locale-project-translation-object-1842380-15.patch44.51 KBakozma
#12 locale-project-translation-object-1842380-12.patch44.49 KBakozma
#5 locale-project-translation-object-1842380-5.patch44.15 KBsutharsan

Comments

sutharsan’s picture

sutharsan commented
Assigned: Unassigned » sutharsan
Issue tags: +sprint

Tagging for extra attention ;)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
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

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
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

sutharsan commented

Agree

sutharsan’s picture

sutharsan commented
Status: Postponed » Needs review
StatusFileSize
new locale-project-translation-object-1842380-5.patch44.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

jair commented
Issue tags: +Needs reroll
jair’s picture

jair commented
Issue summary: View changes

Updated issue summary.

aks22’s picture

aks22 commented
Assigned: sutharsan » aks22
Issue summary: View changes
aks22’s picture

aks22 commented

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

aks22 commented

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

sutharsan’s picture

sutharsan commented

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

akozma’s picture

akozma commented
StatusFileSize
new locale-project-translation-object-1842380-12.patch44.49 KB

Re-roll.

akozma’s picture

akozma commented
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

akozma commented
Status: Needs work » Needs review
StatusFileSize
new locale-project-translation-object-1842380-15.patch44.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

sutharsan commented

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

skipyT’s picture

skipyT commented

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

sutharsan commented
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

skipyT commented

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

sutharsan’s picture

sutharsan commented
Assigned: Unassigned » sutharsan

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

sutharsan’s picture

sutharsan commented
StatusFileSize
new locale-project-translation-object-1842380-22.patch38.29 KB

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

sutharsan commented
Status: Needs work » Needs review
StatusFileSize
new locale-project-translation-object-1842380-23.patch73.13 KB
new interdiff-1842380-22-23.txt69.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

sutharsan commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new locale-project-translation-object-1842380-25.patch79.27 KB
new interdiff-1842380-23-25.txt26.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

sutharsan commented
Assigned: sutharsan » Unassigned
StatusFileSize
new locale-project-translation-object-1842380-27.patch86.57 KB
new interdiff-1842380-25-27.txt28.81 KB

Made test working. Some LocaleTranslationProject methods renamed.

sutharsan’s picture

sutharsan commented
Status: Needs work » Needs review

Go botty

Sutharsan queued 27: locale-project-translation-object-1842380-27.patch for re-testing.

Status: Needs review » Needs work

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

sutharsan’s picture

sutharsan commented
Status: Needs work » Needs review
StatusFileSize
new locale-project-translation-object-1842380-31.patch87.63 KB
new interdiff-1842380-27-31.txt1.21 KB
skipyT’s picture

skipyT commented

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

sutharsan commented

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

skipyT commented
StatusFileSize
new locale.patch90.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

skipyT commented
Status: Needs work » Needs review
StatusFileSize
new locale-project-translation-object-1842380-36.patch92.33 KB
new interdiff.txt40.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

skipyT commented
StatusFileSize
new locale-project-translation-object-1842380-38.patch94.02 KB
new interdiff.txt40.11 KB

rerolled the patch.

skipyT’s picture

skipyT commented
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

sutharsan commented

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

sutharsan commented
Assigned: Unassigned » sutharsan

Working ...

sutharsan’s picture

sutharsan commented
Issue summary: View changes
StatusFileSize
new locale-project-translation-object-1842380-43.patch93.43 KB

For the record, #38 rerolled.

sutharsan’s picture

sutharsan commented
Assigned: sutharsan » Unassigned
Status: Needs work » Needs review
StatusFileSize
new interdiff-1842380-43-44.txt87.65 KB
new locale-project-translation-object-1842380-44.patch100.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

nterbogt commented

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.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Title: Convert $source object to a TranslatableProject class » Convert $source object to a LocaleTranslationSource class

12 years later: We're finally making progress on modernizing and cleaning up locale.module. Some existing issues and currently #3566536: [meta] eliminate core .module files are introducing a LocaleTranslatableProject class but after the most recent changes again only for $project and not $source, which is slightly different. Rescoping this.

@nterbogt: It's been many years since your comment, but #3586654: Avoid scanning the file system for local po files is a related recent issue that seems to significantly improve performance with many languages. There's also discussion on performance in #3037156: Modernize locale history functions.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Closed (duplicate)

#3590050: Deprecate and replace locale_status related functions introduces LocaleTranslationSource now and also cleans up the translation status functions. I think that's the last major step of #3215707: [META] Modernize Locale module, there's still a good amount of work left before we can close that meta, but after that, it's it's mostly just a bunch of relatively simple helper functions.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.