From ac50d6f8dccc5b502c7b7a3a106ef5935184494f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Ho=CC=88=C3=9Fl?= <tobias@hoessl.eu>
Date: Sun, 28 Feb 2021 18:38:51 +0100
Subject: [PATCH] Bugfix: better handling of inconsistent states in amendments
 if the base motion has shrunk

---
 .../motions/motion-repository.service.ts      | 42 ++++++++++---
 .../src/app/core/ui-services/diff.service.ts  |  5 +-
 .../motion-detail-diff.component.ts           | 22 +++++--
 .../motion-detail.component.html              |  7 +++
 .../motion-detail.component.scss              |  5 ++
 .../motion-detail/motion-detail.component.ts  | 63 +++++++++++++------
 .../motions/services/motion-pdf.service.ts    | 30 +++++----
 7 files changed, 127 insertions(+), 47 deletions(-)

diff --git a/client/src/app/core/repositories/motions/motion-repository.service.ts b/client/src/app/core/repositories/motions/motion-repository.service.ts
index 610785404..2730d5f7a 100644
--- a/client/src/app/core/repositories/motions/motion-repository.service.ts
+++ b/client/src/app/core/repositories/motions/motion-repository.service.ts
@@ -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;
diff --git a/client/src/app/core/ui-services/diff.service.ts b/client/src/app/core/ui-services/diff.service.ts
index 429a671e2..fcac55fcd 100644
--- a/client/src/app/core/ui-services/diff.service.ts
+++ b/client/src/app/core/ui-services/diff.service.ts
@@ -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>';
         }
 
diff --git a/client/src/app/site/motions/modules/motion-detail/components/motion-detail-diff/motion-detail-diff.component.ts b/client/src/app/site/motions/modules/motion-detail/components/motion-detail-diff/motion-detail-diff.component.ts
index 0fd73e919..3b2484211 100644
--- a/client/src/app/site/motions/modules/motion-detail/components/motion-detail-diff/motion-detail-diff.component.ts
+++ b/client/src/app/site/motions/modules/motion-detail/components/motion-detail-diff/motion-detail-diff.component.ts
@@ -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);
         }
diff --git a/client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.html b/client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.html
index 8dbac8f8d..173448648 100644
--- a/client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.html
+++ b/client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.html
@@ -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"
diff --git a/client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.scss b/client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.scss
index 664d1dda5..f2f9239e3 100644
--- a/client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.scss
+++ b/client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.scss
@@ -78,6 +78,11 @@ span {
     font-style: italic;
 }
 
+.alert-inconsistency {
+    color: red;
+    font-style: italic;
+}
+
 .motion-content {
     h4 {
         margin: 10px 10px 15px 0;
diff --git a/client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.ts b/client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.ts
index 95aaee312..4f6860657 100644
--- a/client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.ts
+++ b/client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.ts
@@ -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,
diff --git a/client/src/app/site/motions/services/motion-pdf.service.ts b/client/src/app/site/motions/services/motion-pdf.service.ts
index e8b09b300..bd59f2655 100644
--- a/client/src/app/site/motions/services/motion-pdf.service.ts
+++ b/client/src/app/site/motions/services/motion-pdf.service.ts
@@ -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