Skip to content

Commit

Permalink
Fix bug 1606302 - Show Approve button for Unreviewed FTL strings. (#1550
Browse files Browse the repository at this point in the history
)
  • Loading branch information
adngdb committed Jan 31, 2020
1 parent 55abec8 commit c5ce525
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 4 deletions.
27 changes: 23 additions & 4 deletions frontend/src/core/editor/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@

import { createSelector } from 'reselect';

import * as entities from 'core/entities';
import * as plural from 'core/plural';
import { fluent } from 'core/utils';
import * as history from 'modules/history';
import { NAME } from '.';

import type { EntityTranslation } from 'core/api';
import type { EntityTranslation, Entity } from 'core/api';
import type { HistoryTranslation, HistoryState } from 'modules/history';
import type { EditorState } from '.';

Expand All @@ -20,6 +21,7 @@ export function _existingTranslation(
editor: EditorState,
activeTranslation: EntityTranslation,
history: HistoryState,
entity: Entity,
): ?(EntityTranslation | HistoryTranslation) {
const { translation, initialTranslation } = editor;

Expand Down Expand Up @@ -60,9 +62,25 @@ export function _existingTranslation(
}
// If translation is a string, from the generic editor.
else {
existingTranslation = history.translations.find(
t => t.string === translation
)
// Except it might actually be a simple Fluent message from the SimpleEditor.
if (entity.format === 'ftl') {
// This case happens when the format is Fluent and the string is simple.
// We thus store a simplified version of the Fluent message in memory,
// but the history has actual Fluent-formatted strings (with ID).
// Thus, we need to reconstruct the Fluent message in order to be able
// to compare it with items in history.
const reconstructed = fluent.serializer.serializeEntry(
fluent.getReconstructedMessage(entity.original, translation)
);
existingTranslation = history.translations.find(
t => t.string === reconstructed
)
}
else {
existingTranslation = history.translations.find(
t => t.string === translation
)
}
}
}
return existingTranslation;
Expand All @@ -80,6 +98,7 @@ export const sameExistingTranslation: Function = createSelector(
editorSelector,
plural.selectors.getTranslationForSelectedEntity,
historySelector,
entities.selectors.getSelectedEntity,
_existingTranslation
);

Expand Down
30 changes: 30 additions & 0 deletions frontend/src/core/editor/selectors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ const EDITOR_FLUENT = {
initialTranslation: fluent.parser.parseEntry('msg = something'),
};

const ENTITY = {
format: 'po',
};

const ENTITY_FLUENT = {
original: 'msg = Allez Morty !',
format: 'ftl',
};

const ACTIVE_TRANSLATION = { pk: 1 };

const HISTORY = {
Expand Down Expand Up @@ -42,6 +51,10 @@ const HISTORY_FLUENT = {
pk: 98,
string: 'msg = hello, world!',
},
{
pk: 431,
string: 'msg = Come on Morty!\n',
},
],
};

Expand All @@ -52,6 +65,7 @@ describe('sameExistingTranslation', () => {
{ ...EDITOR, translation: EDITOR.initialTranslation },
ACTIVE_TRANSLATION,
HISTORY,
ENTITY,
)).toEqual(ACTIVE_TRANSLATION);
});

Expand All @@ -60,6 +74,7 @@ describe('sameExistingTranslation', () => {
{ ...EDITOR_FLUENT, translation: EDITOR_FLUENT.initialTranslation },
ACTIVE_TRANSLATION,
HISTORY_FLUENT,
ENTITY_FLUENT,
)).toEqual(ACTIVE_TRANSLATION);
});

Expand All @@ -68,6 +83,7 @@ describe('sameExistingTranslation', () => {
{ translation: '', initialTranslation: '' },
ACTIVE_TRANSLATION,
HISTORY,
ENTITY,
)).toEqual(ACTIVE_TRANSLATION);
});

Expand All @@ -76,12 +92,14 @@ describe('sameExistingTranslation', () => {
{ ...EDITOR, translation: HISTORY.translations[0].string },
ACTIVE_TRANSLATION,
HISTORY,
ENTITY,
)).toEqual(HISTORY.translations[0]);

expect(_existingTranslation(
{ ...EDITOR, translation: HISTORY.translations[1].string },
ACTIVE_TRANSLATION,
HISTORY,
ENTITY,
)).toEqual(HISTORY.translations[1]);
});

Expand All @@ -97,6 +115,7 @@ describe('sameExistingTranslation', () => {
},
ACTIVE_TRANSLATION,
HISTORY_FLUENT,
ENTITY_FLUENT,
)).toEqual(HISTORY_FLUENT.translations[0]);

expect(_existingTranslation(
Expand All @@ -110,6 +129,7 @@ describe('sameExistingTranslation', () => {
},
ACTIVE_TRANSLATION,
HISTORY_FLUENT,
ENTITY_FLUENT,
)).toEqual(HISTORY_FLUENT.translations[1]);
});

Expand All @@ -118,6 +138,16 @@ describe('sameExistingTranslation', () => {
{ ...EDITOR, translation: '' },
ACTIVE_TRANSLATION,
HISTORY,
ENTITY,
)).toEqual(HISTORY.translations[2]);
});

it('finds a Fluent translation in history from a simplified Fluent string', () => {
expect(_existingTranslation(
{ initialTranslation: '', translation: 'Come on Morty!' },
ACTIVE_TRANSLATION,
HISTORY_FLUENT,
ENTITY_FLUENT,
)).toEqual(HISTORY_FLUENT.translations[2]);
});
});

0 comments on commit c5ce525

Please sign in to comment.