Ver Fonte

Füge Unterstützung für optionale Begründungen in der Wiedervorlage hinzu und optimiere die Kommentarverarbeitung

gc-server3 há 6 dias atrás
pai
commit
44e455f95e

+ 13 - 0
README.md

@@ -258,12 +258,25 @@ setzen das Eingabefeld anhand des aktuellen Datums in Europe/Berlin; erst „Spe
 übernimmt die Änderung. Ein fehlendes Datum lässt sich über „Festlegen“ ergänzen.
 Die Änderung schreibt ausschließlich `Forderungen_Details.Wiedervorlage` und den Kommentar
 `WV auf dd.mm.yyyy angepasst` mit dem angemeldeten Benutzer und Berliner Zeitstempel.
+Eine optionale mehrzeilige „Begründung“ wird nach einem Doppelpunkt an diesen Kommentar
+angehängt; Leerzeichen am Anfang und Ende werden entfernt. Bei Validierungs- oder
+Speicherfehlern bleiben Datum und Begründung im Dialog erhalten.
 Datum und Kommentar werden gemeinsam in einer Transaktion gespeichert. Ein unverändertes
 Datum erzeugt keinen weiteren Kommentar. Danach wird die Liste mit den bestehenden Filtern
 frisch geladen und die Rechnung fokussiert; passt sie nicht mehr zum Filter, erscheint
 der entsprechende Hinweis. Nach einem Login wird ein unterbrochener Speichervorgang nicht
 automatisch wiederholt.
 
+Chat und Wiedervorlage speichern Kommentare über dieselbe Funktion. Hat der Autor in
+`dbo.Benutzer_Rolle` mindestens eine Zuordnung mit `Rolle_ID = 'FIBU'`, erhält der neue
+Kommentar `Aktuell_Fibu = 1`, andernfalls `Aktuell_Abteilung = 1`; das jeweils andere
+Flag des neuen Kommentars ist `0`. Bei bisherigen Kommentaren derselben Kombination
+aus `Client_DB` und `Beleg_Nr` wird nur das entsprechende Flag auf `0` gesetzt.
+Flags, Kommentar und gegebenenfalls Datum werden gemeinsam gespeichert oder
+zurückgerollt. Eine SQL-Server-Sperre auf dem Forderungskopf serialisiert beide
+Speicherwege. Reine Datei-Uploads verändern die Flags nicht. Die beiden Flagspalten
+und `Benutzer_Rolle.Rolle_ID` müssen bereits im Datenbankschema vorhanden sein.
+
 ## Dokumente im Kommentarbereich
 
 Über die Büroklammer oder durch Ablegen auf dem Kommentarfeld können mehrere Dateien

+ 4 - 17
app/comment_submission.py

@@ -6,10 +6,10 @@ import re
 from datetime import datetime
 
 from fastapi import HTTPException
-from sqlalchemy import text
 from starlette.concurrency import run_in_threadpool
 from starlette.datastructures import UploadFile
 
+from .comments import save_comment
 from .config import settings
 from .file_storage import UploadError, get_files, save_upload
 from .forderung_history import history_context
@@ -67,24 +67,11 @@ async def submit_comment(request, pkey, db, scope):
             results.append(result)
 
         if comment.strip():
-            data = dict(
-                pkey,
-                Benutzer_ID=current_principal(request).username,
-                Timestamp=datetime.now().strftime("%Y-%m-%dT%H:%M:%S"),
-                Kommentar=comment,
-            )
             try:
