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

Localized Collection displays default locale content (even though localized content folder exists) #886

Closed
mc72 opened this Issue Aug 24, 2016 · 11 comments

Comments

Projects
None yet
9 participants
@mc72

mc72 commented Aug 24, 2016

Steps to reproduce

  1. Create a new collection entry/create multilingual version of that entry
  2. Save the entry
  3. Clear the cache

Expected behaviour

Localized entry should display on the proper locale content in both front end and cp

Actual behaviour

After clearing the cache, reverts back to the base language entry content on both the front end and backend (even though the localized content folder exists).

Clicking locale button in cp shows that it doesn't see a localized version of the content

This appears to only be happening for collections (not pages, which translate just fine).

Web server:
Mamp Pro locally (but also happening on our staging server)

PHP version:
7.0.8 locally,
7.0.10 on staging server

Statamic version:
2.1.3 (happened in 2.1.2 as well)
Above root

Updated from an older Statamic or fresh install:
Updated

List of installed addons:
Many

Logs

Statamic error log (/local/storage/logs/statamic.log)

[2016-08-24 16:32:50] dev.NOTICE: Content::collectionNames() is deprecated. Use Collection::handles()
[2016-08-24 16:32:50] dev.NOTICE: Assets::getContainers() is deprecated. Use AssetContainer::all()
[2016-08-24 16:32:50] dev.NOTICE: Content::entries() is deprecated. Use Entry::all()
[2016-08-24 16:32:50] dev.NOTICE: Content::pages() is deprecated. Use Page::all()
[2016-08-24 16:32:50] dev.NOTICE: Content::globals() is deprecated. Use GlobalSet::all()
[2016-08-24 16:32:50] dev.NOTICE: Content::taxonomyNames() is deprecated. Use Taxonomy::handles()
[2016-08-24 16:32:50] dev.NOTICE: Roles::slug() is deprecated. Use Role::whereHandle()
[2016-08-24 16:32:50] dev.NOTICE: Roles::slug() is deprecated. Use Role::whereHandle()
[2016-08-24 16:35:47] dev.NOTICE: Content::collectionNames() is deprecated. Use Collection::handles()
[2016-08-24 16:35:47] dev.NOTICE: Assets::getContainers() is deprecated. Use AssetContainer::all()
[2016-08-24 16:35:47] dev.NOTICE: Content::entries() is deprecated. Use Entry::all()
[2016-08-24 16:35:47] dev.NOTICE: Content::pages() is deprecated. Use Page::all()
[2016-08-24 16:35:47] dev.NOTICE: Content::globals() is deprecated. Use GlobalSet::all()
[2016-08-24 16:35:47] dev.NOTICE: Content::taxonomyNames() is deprecated. Use Taxonomy::handles()
[2016-08-24 16:35:47] dev.NOTICE: Roles::slug() is deprecated. Use Role::whereHandle()
[2016-08-24 16:35:47] dev.NOTICE: Roles::slug() is deprecated. Use Role::whereHandle()
[2016-08-24 16:35:56] dev.NOTICE: Content::collectionNames() is deprecated. Use Collection::handles()
[2016-08-24 16:35:56] dev.NOTICE: Assets::getContainers() is deprecated. Use AssetContainer::all()
[2016-08-24 16:35:56] dev.NOTICE: Content::entries() is deprecated. Use Entry::all()
[2016-08-24 16:35:56] dev.NOTICE: Content::pages() is deprecated. Use Page::all()
[2016-08-24 16:35:56] dev.NOTICE: Content::globals() is deprecated. Use GlobalSet::all()
[2016-08-24 16:35:56] dev.NOTICE: Content::taxonomyNames() is deprecated. Use Taxonomy::handles()
[2016-08-24 16:35:56] dev.NOTICE: Roles::slug() is deprecated. Use Role::whereHandle()
[2016-08-24 16:35:56] dev.NOTICE: Roles::slug() is deprecated. Use Role::whereHandle()

collection-localize

@mc72

This comment has been minimized.

mc72 commented Aug 24, 2016

I believe this may be site specific, but I just can't understand why it would effect both the front end and back end and why it's ignoring the localized content directory that exists.

@dddom

This comment has been minimized.

dddom commented Sep 6, 2016

I think I've run into this exact issue today, but for me it wouldn't even localise the collection items at all. Any ideas?

@jasonvarga jasonvarga added the bug label Sep 16, 2016

@baybara-pavel

This comment has been minimized.

baybara-pavel commented Dec 1, 2016

