Skip to content

MWPW-192450: Fix - Lingo placeholders in Studio card preview #760

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.

Sign up for GitHub

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

Merged
afmicka merged 11 commits into from
Apr 21, 2026
Merged

MWPW-192450: Fix - Lingo placeholders in Studio card preview #760

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
66cb6d1
MWPW-192450: Fix - Lingo placeholders in Studio card preview
seanchoi-dev Apr 13, 2026
3db7e42
MWPW-192450: Applying Axel's comment
seanchoi-dev Apr 14, 2026
5d0cf6a
Merge branch 'main' into MWPW-192450
honstar Apr 14, 2026
cf636c8
Merge branch 'main' into MWPW-192450
mirafedas Apr 15, 2026
6da772f
Merge branch 'main' into MWPW-192450
seanchoi-dev Apr 16, 2026
93371e8
MWPW-192450: Taking Ilyas's suggestion for performance.
seanchoi-dev Apr 16, 2026
c0c5908
MWPW-192450: handling empty dictionary case.
seanchoi-dev Apr 16, 2026
f15ad35
Merge branch 'main' into MWPW-192450
seanchoi-dev Apr 16, 2026
d7abacf
Merge branch 'main' into MWPW-192450
afmicka Apr 17, 2026
b6d4999
Merge branch 'main' into MWPW-192450
3ch023 Apr 17, 2026
009e7f8
Merge branch 'main' into MWPW-192450
seanchoi-dev Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions studio/src/mas-fragment-editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,12 @@ export default class MasFragmentEditor extends LitElement {

if (isVariationAfterContext && !skipVariation) {
await this.#attachParentToCachedVariation(existingStore, fragmentPath);
const fragmentLocale = extractLocaleFromPath(fragmentPath);
if (fragmentLocale && fragmentLocale !== Store.localeOrRegion()) {
Store.search.set((prev) => ({ ...prev, region: fragmentLocale }));
Copy link
Copy Markdown
Member

@Axelcureno Axelcureno Apr 13, 2026

Choose a reason for hiding this comment

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

New cached-store locale reload path has no test coverage

These 6 new lines add locale-specific placeholder loading to #initializeFromCachedStore — this is the path Codecov flagged (4 missing lines, 71.42% patch coverage on this file).

The existing test at mas-fragment-editor.test.js:358 only exercises #initializeFromRepository (non-cached). A test for this path would look like:

it('reloads locale placeholders for cached variations when locale differs', async () => {
    const fragmentData = createFragmentData({ id: 'cached-var', locale: 'fr_FR', slug: 'var' });
    const fragment = new Fragment(fragmentData);
    const sourceStore = generateFragmentStore(fragment);
    Store.fragments.list.data.set([sourceStore]);
    Store.filters.value = { locale: 'en_US' };

    el.editorContextStore.isVariation.returns(true);
    sandbox.stub(el, 'resolveVariationParentFragment').resolves(null);
    Store.fragmentEditor.fragmentId.value = 'cached-var';

    await el.initFragment();

    expect(mockRepo.loadPreviewPlaceholders.calledOnce).to.be.true;
    expect(Store.search.get().region).to.equal('fr_FR');
});

await this.repository.loadPreviewPlaceholders();
existingStore.resolvePreviewFragment();
}
} else {
this.localeDefaultFragment = existingStore.parentFragment;
}
Expand Down Expand Up @@ -751,8 +757,6 @@ export default class MasFragmentEditor extends LitElement {
// Initializes editor state for fragments that are not yet present in list store cache.
async #initializeFromRepository(fragmentId) {
try {
// Start loading placeholders early
const placeholdersPromise = this.repository.loadPreviewPlaceholders().catch(() => null);
const fragmentData = await this.repository.aem.sites.cf.fragments.getById(fragmentId);
const fragment = new Fragment(fragmentData);

Expand All @@ -763,8 +767,14 @@ export default class MasFragmentEditor extends LitElement {
const parentFragment = await this.#resolveParentForFetchedVariation(fragmentId, fragment, isVariationAfterContext);
const isVariationForStore = isVariationAfterContext || !!parentFragment;

// Wait for placeholders before creating stores (needed for preview resolution)
await placeholdersPromise;
if (isVariationForStore) {
const fragmentLocale = extractLocaleFromPath(fragment.path);
if (fragmentLocale && fragmentLocale !== Store.localeOrRegion()) {
Store.search.set((prev) => ({ ...prev, region: fragmentLocale }));
}
}

await this.repository.loadPreviewPlaceholders();

const fragmentStore = generateFragmentStore(fragment, parentFragment);
// Only add to main list if not a variation (variations appear under parent's variations panel)
Expand All @@ -775,16 +785,6 @@ export default class MasFragmentEditor extends LitElement {
this.#activateEditorStore(fragmentStore);
this.dispatchFragmentLoaded();

// Handle locale-specific placeholder reload for variations
if (isVariationForStore) {
const fragmentLocale = extractLocaleFromPath(fragment.path);
if (fragmentLocale && fragmentLocale !== Store.localeOrRegion()) {
Store.search.set((prev) => ({ ...prev, region: fragmentLocale }));
await this.repository.loadPreviewPlaceholders();
fragmentStore.resolvePreviewFragment();
}
}

Store.editor.resetChanges();
this.updateTranslatedLocalesStore(isVariationForStore, fragment.path); // no need to await
this.#markInitReady();
Expand Down
14 changes: 8 additions & 6 deletions studio/src/mas-repository.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,9 @@ export class MasRepository extends LitElement {
async loadPreviewPlaceholders() {
if (!this.search.value.path) return;

const cacheKey = `${this.filters.value.locale}_${this.search.value.path}`;
const locale = Store.localeOrRegion();
const path = this.search.value.path;
const cacheKey = `${locale}_${path}`;

// Return cached result if available
if (this.dictionaryCache.has(cacheKey)) {
Expand All @@ -666,15 +668,15 @@ export class MasRepository extends LitElement {
this.#abortControllers.placeholders = new AbortController();

try {
const promise = this.fetchDictionary(this.#abortControllers.placeholders);
const promise = this.fetchDictionary(this.#abortControllers.placeholders, locale);
this.inflightDictionaryRequest = { promise, cacheKey };
const result = await promise;

// Verify cache key hasn't changed during fetch (prevents stale data)
const currentKey = `${this.filters.value.locale}_${this.search.value.path}`;
const currentKey = `${Store.localeOrRegion()}_${this.search.value.path}`;
if (currentKey === cacheKey) {
// If result is empty and locale isn't en_US, try fallback
if ((!result || Object.keys(result).length === 0) && this.filters.value.locale !== 'en_US') {
if ((!result || Object.keys(result).length === 0) && locale !== 'en_US') {
const fallbackContext = {
preview: {
url: 'https://odinpreview.corp.adobe.com/adobe/sites/cf/fragments',
Expand Down Expand Up @@ -702,12 +704,12 @@ export class MasRepository extends LitElement {
}
}

async fetchDictionary(abortController) {
async fetchDictionary(abortController, locale = Store.localeOrRegion()) {
const context = {
preview: {
url: 'https://odinpreview.corp.adobe.com/adobe/sites/cf/fragments',
},
locale: this.filters.value.locale,
locale,
surface: this.search.value.path,
networkConfig: {
mainTimeout: 15000,
Expand Down
58 changes: 54 additions & 4 deletions studio/test/mas-fragment-editor.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import '../src/mas-fragment-editor.js';
import MasFragmentEditor from '../src/mas-fragment-editor.js';
import Store from '../src/store.js';
import { Fragment } from '../src/aem/fragment.js';
import generateFragmentStore, { SourceFragmentStore } from '../src/reactivity/source-fragment-store.js';
import generateFragmentStore from '../src/reactivity/source-fragment-store.js';
import { PAGE_NAMES, CARD_MODEL_PATH, ODIN_PREVIEW_ORIGIN } from '../src/constants.js';
import router from '../src/router.js';
import Events from '../src/events.js';
Expand Down Expand Up @@ -239,6 +239,7 @@ describe('MasFragmentEditor', () => {
search: structuredClone(Store.search.get()),
filters: structuredClone(Store.filters.get()),
translatedLocales: Store.fragmentEditor.translatedLocales.get(),
placeholdersPreview: Store.placeholders.preview.get(),
};

Store.fragments.list.data.value = [];
Expand All @@ -248,6 +249,7 @@ describe('MasFragmentEditor', () => {
Store.search.value = {};
Store.filters.value = { locale: 'en_US' };
Store.fragmentEditor.translatedLocales.value = null;
Store.placeholders.preview.value = null;
});

afterEach(() => {
Expand All @@ -261,6 +263,7 @@ describe('MasFragmentEditor', () => {
Store.search.value = originalStoreState.search;
Store.filters.value = originalStoreState.filters;
Store.fragmentEditor.translatedLocales.value = originalStoreState.translatedLocales;
Store.placeholders.preview.value = originalStoreState.placeholdersPreview;
});

it('returns early when no fragment ID exists', async () => {
Expand Down Expand Up @@ -357,19 +360,66 @@ describe('MasFragmentEditor', () => {

it('reloads locale placeholders for variations when active locale differs', async () => {
const fragmentData = createFragmentData({ id: 'variation-id', locale: 'fr_FR', slug: 'variation' });
const resolvePreviewSpy = sandbox.spy(SourceFragmentStore.prototype, 'resolvePreviewFragment');

Store.filters.value = { locale: 'tr_TR' };
mockRepo.aem.sites.cf.fragments.getById.resolves(fragmentData);
el.editorContextStore.isVariation.returns(true);
sandbox.stub(el, 'resolveVariationParentFragment').resolves(null);
Store.fragmentEditor.fragmentId.value = 'variation-id';

mockRepo.loadPreviewPlaceholders.callsFake(async () => {
Store.placeholders.preview.set({ testDictionary: true });
});

await el.initFragment();

expect(mockRepo.loadPreviewPlaceholders.calledOnce).to.be.true;
Copy link
Copy Markdown
Member

@Axelcureno Axelcureno Apr 13, 2026

Choose a reason for hiding this comment

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

Weakened assertion — preview resolution no longer verified

The old test verified both loadPreviewPlaceholders call count AND resolvePreviewFragment being called. The new test only checks calledOnce and region.

In the new flow, preview resolution happens implicitly via PreviewFragmentStore's constructor subscription to Store.placeholders.preview. This works, but is no longer tested for the variation-locale-differs path.

Consider adding a lightweight assertion that Store.placeholders.preview was set (which confirms the dictionary loaded), even if you don't spy on resolvePreviewFragment directly.

expect(Store.search.get().region).to.equal('fr_FR');
expect(Store.placeholders.preview.get()).to.deep.equal({ testDictionary: true });
});

it('reloads locale placeholders for cached variation when locale differs from Store.localeOrRegion()', async () => {
const fragmentData = createFragmentData({ id: 'cached-var-id', locale: 'fr_FR', slug: 'variation' });
const parentData = createFragmentData({ id: 'parent-id', locale: 'en_US', slug: 'default' });
const sourceStore = generateFragmentStore(new Fragment(fragmentData), new Fragment(parentData));
sandbox.spy(sourceStore, 'resolvePreviewFragment');

mockRepo.loadPreviewPlaceholders.callsFake(async () => {
Store.placeholders.preview.set({ fromCachedVariation: true });
});

el.editorContextStore.isVariation.returns(true);
sandbox.stub(el, 'resolveVariationParentFragment').resolves(new Fragment(parentData));

Store.filters.value = { locale: 'tr_TR' };
Store.search.value = { path: 'sandbox' };
Store.fragments.list.data.value = [sourceStore];
Store.fragmentEditor.fragmentId.value = 'cached-var-id';

await el.initFragment();

expect(mockRepo.loadPreviewPlaceholders.callCount).to.equal(2);
expect(resolvePreviewSpy.calledOnce).to.be.true;
expect(mockRepo.loadPreviewPlaceholders.calledOnce).to.be.true;
expect(Store.search.get().region).to.equal('fr_FR');
expect(Store.placeholders.preview.get()).to.deep.equal({ fromCachedVariation: true });
expect(sourceStore.resolvePreviewFragment.calledOnce).to.be.true;
});

it('does not reload preview placeholders when cached variation locale matches Store.localeOrRegion()', async () => {
const fragmentData = createFragmentData({ id: 'cached-var-id', locale: 'fr_FR', slug: 'variation' });
const parentData = createFragmentData({ id: 'parent-id', locale: 'en_US', slug: 'default' });
const sourceStore = generateFragmentStore(new Fragment(fragmentData), new Fragment(parentData));

el.editorContextStore.isVariation.returns(true);
sandbox.stub(el, 'resolveVariationParentFragment').resolves(new Fragment(parentData));

Store.filters.value = { locale: 'fr_FR' };
Store.search.value = { path: 'sandbox' };
Store.fragments.list.data.value = [sourceStore];
Store.fragmentEditor.fragmentId.value = 'cached-var-id';

await el.initFragment();

expect(mockRepo.loadPreviewPlaceholders.called).to.be.false;
});

it('uses pending parent from create variation event when context is not ready', async () => {
Expand Down
38 changes: 38 additions & 0 deletions studio/test/mas-repository.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2304,4 +2304,42 @@ describe('MasRepository dictionary helpers', () => {
expect(fragmentDeletedEmitStub.calledOnceWith(fragment)).to.be.true;
});
});

describe('loadPreviewPlaceholders', () => {
let previousSearch;
let previousFilters;
let previousPreview;

beforeEach(() => {
previousSearch = structuredClone(Store.search.get());
previousFilters = structuredClone(Store.filters.get());
previousPreview = Store.placeholders.preview.get();
});

afterEach(() => {
Store.search.value = previousSearch;
Store.filters.value = previousFilters;
Store.placeholders.preview.value = previousPreview;
});

it('uses Store.localeOrRegion() for cache key and fetchDictionary locale', async () => {
const repository = createFullRepository();
repository.dictionaryCache.clear();

Store.search.value = { path: 'sandbox' };
Store.filters.value = { locale: 'en_US' };
Store.search.set((prev) => ({ ...prev, region: 'fr_FR' }));
// Unconnected repo: StoreController does not receive Store updates unless we sync.
repository.search.value = Store.search.get();

const fetchStub = sandbox.stub(repository, 'fetchDictionary').resolves({ dictKey: 'dictVal' });

await repository.loadPreviewPlaceholders();

expect(fetchStub.calledOnce).to.be.true;
expect(fetchStub.firstCall.args[1]).to.equal('fr_FR');
expect(repository.dictionaryCache.has('fr_FR_sandbox')).to.be.true;
expect(Store.placeholders.preview.get()).to.deep.equal({ dictKey: 'dictVal' });
});
});
});
Loading