ag-admin/OpenSlides
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix format motions hidden change discrepancy

Browse Source 
Fixes an issue where the "formatMotion" function was working on the wrong
array.
This resulted in unexpected results of both PDF and CSV export

Fixes as issue in the diff services that deleted replaced lines rather than overwriting them
This commit is contained in:
Sean Engelhardt 2019-10-11 14:11:45 +02:00
parent 4a8362deaf
commit 6b1c72d526
3 changed files with 55 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
client/src/app/core/repositories/motions/motion-repository.service.ts
View File

@ -600,43 +600,37 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
highlightLine highlightLine
); );
case ChangeRecoMode.Diff: case ChangeRecoMode.Diff:
let text = ''; const text = [];
const changesToShow = changes.filter(change => { const changesToShow = changes.filter(change => change.showInDiffView());
return change.showInDiffView();
}); for (let i = 0; i < changesToShow.length; i++) {
changesToShow.forEach((change: ViewUnifiedChange, idx: number) => { text.push(
if (idx === 0) { this.diff.extractMotionLineRange(
text += this.diff.extractMotionLineRange(
targetMotion.text, targetMotion.text,
{ {
from: 1, from: i === 0 ? 1 : changesToShow[i - 1].getLineTo(),
to: change.getLineFrom() to: changesToShow[i].getLineFrom()
}, },
true, true,
lineLength, lineLength,
highlightLine highlightLine
); )
} else if (changes[idx - 1].getLineTo() < change.getLineFrom()) { );
text += this.diff.extractMotionLineRange(
targetMotion.text, text.push(
{ this.diff.getChangeDiff(targetMotion.text, changesToShow[i], lineLength, highlightLine)
from: changes[idx - 1].getLineTo(), );
to: change.getLineFrom() }
},
true, text.push(
lineLength, this.diff.getTextRemainderAfterLastChange(
highlightLine targetMotion.text,
); changesToShow,
} lineLength,
text += this.diff.getChangeDiff(targetMotion.text, change, lineLength, highlightLine); highlightLine
}); )
text += this.diff.getTextRemainderAfterLastChange(
targetMotion.text,
changesToShow,
lineLength,
highlightLine
); );
return text; return text.join('');
case ChangeRecoMode.Final: case ChangeRecoMode.Final:
const appliedChanges: ViewUnifiedChange[] = changes.filter(change => change.showInFinalView()); const appliedChanges: ViewUnifiedChange[] = changes.filter(change => change.showInFinalView());
return this.diff.getTextWithChanges(targetMotion.text, appliedChanges, lineLength, highlightLine); return this.diff.getTextWithChanges(targetMotion.text, appliedChanges, lineLength, highlightLine);

65
client/src/app/core/ui-services/diff.service.ts
View File

