Skip to content
This repository has been archived by the owner on Jul 7, 2022. It is now read-only.

Migration shell for migration from core 'Translate' to 'ShadowTranslate' behaviour #26

Closed
wants to merge 2 commits into from
Closed

Migration shell for migration from core 'Translate' to 'ShadowTranslate' behaviour #26

wants to merge 2 commits into from

Conversation

Spriz
Copy link

@Spriz Spriz commented Nov 6, 2015

According to #24 and https://github.com/AD7six/cakephp-shadow-translate#roadmap 馃槃

TODO

  • Make it work with other table names
  • ?

@Spriz
Copy link
Author

Spriz commented Nov 6, 2015

Please just poke with whatever should be changed 馃槃

@Spriz
Copy link
Author

Spriz commented Nov 11, 2015

poke poke @AD7six

$this->hr(1);
}
}
} catch (\Exception $e) {
Copy link
Owner

Choose a reason for hiding this comment

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

Nested try catch is a bit weird - what's the reason for that?

@AD7six
Copy link
Owner

AD7six commented Nov 14, 2015

Looks like it works but I'm not so keen on using the App model classes to do the inserts/updates. It's going to be susceptible to validation problems, and it's updating the same records once per translated field that way - that's potentially very inefficient.

I'd much rather aim for a solution ala:

INSERT INTO post_translations (
    SELECT
        posts.id,
        'xx', -- run once per language
        posts_title_translations.content,
        posts_body_translations.content
    FROM
        posts
    LEFT JOIN
        i18n as posts_title_translations ON (
            posts_title_translations.locale = "xx" AND
            posts_title_translations.model = "Posts" AND
            posts_title_translations.foreign_key = posts.id AND
            posts_title_translations.field = 'title'
    )
    LEFT JOIN
        i18n as posts_body_translations ON (
            posts_body_translations.locale = "xx" AND
            posts_body_translations.model = "Posts" AND
            posts_body_translations.foreign_key = posts.id AND
            posts_body_translations.field = 'body'
    )
    etc.
);

-- account for having created empty translations
delete from post_translations where title is null and body is null

The above sql is how I'd do it without a shell and is likely one or two orders of magnitude faster than the current shell.

I'm flexible, but a solution needs to work in principle with any amount of data - not just trivial amounts, so
I'd recommend to structure the shell as migrating a table (like posts) not everything in the i18n table in one go. That also permits structuring the shell as:

    $this->process(cli argument);
    ...
}

public function process($translationTableName) {
    $translationTable = TableRegistry::get($translationTableName);
    $params = ['limit' => 100, ['conditions' => ['model' => $translationTableName]];
    while ($i18nRows = $i18nTable->find($params)) {
        foreach($i18nRows as $row) {
            .... construct a post translation record.
            $translationTable->save($record);
        }
        $params['conditions']['id > '] = $row->id;
    }
}

Are you game to make those changes?

@Spriz
Copy link
Author

Spriz commented Nov 16, 2015

Mmh, I'm not sure if we "should" meet any validation in the models, but this could be fixed if I'd just change the language of the Table and then just patchEntity() - right? (http://book.cakephp.org/3.0/en/orm/behaviors/translate.html#saving-in-another-language)

I agree that it's about as inefficient/stupid as it can be, and that it could be optimised a lot in terms fo speed using a structure somewhat like what you wrote - but this is generic and works (as far as I can see 馃槢) for all cases with running just one single script - that's what I prioritised when doing it.

Also I'm co owner of a small startup, and we're really busy atm trying to raise interest from investors - so there could be quite some time before I'd have time to get back to this. We already used this to migrate all our DB translations - although #25 does keep us from using ShadowTranslate in production yet 馃樋

How about having both solutions?

@AD7six
Copy link
Owner

AD7six commented Nov 17, 2015

How about having both solutions?

I don't know what two solutions you mean..? Writing two implementations doesn't make sense to me.

it could be optimised

I'm actually more interested/concerned with being sure it will complete on a more sizeable amount of data than merely optimizing it. I don't want to add a cli which immediately becomes a maintenance problem for me because "it doesn't work" with a realistic but large number of translation records.

we're really busy atm

Me too =)

@Spriz
Copy link
Author

Spriz commented Nov 18, 2015

I don't know what two solutions you mean..? Writing two implementations doesn't make sense to me.

I mean both having a possibility to import all tables at once, and one at a time 馃槃

I'm actually more interested/concerned with being sure it will complete on a more sizeable amount of > >data than merely optimizing it. I don't want to add a cli which immediately becomes a maintenance >problem for me because "it doesn't work" with a realistic but large number of translation records.

Yeah, I see your point - I have no clue about how many translations people have in their DB's

I think I'll just let it stay for now, maybe someone else having the need for this feature will pick it up from here :-) At least they can use this solution for small solutions like ours (8k rows in i18n table) if they'd like to :)

@AD7six
Copy link
Owner

AD7six commented Jun 8, 2016

Closing for now, we can revisit in the future.

Thanks for the effort anyway :)

@AD7six AD7six closed this Jun 8, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants