Commit d91ed3f7 authored by Diegodlh's avatar Diegodlh
Browse files

Merge branch 'field-output' into dev

parents a5e99726 4593a6ad
import { Webpage } from "../webpage/webpage";
import { Domain } from "./domain";
describe("Domain class", () => {
......@@ -5,3 +6,72 @@ describe("Domain class", () => {
expect(new Domain("example.com").domain).toBe("example.com");
});
});
it("does not make citatations for non-applicable template outputs", async () => {
const domain = new Domain("example.com", {
templates: [
{
path: "/template2",
label: "non-applicable",
fields: [
{
fieldname: "itemType",
required: true,
procedures: [
{
selections: [],
transformations: [],
},
],
},
{
fieldname: "title",
required: true,
procedures: [
{
selections: [],
transformations: [],
},
],
},
],
},
{
path: "/template1",
label: "applicable",
fields: [
{
fieldname: "itemType",
required: true,
procedures: [
{
selections: [{ type: "fixed", config: "webpage" }],
transformations: [],
},
],
},
{
fieldname: "title",
required: true,
procedures: [
{
selections: [{ type: "fixed", config: "title" }],
transformations: [],
},
],
},
],
},
],
});
const target = new Webpage("https://example.com/target");
const translationOutput = await domain.translate(target, {
onlyApplicable: false,
});
const templateOutputs = translationOutput.translation.outputs;
expect(templateOutputs.length).toBe(2);
expect(templateOutputs[0].template.applicable).toBe(false);
expect(templateOutputs[0].citation).toBeUndefined();
expect(templateOutputs[1].template.applicable).toBe(true);
expect(templateOutputs[1].citation).toBeDefined();
});
......@@ -89,22 +89,16 @@ export class Domain {
}
// translate target with templates for paths returned above
let templateOutputs = await this.templates.translateWith(
const templateOutputs = await this.templates.translateWith(
target,
templatePaths,
{
tryAllTemplates: options.allTemplates,
useFallback: options.forceTemplatePaths === undefined,
onlyApplicable: options.onlyApplicable,
}
);
// parse output with onlyApplicable option
if (options.onlyApplicable) {
templateOutputs = templateOutputs.filter(
(templateOutput) => templateOutput.applicable
);
}
let baseCitation: MediaWikiBaseFieldCitation | undefined;
if (options.fillWithCitoid) {
// baseCitation = (await target.cache.citoid.getData()).citation
......@@ -164,17 +158,21 @@ export class Domain {
baseCitation?: MediaWikiBaseFieldCitation
): Translation {
// create citoid citations from output
const citation = this.makeCitation(
templateOutput.outputs,
// With this we are setting the output citation's URL to that of the
// target Webpage object, which does not follow redirects.
// We may change this to the final response URL, but there may be cases
// where we do not want to do that (see T210871).
// Alternatively, we may let users manually change this using a URL
// template field.
templateOutput.target.url.href,
baseCitation
);
let citation;
if (templateOutput.applicable) {
// only make citations for applicable template outputs
citation = this.makeCitation(
templateOutput.outputs,
// With this we are setting the output citation's URL to that of the
// target Webpage object, which does not follow redirects.
// We may change this to the final response URL, but there may be cases
// where we do not want to do that (see T210871).
// Alternatively, we may let users manually change this using a URL
// template field.
templateOutput.target.url.href,
baseCitation
);
}
let fields: FieldInfo[] | undefined;
if (templateFieldInfo) {
......@@ -216,6 +214,7 @@ export class Domain {
required: fieldOutput.required,
procedures,
output: fieldOutput.output,
valid: fieldOutput.valid,
applicable: fieldOutput.applicable,
};
return fieldInfo;
......@@ -309,7 +308,7 @@ type Translation = {
path: string | undefined; // undefined for fallback template
fields?: FieldInfo[];
};
citation: WebToCitCitation;
citation: WebToCitCitation | undefined;
timestamp: string;
};
......@@ -325,7 +324,8 @@ type FieldInfo = {
transformations: Array<TransformationDefinition & { output: StepOutput }>;
output: StepOutput;
}[];
output: Array<string | null>; // this is a validated output; no need to have separate valid property
output: StepOutput;
valid: boolean;
applicable: boolean;
};
......
......@@ -112,13 +112,22 @@ describe("Use an applicable template", () => {
expect(output.applicable).toBe(true);
});
it("skips non-applicable templates", async () => {
it("skips non-applicable templates by default", async () => {
const output = (
await configuration.translateWith(target, paths)
)[0] as TemplateOutput;
expect(output.template.label).toBe("second template");
});
it("optionally returns non-applicable template outputs", async () => {
const outputs = (await configuration.translateWith(target, paths, {
onlyApplicable: false,
})) as TemplateOutput[];
expect(outputs.length).toBe(2);
expect(outputs[0].applicable).toBe(false);
expect(outputs[1].applicable).toBe(true);
});
it("outputs the expected results", async () => {
const output = (
await configuration.translateWith(target, paths)
......
......@@ -178,7 +178,12 @@ export class TemplateConfiguration extends DomainConfiguration<
async translateWith(
target: Webpage,
paths: string[],
{ useFallback = true, preferSamePath = true, tryAllTemplates = false } = {}
{
useFallback = true,
preferSamePath = true,
tryAllTemplates = false,
onlyApplicable = true,
} = {}
): Promise<TemplateOutput[]> {
const templates: BaseTranslationTemplate[] = this.get(paths);
if (preferSamePath) {
......@@ -209,6 +214,8 @@ export class TemplateConfiguration extends DomainConfiguration<
if (output.applicable) {
outputs.push(output);
break;
} else if (!onlyApplicable) {
outputs.push(output);
}
}
}
......
......@@ -116,6 +116,23 @@ it("marks empty outputs as invalid", async () => {
expect(fieldOutput.valid).toBe(false);
});
it("always returns field output, even if invalid", async () => {
const field = new TemplateField({
fieldname: "itemType",
required: true,
procedures: [
{
selections: [{ type: "fixed", config: "invalidType" }],
transformations: [],
},
],
});
const target = new Webpage("https://example.com/target");
const output = await field.translate(target);
expect(output.output).toEqual(["invalidType"]);
expect(output.valid).toBe(false);
});
it("constructor optionally skips invalid procedure definitions", () => {
const warnSpy = jest.spyOn(log, "warn").mockImplementation();
const definition: unknown = {
......
......@@ -5,7 +5,6 @@ import { MediaWikiBaseFieldCitation, MediaWikiCreator } from "../citoid";
import {
TemplateFieldDefinition,
TemplateFieldOutput,
ProcedureOutput,
ProcedureDefinition,
} from "../types";
import log from "loglevel";
......@@ -77,16 +76,24 @@ export class TemplateField extends TranslationField {
const procedureOutputs = await Promise.all(
this.procedures.map((procedure) => procedure.translate(target))
);
const combinedOutput = ([] as string[]).concat(
let combinedOutput = ([] as string[]).concat(
...procedureOutputs.map((output) => output.output.procedure)
);
const output = this.validate(combinedOutput);
const valid = output.length > 0 && output.every((value) => value !== null);
// if field does not expect an array for output, join output values
if (!this.isArray && combinedOutput.length > 0) {
combinedOutput = [combinedOutput.join()];
}
combinedOutput = combinedOutput.map((value) => value.trim());
const valid = this.validate(combinedOutput);
const fieldOutput: TemplateFieldOutput = {
fieldname: this.name,
required: this.required,
procedureOutputs: procedureOutputs,
output,
output: combinedOutput,
valid,
applicable: valid || !this.required,
control: this.isControl,
......@@ -102,20 +109,10 @@ export class TemplateField extends TranslationField {
};
}
private validate(
output: ProcedureOutput["output"]["procedure"]
): Array<string | null> {
if (!this.isArray && output.length) {
// alternatively, consider:
// a. keep first element only
// b. return [null] (invalid)
output = [output.join()];
}
const validatedOutput = output.map((output) => {
output = output.trim();
return this.pattern.test(output) ? output : null;
});
return validatedOutput;
private validate(output: TemplateFieldOutput["output"]): boolean {
const valid =
output.length > 0 && output.every((value) => this.pattern.test(value));
return valid;
}
}
......@@ -140,11 +137,6 @@ export function outputToCitation(
// ignore invalid and control fields
if (field.valid && !field.control) {
const fieldName = field.fieldname;
// fixme: reconsider whether field output should accept null values
// see T302024
if (field.output.some((value) => value === null)) {
throw new Error(`Unexpected non-string value in valid field output`);
}
fields.set(fieldName, field.output as string[]);
}
}
......
......@@ -178,8 +178,8 @@ export type TemplateOutput = {
export type TemplateFieldOutput = {
fieldname: FieldName;
procedureOutputs: ProcedureOutput[];
output: Array<string | null>; // todo: change Array<string>, see T302024
valid: boolean; // todo: remove, see T302024
output: StepOutput;
valid: boolean;
required: boolean;
applicable: boolean; // valid || !required
control: boolean;
......
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