@ -1333,7 +1333,6 @@ export class DiffService {
} }
const fragment = this.htmlToFragment(htmlIn); const fragment = this.htmlToFragment(htmlIn);
this.insertInternalLineMarkers(fragment); this.insertInternalLineMarkers(fragment);
if (toLine === null) { if (toLine === null) {
const internalLineMarkers = fragment.querySelectorAll('OS-LINEBREAK'), const internalLineMarkers = fragment.querySelectorAll('OS-LINEBREAK'),
@ -1363,6 +1362,7 @@ export class DiffService {
fromChildTraceAbs.shift(); fromChildTraceAbs.shift();
const previousHtml = this.serializePartialDomToChild(fragment, fromChildTraceAbs, false); const previousHtml = this.serializePartialDomToChild(fragment, fromChildTraceAbs, false);
toChildTraceAbs.shift(); toChildTraceAbs.shift();
const followingHtml = this.serializePartialDomFromChild(fragment, toChildTraceAbs, false); const followingHtml = this.serializePartialDomFromChild(fragment, toChildTraceAbs, false);
let currNode: Node = fromLineNode, let currNode: Node = fromLineNode,
@ -1552,54 +1552,38 @@ export class DiffService {
* @returns {Node[]} * @returns {Node[]}
*/ */
public replaceLinesMergeNodeArrays(nodes1: Node[], nodes2: Node[]): Node[] { public replaceLinesMergeNodeArrays(nodes1: Node[], nodes2: Node[]): Node[] {
if (nodes1.length === 0) { if (nodes1.length === 0 || nodes2.length === 0) {
return nodes2; return nodes1.length ? nodes1 : nodes2;
}
if (nodes2.length === 0) {
return nodes1;
} }
const out = []; const out: Node[] = nodes1.slice(0, -1);
for (let i = 0; i < nodes1.length - 1; i++) { const lastNode: Node = nodes1[nodes1.length - 1];
out.push(nodes1[i]); const firstNode: Node = nodes2[0];
}
const lastNode = nodes1[nodes1.length - 1],
firstNode = nodes2[0];
if (lastNode.nodeType === TEXT_NODE && firstNode.nodeType === TEXT_NODE) { if (lastNode.nodeType === TEXT_NODE && firstNode.nodeType === TEXT_NODE) {
const newTextNode = lastNode.ownerDocument.createTextNode(lastNode.nodeValue + firstNode.nodeValue); const newTextNode: Text = lastNode.ownerDocument.createTextNode(lastNode.nodeValue + firstNode.nodeValue);
out.push(newTextNode); out.push(newTextNode);
} else if (lastNode.nodeName === firstNode.nodeName) { } else if (lastNode.nodeName === firstNode.nodeName) {
const lastElement = <Element>lastNode, const lastElement: Element = lastNode as Element;
newNode = lastNode.ownerDocument.createElement(lastNode.nodeName); const newNode: HTMLElement = lastNode.ownerDocument.createElement(lastNode.nodeName);
for (let i = 0; i < lastElement.attributes.length; i++) {
const attr = lastElement.attributes[i]; for (const attr of Array.from(lastElement.attributes)) {
newNode.setAttribute(attr.name, attr.value); newNode.setAttribute(attr.name, attr.value);
} }
// Remove #text nodes inside of List elements (OL/UL), as they are confusing // Remove #text nodes inside of List elements (OL/UL), as they are confusing
let lastChildren, firstChildren; let lastChildren: Node[];
let firstChildren: Node[];
if (lastElement.nodeName === 'OL' || lastElement.nodeName === 'UL') { if (lastElement.nodeName === 'OL' || lastElement.nodeName === 'UL') {
lastChildren = []; lastChildren = Array.from(lastElement.childNodes).filter(child => child.nodeType === ELEMENT_NODE);
firstChildren = []; firstChildren = Array.from(firstNode.childNodes).filter(child => child.nodeType === ELEMENT_NODE);
for (let i = 0; i < firstNode.childNodes.length; i++) {
if (firstNode.childNodes[i].nodeType === ELEMENT_NODE) {
firstChildren.push(firstNode.childNodes[i]);
}
}
for (let i = 0; i < lastElement.childNodes.length; i++) {
if (lastElement.childNodes[i].nodeType === ELEMENT_NODE) {
lastChildren.push(lastElement.childNodes[i]);
}
}
} else { } else {
lastChildren = lastElement.childNodes; lastChildren = Array.from(lastElement.childNodes);
firstChildren = firstNode.childNodes; firstChildren = Array.from(firstNode.childNodes);
} }
const children = this.replaceLinesMergeNodeArrays(lastChildren, firstChildren); const children = this.replaceLinesMergeNodeArrays(lastChildren, firstChildren) as Node[];
for (let i = 0; i < children.length; i++) { for (const child of children) {
newNode.appendChild(children[i]); newNode.appendChild(child);
} }
out.push(newNode); out.push(newNode);
@ -1612,11 +1596,7 @@ export class DiffService {
} }
} }
for (let i = 1; i < nodes2.length; i++) { return out.concat(nodes2.slice(1, nodes2.length));
out.push(nodes2[i]);
}
return out;
} }
/** /**
@ -1755,7 +1735,8 @@ export class DiffService {
this.removeCSSClass(forgottenSplitClasses[i], 'os-split-after'); this.removeCSSClass(forgottenSplitClasses[i], 'os-split-after');
} }
return this.serializeDom(mergedFragment, true); const replacedHtml = this.serializeDom(mergedFragment, true);
return replacedHtml;
} }
/** /**

9
client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.ts
View File

@ -854,7 +854,14 @@ export class MotionDetailComponent extends BaseViewComponent implements OnInit,
public getFormattedTextPlain(): string { public getFormattedTextPlain(): string {
// Prevent this.allChangingObjects to be reordered from within formatMotion // Prevent this.allChangingObjects to be reordered from within formatMotion
const changes: ViewUnifiedChange[] = Object.assign([], this.getAllTextChangingObjects()); const changes: ViewUnifiedChange[] = Object.assign([], this.getAllTextChangingObjects());
return this.repo.formatMotion(this.motion.id, this.crMode, changes, this.lineLength, this.highlightedLine); const formatedText = this.repo.formatMotion(
this.motion.id,
this.crMode,
changes,
this.lineLength,
this.highlightedLine
);
return formatedText;
} }
/** /**