Bugfix: better handling of inconsistent states in amendments if the base motion has shrunk

This commit is contained in:
Tobias Hößl 2021-02-28 18:38:51 +01:00 committed by Emanuel Schütze
parent f80ac1d9c5
commit ac50d6f8dc
7 changed files with 127 additions and 47 deletions

View File

@ -336,13 +336,18 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
const changeRecos = viewMotion.changeRecommendations.filter(changeReco => const changeRecos = viewMotion.changeRecommendations.filter(changeReco =>
changeReco.showInFinalView() changeReco.showInFinalView()
); );
return this.getAmendmentParagraphLines( try {
viewMotion, return this.getAmendmentParagraphLines(
this.motionLineLength, viewMotion,
ChangeRecoMode.Changed, this.motionLineLength,
changeRecos, ChangeRecoMode.Changed,
false changeRecos,
); false
);
} catch (e) {
// Inconsistency between motion and amendment -> the best we can do is not to fail completely
return [];
}
} else { } else {
return []; return [];
} }
@ -810,6 +815,13 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
let paragraph: string; let paragraph: string;
let paragraphHasChanges; let paragraphHasChanges;
if (baseParagraphs[paraNo] === undefined) {
const msg =
'Inconsistent data. An amendment is probably referring to a non-existant line number. ' +
'You can back up its content when editing it and delete it afterwards.';
return '<em style="color: red; font-weight: bold;">' + msg + '</em>';
}
if (newText === null) { if (newText === null) {
paragraph = baseParagraphs[paraNo]; paragraph = baseParagraphs[paraNo];
paragraphHasChanges = false; paragraphHasChanges = false;
@ -855,6 +867,7 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
* @param {ViewMotionChangeRecommendation[]} changeRecommendations * @param {ViewMotionChangeRecommendation[]} changeRecommendations
* @param {boolean} includeUnchanged * @param {boolean} includeUnchanged
* @returns {DiffLinesInParagraph} * @returns {DiffLinesInParagraph}
* @throws Error
*/ */
public getAmendmentParagraphLines( public getAmendmentParagraphLines(
amendment: ViewMotion, amendment: ViewMotion,
@ -876,7 +889,11 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
return amendmentParagraphs return amendmentParagraphs
?.map( ?.map(
(newText: string, paraNo: number): DiffLinesInParagraph => { (newText: string, paraNo: number): DiffLinesInParagraph => {
if (newText !== null) { if (baseParagraphs[paraNo] === undefined) {
throw new Error(
'Inconsistent data. An amendment is probably referring to a non-existant line number.'
);
} else if (newText !== null) {
return this.diff.getAmendmentParagraphsLines( return this.diff.getAmendmentParagraphsLines(
paraNo, paraNo,
baseParagraphs[paraNo], baseParagraphs[paraNo],
@ -950,6 +967,12 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
if (newText === null) { if (newText === null) {
return null; return null;
} }
if (baseParagraphs[paraNo] === undefined) {
console.error(
'Inconsistent data. An amendment is probably referring to a non-existant line number.'
);
return null;
}
const origText = baseParagraphs[paraNo], const origText = baseParagraphs[paraNo],
diff = this.diff.diff(origText, newText), diff = this.diff.diff(origText, newText),
@ -990,6 +1013,9 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
return (amendment.amendment_paragraphs || []).map((newText: string, paraNo: number): string => { return (amendment.amendment_paragraphs || []).map((newText: string, paraNo: number): string => {
const origText = baseParagraphs[paraNo]; const origText = baseParagraphs[paraNo];
if (origText === undefined) {
throw new Error('Inconsistent data. An amendment is probably referring to a non-existant line number.');
}
if (newText === null) { if (newText === null) {
return origText; return origText;

View File

@ -2238,7 +2238,8 @@ export class DiffService {
// That's a pretty serious inconsistency that should not happen at all, // That's a pretty serious inconsistency that should not happen at all,
// we're just doing some basic damage control here. // we're just doing some basic damage control here.
const msg = const msg =
'Inconsistent data. A change recommendation is probably referring to a non-existant line number.'; 'Inconsistent data. A change recommendation or amendment is probably referring to a non-existant line number. ' +
'If it is an amendment, you can back up its content when editing it and delete it afterwards';
return '<em style="color: red; font-weight: bold;">' + msg + '</em>'; return '<em style="color: red; font-weight: bold;">' + msg + '</em>';
} }
@ -2299,7 +2300,7 @@ export class DiffService {
// That's a pretty serious inconsistency that should not happen at all, // That's a pretty serious inconsistency that should not happen at all,
// we're just doing some basic damage control here. // we're just doing some basic damage control here.
const msg = const msg =
'Inconsistent data. A change recommendation is probably referring to a non-existant line number.'; 'Inconsistent data. A change recommendation or amendment is probably referring to a non-existant line number.';
return '<em style="color: red; font-weight: bold;">' + msg + '</em>'; return '<em style="color: red; font-weight: bold;">' + msg + '</em>';
} }

View File

@ -140,9 +140,14 @@ export class MotionDetailDiffComponent extends BaseViewComponentDirective implem
let baseText: LineNumberedString; let baseText: LineNumberedString;
if (this.motion.isParagraphBasedAmendment()) { if (this.motion.isParagraphBasedAmendment()) {
baseText = this.motionRepo try {
.getAllAmendmentParagraphsWithOriginalLineNumbers(this.motion, this.lineLength, true) baseText = this.motionRepo
.join('\n'); .getAllAmendmentParagraphsWithOriginalLineNumbers(this.motion, this.lineLength, true)
.join('\n');
} catch (e) {
console.error(e);
return '';
}
} else { } else {
baseText = this.lineNumbering.insertLineNumbers(this.motion.text, this.lineLength); baseText = this.lineNumbering.insertLineNumbers(this.motion.text, this.lineLength);
} }
@ -184,9 +189,14 @@ export class MotionDetailDiffComponent extends BaseViewComponentDirective implem
} }
let baseText: LineNumberedString; let baseText: LineNumberedString;
if (this.motion.isParagraphBasedAmendment()) { if (this.motion.isParagraphBasedAmendment()) {
baseText = this.motionRepo try {
.getAllAmendmentParagraphsWithOriginalLineNumbers(this.motion, this.lineLength, true) baseText = this.motionRepo
.join('\n'); .getAllAmendmentParagraphsWithOriginalLineNumbers(this.motion, this.lineLength, true)
.join('\n');
} catch (e) {
console.error(e);
return '';
}
} else { } else {
baseText = this.lineNumbering.insertLineNumbers(this.motion.text, this.lineLength); baseText = this.lineNumbering.insertLineNumbers(this.motion.text, this.lineLength);
} }

View File

@ -769,6 +769,10 @@
{{ 'This field is required.' | translate }} {{ 'This field is required.' | translate }}
</div> </div>
</section> </section>
<section *ngFor="let paragraph of contentForm.value.broken_paragraphs">
<em class="red-warning-text">{{ 'This paragraph does not exist in the main motion anymore:' | translate }}</em>
<div class="motion-text" [innerHTML]="paragraph | trust: 'html'"></div>
</section>
</ng-container> </ng-container>
<!-- Paragraph-based amendments --> <!-- Paragraph-based amendments -->
@ -900,6 +904,9 @@
<i *ngIf="motion.parent">{{ 'No changes at the text.' | translate }}</i> <i *ngIf="motion.parent">{{ 'No changes at the text.' | translate }}</i>
<i *ngIf="!motion.parent">{{ 'The parent motion was deleted.' | translate }}</i> <i *ngIf="!motion.parent">{{ 'The parent motion was deleted.' | translate }}</i>
</div> </div>
<div class="alert alert-info alert-inconsistency" *ngIf="amendmentErrorMessage">
<i [innerHTML]="amendmentErrorMessage"></i>
</div>
<ng-container *ngIf="motion.parent && !isRecoMode(ChangeRecoMode.Diff) && !isFinalEdit"> <ng-container *ngIf="motion.parent && !isRecoMode(ChangeRecoMode.Diff) && !isFinalEdit">
<div <div
*ngFor="let paragraph of getAmendmentParagraphs(); trackBy: trackByIndex" *ngFor="let paragraph of getAmendmentParagraphs(); trackBy: trackByIndex"

View File

@ -78,6 +78,11 @@ span {
font-style: italic; font-style: italic;
} }
.alert-inconsistency {
color: red;
font-style: italic;
}
.motion-content { .motion-content {
h4 { h4 {
margin: 10px 10px 15px 0; margin: 10px 10px 15px 0;

View File

@ -430,6 +430,8 @@ export class MotionDetailComponent extends BaseViewComponentDirective implements
public recommendationReferencingMotions: ViewMotion[] = []; public recommendationReferencingMotions: ViewMotion[] = [];
public amendmentErrorMessage: string = null;
/** /**
* Constructs the detail view. * Constructs the detail view.
* *
@ -816,6 +818,7 @@ export class MotionDetailComponent extends BaseViewComponentDirective implements
if (formMotion.isParagraphBasedAmendment()) { if (formMotion.isParagraphBasedAmendment()) {
contentPatch.selected_paragraphs = []; contentPatch.selected_paragraphs = [];
contentPatch.broken_paragraphs = [];
const parentMotion = this.repo.getViewModel(formMotion.parent_id); const parentMotion = this.repo.getViewModel(formMotion.parent_id);
// Hint: lineLength is sometimes not loaded yet when this form is initialized; // Hint: lineLength is sometimes not loaded yet when this form is initialized;
// This doesn't hurt as long as patchForm is called when editing mode is started, i.e., later. // This doesn't hurt as long as patchForm is called when editing mode is started, i.e., later.
@ -833,6 +836,18 @@ export class MotionDetailComponent extends BaseViewComponentDirective implements
contentPatch['text_' + paragraphNo] = formMotion.amendment_paragraphs[paragraphNo]; contentPatch['text_' + paragraphNo] = formMotion.amendment_paragraphs[paragraphNo];
} }
}); });
// If the motion has been shortened after the amendment has been created, we will show the paragraphs
// of the amendment as read-only
for (
let paragraphNo = paragraphsToChoose.length;
paragraphNo < formMotion.amendment_paragraphs.length;
paragraphNo++
) {
if (formMotion.amendment_paragraphs[paragraphNo] !== null) {
contentPatch.broken_paragraphs.push(formMotion.amendment_paragraphs[paragraphNo]);
}
}
} }
} }
@ -867,6 +882,7 @@ export class MotionDetailComponent extends BaseViewComponentDirective implements
tags_id: [], tags_id: [],
origin: [''], origin: [''],
selected_paragraphs: [], selected_paragraphs: [],
broken_paragraphs: [],
statute_amendment: [''], // Internal value for the checkbox, not saved to the model statute_amendment: [''], // Internal value for the checkbox, not saved to the model
statute_paragraph_id: [''], statute_paragraph_id: [''],
motion_block_id: [], motion_block_id: [],
@ -1039,15 +1055,21 @@ export class MotionDetailComponent extends BaseViewComponentDirective implements
* TODO: Cleanup: repo function could be injected part of the model, to have easier access * TODO: Cleanup: repo function could be injected part of the model, to have easier access
* *
* @returns {DiffLinesInParagraph[]} * @returns {DiffLinesInParagraph[]}
* @throws Error
*/ */
public getAmendmentParagraphs(): DiffLinesInParagraph[] { public getAmendmentParagraphs(): DiffLinesInParagraph[] {
return this.repo.getAmendmentParagraphLines( try {
this.motion, this.amendmentErrorMessage = null;
this.lineLength, return this.repo.getAmendmentParagraphLines(
this.crMode, this.motion,
this.changeRecommendations, this.lineLength,
this.showAmendmentContext this.crMode,
); this.changeRecommendations,
this.showAmendmentContext
);
} catch (e) {
this.amendmentErrorMessage = e.toString();
}
} }
public getAmendmentParagraphLinesTitle(paragraph: DiffLinesInParagraph): string { public getAmendmentParagraphLinesTitle(paragraph: DiffLinesInParagraph): string {
@ -1209,17 +1231,22 @@ export class MotionDetailComponent extends BaseViewComponentDirective implements
changeRecommendation: null changeRecommendation: null
}; };
if (this.motion.isParagraphBasedAmendment()) { if (this.motion.isParagraphBasedAmendment()) {
const lineNumberedParagraphs = this.repo.getAllAmendmentParagraphsWithOriginalLineNumbers( try {
this.motion, const lineNumberedParagraphs = this.repo.getAllAmendmentParagraphsWithOriginalLineNumbers(
this.lineLength, this.motion,
false this.lineLength,
); false
data.changeRecommendation = this.changeRecoRepo.createAmendmentChangeRecommendationTemplate( );
this.motion, data.changeRecommendation = this.changeRecoRepo.createAmendmentChangeRecommendationTemplate(
lineNumberedParagraphs, this.motion,
lineRange, lineNumberedParagraphs,
this.lineLength lineRange,
); this.lineLength
);
} catch (e) {
console.error(e);
return;
}
} else { } else {
data.changeRecommendation = this.changeRecoRepo.createMotionChangeRecommendationTemplate( data.changeRecommendation = this.changeRecoRepo.createMotionChangeRecommendationTemplate(
this.motion, this.motion,

View File

@ -580,19 +580,23 @@ export class MotionPdfService {
if (motion.isParagraphBasedAmendment()) { if (motion.isParagraphBasedAmendment()) {
// this is logically redundant with the formation of amendments in the motion-detail html. // this is logically redundant with the formation of amendments in the motion-detail html.
// Should be refactored in a way that a service returns the correct html for both cases // Should be refactored in a way that a service returns the correct html for both cases
const changeRecos = this.changeRecoRepo.getChangeRecoOfMotion(motion.id); try {
const amendmentParas = this.motionRepo.getAmendmentParagraphLines( const changeRecos = this.changeRecoRepo.getChangeRecoOfMotion(motion.id);
motion, const amendmentParas = this.motionRepo.getAmendmentParagraphLines(
lineLength, motion,
crMode, lineLength,
changeRecos, crMode,
false changeRecos,
); false
for (const paragraph of amendmentParas) { );
motionText += '<h3>' + this.motionRepo.getAmendmentParagraphLinesTitle(paragraph) + '</h3>'; for (const paragraph of amendmentParas) {
motionText += `<div class="paragraphcontext">${paragraph.textPre}</div>`; motionText += '<h3>' + this.motionRepo.getAmendmentParagraphLinesTitle(paragraph) + '</h3>';
motionText += paragraph.text; motionText += `<div class="paragraphcontext">${paragraph.textPre}</div>`;
motionText += `<div class="paragraphcontext">${paragraph.textPost}</div>`; motionText += paragraph.text;
motionText += `<div class="paragraphcontext">${paragraph.textPost}</div>`;
}
} catch (e) {
motionText += '<em style="color: red; font-weight: bold;">' + e.toString() + '</em>';
} }
} else if (motion.isStatuteAmendment()) { } else if (motion.isStatuteAmendment()) {
// statute amendments // statute amendments