-                query = templates.env.get_template("forderungen/details/queries/post/post_comments.sql").render(
-                    data=data
+                save_comment(
+                    db, pkey, username=current_principal(request).username,
+                    timestamp=datetime.now().strftime("%Y-%m-%dT%H:%M:%S"), comment=comment,
                 )
-                for statement in query.split(";"):
-                    if statement.strip():
-                        db.execute(
-                            text(statement).execution_options(
-                                query_name="forderungen/details/queries/post/post_comments.sql"
-                            ),
-                            data,
-                        )
                 db.commit()
                 comment_saved = True
             except Exception:

+ 46 - 0
app/comments.py

@@ -0,0 +1,46 @@
+"""Transactional comment writes shared by chat and follow-up editing."""
+
+from sqlalchemy import text
+
+
+def lock_comment_invoice(db, pkey):
+    """Serialize comment writers before touching details or current flags."""
+    row = db.execute(
+        text("""
+            SELECT Beleg_Nr FROM dbo.Forderungen_Kopf WITH (UPDLOCK, HOLDLOCK)
+            WHERE Client_DB = :Client_DB AND Beleg_Nr = :Beleg_Nr
+        """),
+        pkey,
+    ).first()
+    if row is None:
+        raise ValueError("Forderung nicht gefunden")
+
+
+def save_comment(db, pkey, *, username, timestamp, comment):
+    """Write a comment and its current flags; the caller owns the transaction."""
+    lock_comment_invoice(db, pkey)
+    fibu = db.execute(
+        text("""
+            SELECT TOP 1 1 FROM dbo.Benutzer_Rolle
+            WHERE Benutzer_ID = :Benutzer_ID AND Rolle_ID = 'FIBU'
+        """),
+        {"Benutzer_ID": username},
+    ).first() is not None
+    # The identifier comes exclusively from these two constants, never from input.
+    column = "Aktuell_Fibu" if fibu else "Aktuell_Abteilung"
+    db.execute(
+        text(f"""
+            UPDATE dbo.Forderungen_Kommentar SET {column} = 0
+            WHERE Client_DB = :Client_DB AND Beleg_Nr = :Beleg_Nr
+        """),
+        pkey,
+    )
+    db.execute(
+        text("""
+            INSERT INTO dbo.Forderungen_Kommentar
+                (Client_DB, Beleg_Nr, [Timestamp], Benutzer_ID, Kommentar, Aktuell_Fibu, Aktuell_Abteilung)
+            VALUES (:Client_DB, :Beleg_Nr, :Timestamp, :Benutzer_ID, :Kommentar, :Aktuell_Fibu, :Aktuell_Abteilung)
+        """),
+        dict(pkey, Benutzer_ID=username, Timestamp=timestamp, Kommentar=comment,
+             Aktuell_Fibu=int(fibu), Aktuell_Abteilung=int(not fibu)),
+    )

+ 12 - 12
app/wiedervorlage.py

@@ -11,6 +11,7 @@ from fastapi.responses import HTMLResponse, RedirectResponse
 from sqlalchemy import text
 from sqlalchemy.orm import Session
 
+from .comments import lock_comment_invoice, save_comment
 from .db import get_session
 from .jinja_templates import templates
 from .navigation import list_return_url, valid_focus
@@ -67,6 +68,7 @@ def _context(request, primary_key, db):
         next_week=(today + timedelta(days=7)).isoformat(),
         selected=current.isoformat() if current else "",
         error="",
+        begruendung="",
         return_url=return_url,
         modal_url=request.url.path + "?" + urlencode({"return_to": return_url}),
     )
