Commit 1ef70d22 authored by Diegodlh's avatar Diegodlh
Browse files

make revision parser ignore template definitions with missing mandatory fields

parent 467f9c26
......@@ -269,6 +269,9 @@ it("multiple templates for the same path are silently ignored", () => {
describe("Configuration revisions", () => {
const warnSpy = jest.spyOn(log, "warn").mockImplementation();
beforeEach(() => {
jest.clearAllMocks();
});
it("skips misformatted elements individually", () => {
const content = JSON.stringify([
{
......@@ -360,4 +363,31 @@ describe("Configuration revisions", () => {
},
]);
});
it("skips templates with missing mandatory fields", () => {
const definitions: TemplateDefinition[] = [
{
path: "/",
fields: [
{
fieldname: "itemType",
required: true,
procedures: [],
},
],
},
];
const revision: ContentRevision = {
revid: 0,
timestamp: "",
content: JSON.stringify(definitions),
};
const configuration = new TemplateConfiguration(
"example.com",
["itemType", "title"],
undefined
);
configuration.loadRevision(revision);
expect(warnSpy).toHaveBeenCalledTimes(1);
expect(configuration.get().length).toBe(0);
});
});
......@@ -137,17 +137,18 @@ export class TemplateConfiguration extends DomainConfiguration<
// and convert back to json
try {
const template = new TranslationTemplate(this.domain, definition, {
forceRequiredFields: this.mandatoryFields,
strict: false,
});
templateDefinitions.push(template.toJSON());
} catch {
} catch (error) {
let info = "Ignoring misformatted template";
if ("path" in definition) {
info = info + ` for path "${definition.path}"`;
} else {
info = info + ` at index ${index}`;
}
log.info(info);
log.warn(info + `: ${error}`);
}
return templateDefinitions;
},
......@@ -157,15 +158,9 @@ export class TemplateConfiguration extends DomainConfiguration<
}
loadConfiguration(templates: TemplateDefinition[]): void {
if (this.mandatoryFields === undefined) {
throw new Error(
"Mandatory template fields must be defined before loading any template configuration"
);
}
// fixme?: wiping previous translation templates erases template caches
// see T302239
this.templates = [];
// silently ignore duplicate
templates.forEach((definition) => {
try {
this.add(definition);
......
......@@ -35,9 +35,7 @@ export abstract class BaseTranslationTemplate {
const fieldnames = template.fields.map((field) => field.fieldname);
for (const field of forceRequiredFields) {
if (!fieldnames.includes(field)) {
throw new Error(
`Mandatory field "${field}" missing from template definition`
);
throw new MissingFieldError(field);
}
}
}
......@@ -189,3 +187,10 @@ class DuplicateFieldError extends Error {
this.name = "Duplicate field error";
}
}
class MissingFieldError extends Error {
constructor(fieldname: string) {
super(`Mandatory field "${fieldname}" missing from template definition`);
this.name = "MissingFieldError";
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment