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

Merge pull request #5920 from CatoTH/bugfix/inconsistency-amendment-paragraphs

Browse Source 
Bugfix: better handling of inconsistent states in amendments if the b…
This commit is contained in:
Emanuel Schütze 2021-03-04 21:26:12 +01:00 committed by GitHub
commit 291402e159
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 127 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -336,13 +336,18 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
const changeRecos = viewMotion.changeRecommendations.filter(changeReco =>
changeReco.showInFinalView()
);
return this.getAmendmentParagraphLines(
viewMotion,
this.motionLineLength,
ChangeRecoMode.Changed,
changeRecos,
false
);
try {
return this.getAmendmentParagraphLines(
viewMotion,
this.motionLineLength,
ChangeRecoMode.Changed,
changeRecos,
false
);
} catch (e) {
// Inconsistency between motion and amendment -> the best we can do is not to fail completely
return [];
}
} else {
return [];
}
@ -810,6 +815,13 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
let paragraph: string;
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) {
paragraph = baseParagraphs[paraNo];
paragraphHasChanges = false;
@ -855,6 +867,7 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
* @param {ViewMotionChangeRecommendation[]} changeRecommendations
* @param {boolean} includeUnchanged
* @returns {DiffLinesInParagraph}
* @throws Error
*/
public getAmendmentParagraphLines(
amendment: ViewMotion,
@ -876,7 +889,11 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
return amendmentParagraphs
?.map(
(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(
paraNo,
baseParagraphs[paraNo],
@ -950,6 +967,12 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
if (newText === 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],
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 => {
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) {
return origText;

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

@ -2238,7 +2238,8 @@ export class DiffService {
// That's a pretty serious inconsistency that should not happen at all,
// we're just doing some basic damage control here.
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>';
}
@ -2299,7 +2300,7 @@ export class DiffService {
// That's a pretty serious inconsistency that should not happen at all,
// we're just doing some basic damage control here.
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>';
}

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

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

7
client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.html
View File

@ -769,6 +769,10 @@
{{ 'This field is required.' | translate }}
</div>
</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>
<!-- Paragraph-based amendments -->
@ -900,6 +904,9 @@
<i *ngIf="motion.parent">{{ 'No changes at the text.' | translate }}</i>
<i *ngIf="!motion.parent">{{ 'The parent motion was deleted.' | translate }}</i>
</div>
<div class="alert alert-info alert-inconsistency" *ngIf="amendmentErrorMessage">
<i [innerHTML]="amendmentErrorMessage"></i>
</div>
<ng-container *ngIf="motion.parent && !isRecoMode(ChangeRecoMode.Diff) && !isFinalEdit">
<div
*ngFor="let paragraph of getAmendmentParagraphs(); trackBy: trackByIndex"

5
client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.scss
View File

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

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

@ -430,6 +430,8 @@ export class MotionDetailComponent extends BaseViewComponentDirective implements
public recommendationReferencingMotions: ViewMotion[] = [];
public amendmentErrorMessage: string = null;
/**
* Constructs the detail view.
*
@ -816,6 +818,7 @@ export class MotionDetailComponent extends BaseViewComponentDirective implements
if (formMotion.isParagraphBasedAmendment()) {
contentPatch.selected_paragraphs = [];
contentPatch.broken_paragraphs = [];
const parentMotion = this.repo.getViewModel(formMotion.parent_id);
// 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.
@ -833,6 +836,18 @@ export class MotionDetailComponent extends BaseViewComponentDirective implements
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: [],
origin: [''],
selected_paragraphs: [],
broken_paragraphs: [],
statute_amendment: [''], // Internal value for the checkbox, not saved to the model
statute_paragraph_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
*
* @returns {DiffLinesInParagraph[]}
* @throws Error
*/
public getAmendmentParagraphs(): DiffLinesInParagraph[] {
return this.repo.getAmendmentParagraphLines(
this.motion,
this.lineLength,
this.crMode,
this.changeRecommendations,
this.showAmendmentContext
);
try {
this.amendmentErrorMessage = null;
return this.repo.getAmendmentParagraphLines(
this.motion,
this.lineLength,
this.crMode,
this.changeRecommendations,
this.showAmendmentContext
);
} catch (e) {
this.amendmentErrorMessage = e.toString();
}
}
public getAmendmentParagraphLinesTitle(paragraph: DiffLinesInParagraph): string {
@ -1209,17 +1231,22 @@ export class MotionDetailComponent extends BaseViewComponentDirective implements
changeRecommendation: null
};
if (this.motion.isParagraphBasedAmendment()) {
const lineNumberedParagraphs = this.repo.getAllAmendmentParagraphsWithOriginalLineNumbers(
this.motion,
this.lineLength,
false
);
data.changeRecommendation = this.changeRecoRepo.createAmendmentChangeRecommendationTemplate(
this.motion,
lineNumberedParagraphs,
lineRange,
this.lineLength
);
try {
const lineNumberedParagraphs = this.repo.getAllAmendmentParagraphsWithOriginalLineNumbers(
this.motion,
this.lineLength,
false
);
data.changeRecommendation = this.changeRecoRepo.createAmendmentChangeRecommendationTemplate(
this.motion,
lineNumberedParagraphs,
lineRange,
this.lineLength
);
} catch (e) {
console.error(e);
return;
}
} else {
data.changeRecommendation = this.changeRecoRepo.createMotionChangeRecommendationTemplate(
this.motion,

30
client/src/app/site/motions/services/motion-pdf.service.ts
View File

@ -580,19 +580,23 @@ export class MotionPdfService {
if (motion.isParagraphBasedAmendment()) {
// 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
const changeRecos = this.changeRecoRepo.getChangeRecoOfMotion(motion.id);
const amendmentParas = this.motionRepo.getAmendmentParagraphLines(
motion,
lineLength,
crMode,
changeRecos,
false
);
for (const paragraph of amendmentParas) {
motionText += '<h3>' + this.motionRepo.getAmendmentParagraphLinesTitle(paragraph) + '</h3>';
motionText += `<div class="paragraphcontext">${paragraph.textPre}</div>`;
motionText += paragraph.text;
motionText += `<div class="paragraphcontext">${paragraph.textPost}</div>`;
try {
const changeRecos = this.changeRecoRepo.getChangeRecoOfMotion(motion.id);
const amendmentParas = this.motionRepo.getAmendmentParagraphLines(
motion,
lineLength,
crMode,
changeRecos,
false
);
for (const paragraph of amendmentParas) {
motionText += '<h3>' + this.motionRepo.getAmendmentParagraphLinesTitle(paragraph) + '</h3>';
motionText += `<div class="paragraphcontext">${paragraph.textPre}</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()) {
// statute amendments