@@ -92,10 +94,12 @@ def edit_wiedervorlage(request: Request, primary_key: str, db: Session = Depends
 
 @router.post("/app/forderungen/liste/{primary_key}/wiedervorlage", response_class=HTMLResponse)
 def save_wiedervorlage(
-    request: Request, primary_key: str, wiedervorlage: str = Form(""), db: Session = Depends(get_session)
+    request: Request, primary_key: str, wiedervorlage: str = Form(""), begruendung: str = Form(""),
+    db: Session = Depends(get_session),
 ):
     context = _context(request, primary_key, db)
     context["selected"] = wiedervorlage
+    context["begruendung"] = begruendung.strip()
     try:
         if not re.fullmatch(r"\d{4}-\d{2}-\d{2}", wiedervorlage):
             raise ValueError
@@ -105,6 +109,7 @@ def save_wiedervorlage(
         return _render(request, context, 422)
 
     try:
+        lock_comment_invoice(db, context["pkey"])
         existing = (
             db.execute(
                 text("""
@@ -140,17 +145,12 @@ def save_wiedervorlage(
                 """),
                     values,
                 )
-            db.execute(
-                text("""
-                INSERT INTO dbo.Forderungen_Kommentar (Client_DB, Beleg_Nr, [Timestamp], Benutzer_ID, Kommentar)
-                VALUES (:Client_DB, :Beleg_Nr, :Timestamp, :Benutzer_ID, :Kommentar)
-            """),
-                dict(
-                    context["pkey"],
-                    Timestamp=_now_berlin().replace(tzinfo=None),
-                    Benutzer_ID=current_principal(request).username,
-                    Kommentar=f'WV auf {selected.strftime("%d.%m.%Y")} angepasst',
-                ),
+            comment = f'WV auf {selected.strftime("%d.%m.%Y")} angepasst'
+            if context["begruendung"]:
+                comment += ": " + context["begruendung"]
+            save_comment(
+                db, context["pkey"], username=current_principal(request).username,
+                timestamp=_now_berlin().replace(tzinfo=None), comment=comment,
             )
         db.commit()
     except Exception:

+ 0 - 2
templates/forderungen/details/queries/post/post_comments.sql

@@ -1,2 +0,0 @@
-INSERT INTO [dbo].[Forderungen_Kommentar] ([Client_DB], [Beleg_Nr], [Timestamp], [Benutzer_ID], [Kommentar])
-VALUES (:Client_DB, :Beleg_Nr, :Timestamp, :Benutzer_ID, :Kommentar)

+ 2 - 0
templates/forderungen/wiedervorlage/modal.html

@@ -16,6 +16,8 @@
       <button type="button" class="btn btn-outline-secondary" data-gc-date="{{ next_week }}">Heute + 7 Tage</button>
     </div>
     <p class="gc-date-status" role="status" aria-live="polite"></p>
+    <label class="form-label" for="wiedervorlage-reason">Begründung</label>
+    <textarea class="form-control" id="wiedervorlage-reason" name="begruendung" rows="3">{{ begruendung }}</textarea>
   </div>
   <div class="modal-footer">
     <button type="button" class="btn btn-link" data-coreui-dismiss="modal">Abbrechen</button>

+ 121 - 0
tests/test_comments.py

@@ -0,0 +1,121 @@
+"""Exercise both production comment entry points and transactional flag updates."""
+
+import json
+import re
+import unittest
+from unittest.mock import patch
+
+from tests import test_wiedervorlage as fixture
+
+
+class CommentFlagsTests(unittest.TestCase):
+    def setUp(self):
+        self.fixture = fixture.WiedervorlageTests()
+        self.fixture.setUp()
+        self.addCleanup(self.fixture.doCleanups)
+        self.db = self.fixture.db
+        self.db.connection.execute(
+            'UPDATE dbo.Forderungen_Kommentar SET Aktuell_Fibu = 1, Aktuell_Abteilung = 1'
+        )
+        self.db.connection.execute("""
+            INSERT INTO dbo.Forderungen_Kommentar
+                (Client_DB, Beleg_Nr, Timestamp, Benutzer_ID, Kommentar, Aktuell_Fibu, Aktuell_Abteilung)
+            VALUES ('1', 'R1', '2026-01-03T12:00:00', 'other', 'Second old comment', 1, 1)
+        """)
+        self.db.connection.commit()
+
+    def snapshot(self):
+        return self.db.connection.execute(
+            'SELECT * FROM dbo.Forderungen_Kommentar ORDER BY rowid'
+        ).fetchall()
+
+    def post(self, channel, day=6):
+        if channel == 'wv':
+            response = self.fixture.post(f'2026-11-{day:02}', begruendung='Neue Zusage')
+            return response.status_code == 200
+        response = self.fixture.client.post(
+            '/app/forderungen/details/1_R1',
+            data={'source': 'comments', '_csrf': 'csrf', 'Kommentar': 'Chat comment',
+                  'Client_DB': '2', 'Beleg_Nr': 'R2', 'Benutzer_ID': 'intruder', 'Rolle_ID': 'FIBU'},
+        )
+        self.assertEqual(response.status_code, 200)
+        result = json.loads(re.search(r'data-gc-send-result>(.*?)</script>', response.text, re.S)[1])
+        return result['comment_saved']
+
+    def assert_flags(self, before, fibu):
+        rows = self.snapshot()
+        self.assertEqual(len(rows), len(before) + 1)
+        for previous, current in zip(before, rows):
+            expected = list(previous)
+            if previous[:2] == ('1', 'R1'):
+                expected[5 if fibu else 6] = 0
+            self.assertEqual(current, tuple(expected))
+        self.assertEqual(rows[-1][:2], ('1', 'R1'))
+        self.assertEqual(rows[-1][3], 'tester')
+        self.assertEqual(rows[-1][5:], (int(fibu), int(not fibu)))
+
+    def test_both_channels_reset_only_author_group_and_invoice(self):
+        # Alternate roles and entry points on the same invoice, including repeated writes.
+        for day, (channel, fibu) in enumerate(
+            [('chat', False), ('wv', False), ('chat', True), ('wv', True), ('chat', True), ('wv', False)], 6
+        ):
+            with self.subTest(channel=channel, fibu=fibu):
+                self.db.connection.execute("DELETE FROM dbo.Benutzer_Rolle WHERE Rolle_ID = 'FIBU'")
+                if fibu:
+                    # Existing MA role remains: any FIBU assignment must qualify.
+                    self.db.connection.execute(
+                        "INSERT INTO dbo.Benutzer_Rolle VALUES ('tester', 'Tester', 'Buchhaltung', 'FIBU')"
+                    )
+                self.db.connection.commit()
+                before = self.snapshot()
+                self.assertTrue(self.post(channel, day))
+                self.assert_flags(before, fibu)
+
+    def test_insert_update_role_lookup_and_commit_failures_roll_back_everything(self):
+        for channel in ['chat', 'wv']:
+            for failure in ['INSERT INTO dbo.Forderungen_Kommentar', 'UPDATE dbo.Forderungen_Kommentar',
+                            'FROM dbo.Benutzer_Rolle', 'commit']:
+                with self.subTest(channel=channel, failure=failure):
+                    before = self.snapshot()
+                    execute = self.db.execute
+
+                    def failing(query, values=None):
+                        if failure in str(query):
+                            raise RuntimeError('database unavailable')
+                        return execute(query, values)
+
+                    target = 'app.wiedervorlage' if channel == 'wv' else 'app.comment_submission'
+                    if failure == 'commit':
+                        mocked = patch.object(self.db, 'commit', side_effect=RuntimeError('commit failed'))
+                    else:
+                        mocked = patch.object(self.db, 'execute', side_effect=failing)
+                    with mocked, self.assertLogs(target, level='ERROR'):
+                        self.assertFalse(self.post(channel))
+                    self.assertEqual(self.snapshot(), before)
+                    self.assertIsNone(self.fixture.row())
+
+    def test_lock_is_acquired_before_any_write_in_both_channels(self):
+        for channel in ['chat', 'wv']:
+            queries = []
+            execute = self.db.execute
+
+            def recording(query, values=None):
+                queries.append(str(query))
+                return execute(query, values)
+
+            with patch.object(self.db, 'execute', side_effect=recording):
+                self.assertTrue(self.post(channel))
+            lock = next(i for i, sql in enumerate(queries)
+                        if 'Forderungen_Kopf WITH (UPDLOCK, HOLDLOCK)' in sql)
+            writes = [i for i, sql in enumerate(queries) if re.match(r'\s*(INSERT|UPDATE)\b', sql)]
+            self.assertTrue(writes)
+            self.assertLess(lock, min(writes))
+
+    def test_unchanged_date_does_not_reset_flags(self):
+        before = self.snapshot()
+        self.assertEqual(self.fixture.post('2026-10-15', begruendung='Text').status_code, 200)
+        self.assertEqual(self.snapshot(), before)
+
+
+if __name__ == '__main__':
+    unittest.main()

+ 12 - 3
tests/test_forderung_history.py

@@ -72,12 +72,16 @@ class FixtureDB:
         top = re.search(r"SELECT TOP (\d+)", sql, re.I)
         if top:
             sql = re.sub(r"SELECT TOP \d+", "SELECT", sql, count=1, flags=re.I).rstrip() + " LIMIT " + top[1]
+        sql = re.sub(r"\s+WITH \(UPDLOCK, HOLDLOCK\)", "", sql)
         return Result(self.connection.execute(sql, values or {}))
 
     def commit(self):
         self.commits += 1
         self.connection.commit()
 
+    def rollback(self):
+        self.connection.rollback()
+
 
 def seed(db):
     db.connection.executescript("""
@@ -91,8 +95,11 @@ def seed(db):
             Beleg_Datum TEXT, Beleg_Beschreibung TEXT, Betrag REAL, Benutzer_ID TEXT, Bezahlt TEXT, Beleg_Art TEXT
         );
         CREATE TABLE dbo.Forderungen_Kommentar (
-            Client_DB TEXT, Beleg_Nr TEXT, Timestamp TEXT, Benutzer_ID TEXT, Kommentar TEXT
+            Client_DB TEXT, Beleg_Nr TEXT, Timestamp TEXT, Benutzer_ID TEXT, Kommentar TEXT,
+            Aktuell_Fibu INTEGER DEFAULT 0, Aktuell_Abteilung INTEGER DEFAULT 0
         );
+        CREATE TABLE dbo.Benutzer_Rolle (Benutzer_ID TEXT, Name TEXT, Rolle_Name TEXT, Rolle_ID TEXT);
+        INSERT INTO dbo.Benutzer_Rolle VALUES ('tester', 'Tester', 'Mitarbeiter', 'MA');
         CREATE VIEW dbo.Forderungen_Kommentar_Benutzer AS
             SELECT *, 'Test User' AS Name, 'MA' AS Rolle_ID, 'Mitarbeiter' AS Rolle_Name
             FROM Forderungen_Kommentar;
@@ -132,7 +139,8 @@ def seed(db):
         )
     for tenant, number in [("1", "R1"), ("1", "R2"), ("1", "R3"), ("1", "H1"), ("1", "H2"), ("1", "X1"), ("2", "R1")]:
         db.connection.execute(
-            "INSERT INTO dbo.Forderungen_Kommentar VALUES (?, ?, '2026-01-02T12:00:00', 'tester', ?)",
+            "INSERT INTO dbo.Forderungen_Kommentar (Client_DB, Beleg_Nr, Timestamp, Benutzer_ID, Kommentar) "
+            "VALUES (?, ?, '2026-01-02T12:00:00', 'tester', ?)",
             (tenant, number, f"Comment {tenant}/{number}"),
         )
     db.connection.commit()
@@ -299,7 +307,8 @@ class HistoryTests(unittest.TestCase):
 
     def test_comment_order_and_escaping(self):
         self.db.connection.execute(
-            "INSERT INTO dbo.Forderungen_Kommentar VALUES ('1', 'R1', '2025-01-01T12:00:00', 'tester', '<script>old</script>')"
+            "INSERT INTO dbo.Forderungen_Kommentar (Client_DB, Beleg_Nr, Timestamp, Benutzer_ID, Kommentar) "
+            "VALUES ('1', 'R1', '2025-01-01T12:00:00', 'tester', '<script>old</script>')"
         )
         response = self.client.get("/app/forderungen/details/1_R1/kommentare?scope=debtor")
         self.assertIn("&lt;script&gt;old&lt;/script&gt;", response.text)

+ 4 - 0
tests/test_uploads.py

@@ -110,10 +110,14 @@ class UploadRouteTests(unittest.TestCase):
         return json.loads(re.search(r'data-gc-send-result>(.*?)</script>', response.text, re.S)[1])
 
     def test_file_only_no_comment_and_first_list(self):
+        self.db.connection.execute('UPDATE dbo.Forderungen_Kommentar SET Aktuell_Fibu = 1, Aktuell_Abteilung = 1')
+        self.db.connection.commit()
+        before = self.db.connection.execute('SELECT * FROM dbo.Forderungen_Kommentar').fetchall()
         response = self.post([('ä # + %20.txt', b'content')])
         result = self.result(response)
         self.assertFalse(result['comment_saved']); self.assertTrue(result['files'][0]['success'])
         self.assertEqual(self.db.commits, 0)
+        self.assertEqual(self.db.connection.execute('SELECT * FROM dbo.Forderungen_Kommentar').fetchall(), before)
         self.assertIn('id="case-files"', response.text)
         self.assertIn('hx-swap-oob="outerHTML"', response.text)
         name = get_files(KEY)[0].relative_path

+ 1 - 2
tests/test_verursacher.py

@@ -19,8 +19,7 @@ class VerursacherTests(unittest.TestCase):
             ALTER TABLE dbo.Forderungen_Kopf ADD Verursacher_Benutzer_ID TEXT;
             ALTER TABLE dbo.Forderungen_Kopf ADD Verursacher_Text TEXT;
             UPDATE dbo.Forderungen_Kopf SET Verursacher_Benutzer_ID = 'original', Verursacher_Text = 'Original Person';
-            CREATE TABLE dbo.Benutzer_Rolle (Benutzer_ID TEXT, Name TEXT, Rolle_Name TEXT);
-            INSERT INTO dbo.Benutzer_Rolle VALUES
+            INSERT INTO dbo.Benutzer_Rolle (Benutzer_ID, Name, Rolle_Name) VALUES
                 ('original', 'Original Person', 'Service'),
                 ('alice', 'Alice', 'Service'), ('alice', 'Alice', 'Buchhaltung'),
                 ('bob', 'Bob', 'Leitung'), ('id-only', NULL, NULL), ('', 'Invalid', 'Service');

+ 22 - 1
tests/test_wiedervorlage.py

@@ -94,11 +94,32 @@ class WiedervorlageTests(unittest.TestCase):
         self.assertEqual(len(self.comments()), 1)
 
     def test_same_date_and_repeated_submission_do_not_duplicate_comments(self):
-        self.post('2026-10-15')
+        self.post('2026-10-15', begruendung='Nur eine Begründung')
         self.assertIsNone(self.row()); self.assertEqual(self.comments(), [])
         self.post(); self.post()
         self.assertEqual(len(self.comments()), 1)
 
+    def test_optional_reason_is_trimmed_and_preserves_multiline_text(self):
+        self.assertIn('name="begruendung" rows="3"', self.get().text)
+        reason = '  Rücksprache <Kunde> & Fibu\nNeue Zusage  '
+        self.assertEqual(self.post(begruendung=reason).status_code, 200)
+        self.assertEqual(self.comments()[0][3],
+                         'WV auf 06.11.2026 angepasst: Rücksprache <Kunde> & Fibu\nNeue Zusage')
+        self.assertEqual(self.post('2026-11-07', begruendung=' \n ').status_code, 200)
+        self.assertEqual(self.comments()[1][3], 'WV auf 07.11.2026 angepasst')
+
+    def test_reason_is_retained_and_escaped_on_validation_and_save_errors(self):
+        reason = '  <script>Text</script> & Rückfrage\nZweite Zeile  '
+        response = self.post('invalid', begruendung=reason)
+        self.assertEqual(response.status_code, 422)
+        self.assertIn('&lt;script&gt;Text&lt;/script&gt; &amp; Rückfrage\nZweite Zeile</textarea>', response.text)
+        with patch.object(self.db, 'commit', side_effect=RuntimeError('offline')), self.assertLogs('app.wiedervorlage', level='ERROR'):
+            response = self.post(begruendung=reason)
+        self.assertEqual(response.status_code, 503)
+        self.assertIn('value="2026-11-06"', response.text)
+        self.assertIn('&lt;script&gt;Text&lt;/script&gt; &amp; Rückfrage\nZweite Zeile</textarea>', response.text)
+        self.assertEqual(self.comments(), [])
+
     def test_invalid_or_empty_dates_never_write(self):
         for value in ['', '2026-02-30', '2026-13-01', '30.10.2026', '20261030', "x'; DROP TABLE dbo.Forderungen_Details"]:
             with self.subTest(value=value):