This problem occurs when exist more than one collection or taxonomy. It`s really breaks all my workflow

@subpixelch

This comment has been minimized.

subpixelch commented Dec 2, 2016

@jackmcdade @jasonvarga
This one is: Affecting production. Human sacrifice, docs and cats living together. Mass hysteria. 😰
Bug is still in v2.1.18 -> Please fix it ASAP. This one is really critical for anyone who has multi language sites!

@rrelmy

This comment has been minimized.

rrelmy commented Dec 2, 2016

I tracked down the bug to the method Statamic\Stache\Updater::updateLocaleAggregate.

Inside the loop over the repositories the $modified and $deleted variables will be overwritten by the filtered list. Later repos never receive the full list of files.

The fix is pretty easy, just rename the variables inside the foreach loop.

@jasonvarga @jackmcdade @jaggy Please, fix this quickly as possible. We really need multilanguage support!

    /**
     * @param string $locale
     * @param AggregateRepository $repo
     * @param Collection $modified
     * @param Collection $deleted
     */
    private function updateLocaleAggregate($locale, $repo, $modified, $deleted)
    {
        foreach ($repo->repos() as $repo) {
            // Using map because filter doesnt pass keys in Laravel 5.1
            $modifiedFiles = $modified->map(function ($contents, $path) use ($repo) {
                $key = $this->driver->getKeyFromPath($path);
                return ($key === $repo->key()) ? $contents : false;
            })->filter();

            // Using map because filter doesnt pass keys in Laravel 5.1
            $deletedFiles = $deleted->map(function ($contents, $path) use ($repo) {
                $key = $this->driver->getKeyFromPath($path);
                return ($key === $repo->key()) ? $contents : false;
            })->filter();

            $this->updateLocale($locale, $repo, $modifiedFiles, $deletedFiles);
        }
    }
@jasonvarga

This comment has been minimized.

Member

jasonvarga commented Dec 2, 2016

Thanks Remy, we'll check this out. Much appreciated.

@phpneil

This comment has been minimized.

phpneil commented Dec 8, 2016

@baybara-pavel's observation was insightful - although it's not just having more than one collection that is the problem, as it's only the first (alphabetically speaking) collection that will be localised - all others are overlooked.

This was very much a thing for me too, and @rrelmy's fix above worked just fine. Thank you Rémy!

@peda

This comment has been minimized.

peda commented Dec 16, 2016

@rrelmy thanks that solved it :-) just wasted a few hours looking for that bug - next time I'll google first

@peda

This comment has been minimized.

peda commented Dec 16, 2016

@jasonvarga do you think we can get that fixed in 2.2 as well ?

@peda

This comment has been minimized.

peda commented Dec 16, 2016

@rrelmy
there is another bug in this section which isn't visible until #1164 is fixed

Modifies are stored in array like "path" => "content" but deletes are stored in an indexed array of the format 0 => "path"

    /**
     * @param string $locale
     * @param AggregateRepository $repo
     * @param Collection $modified
     * @param Collection $deleted
     */
    private function updateLocaleAggregate($locale, $repo, $modified, $deleted)
    {
        foreach ($repo->repos() as $repo) {
            // Using map because filter doesnt pass keys in Laravel 5.1
            $modifiedFiles = $modified->map(function ($contents, $path) use ($repo) {
                $key = $this->driver->getKeyFromPath($path);
                return ($key === $repo->key()) ? $contents : false;
            })->filter();

            // Using map because filter doesnt pass keys in Laravel 5.1
            $deletedFiles = $deleted->map(function ($path, $idx) use ($repo) {
                $key = $this->driver->getKeyFromPath($path);
                return ($key === $repo->key()) ? $contents : false;
            })->filter();

            $this->updateLocale($locale, $repo, $modifiedFiles, $deletedFiles);
        }
    }
@rrelmy

This comment has been minimized.

rrelmy commented Dec 21, 2016

The fixed code would look like

    private function updateLocaleAggregate($locale, $repo, $modified, $deleted)
    {
        foreach ($repo->repos() as $repo) {
            // Using map because filter doesnt pass keys in Laravel 5.1
            $modifiedFiles = $modified->map(function ($contents, $path) use ($repo) {
                $key = $this->driver->getKeyFromPath($path);
                return ($key === $repo->key()) ? $contents : false;
            })->filter();

            $deletedFiles = $deleted->filter(function ($path) use ($repo) {
                return $this->driver->getKeyFromPath($path) === $repo->key();
            });

            $this->updateLocale($locale, $repo, $modifiedFiles, $deletedFiles);
        }
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment