From 80124238448beb82eb89bb4f6af7fed0d314eb82 Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 8 Oct 2026 23:18:36 +0200 Subject: [PATCH] Persist rejected case tag suggestions and prevent removed tag resurrection --- app/modules/sag/templates/detail_v4.html | 38 +++++-- app/tags/backend/router.py | 34 +++++- app/workflows/classification.py | 5 + docs/workflow-manuel-testplan.md | 9 ++ tests/test_case_tag_suggestion_dismissal.py | 36 +++++++ tests/test_case_tag_suggestion_ui.py | 109 ++++++++++++++++++++ tests/test_workflow_dismissed_keywords.py | 55 ++++++++++ tests/test_workflow_tags.py | 6 +- 8 files changed, 281 insertions(+), 11 deletions(-) create mode 100644 tests/test_case_tag_suggestion_dismissal.py create mode 100644 tests/test_case_tag_suggestion_ui.py create mode 100644 tests/test_workflow_dismissed_keywords.py diff --git a/app/modules/sag/templates/detail_v4.html b/app/modules/sag/templates/detail_v4.html index e679ad0..f6cf35d 100644 --- a/app/modules/sag/templates/detail_v4.html +++ b/app/modules/sag/templates/detail_v4.html @@ -8400,12 +8400,6 @@ tags = await loadLegacyTags(); } - // Final fallback: use server-rendered tags from page context when API calls return empty. - if ((!Array.isArray(tags) || tags.length === 0) && Array.isArray(initialCaseTagsSnapshot) && initialCaseTagsSnapshot.length > 0) { - tags = normalizeLegacyTags(initialCaseTagsSnapshot); - usingLegacyCaseTags = true; - } - if (!Array.isArray(tags) || tags.length === 0) { moduleContainer.innerHTML = '
Ingen tags på sagen endnu
'; setModuleContentState('tags', false); @@ -8457,6 +8451,7 @@ const matched = Array.isArray(item.matched_words) ? item.matched_words.join(', ') : ''; const hint = matched ? `Match: ${escapeHtml(matched)}` : 'Tilføj tag'; return ` + + + `; }).join('')}`; } catch (error) { @@ -8476,6 +8477,24 @@ } } + async function dismissSuggestedCaseTag(tagId) { + const numericTagId = Number(tagId || 0); + if (!numericTagId) return; + try { + const response = await fetch(`/api/v1/tags/entity/case/${caseId}/suggestions/${numericTagId}`, { + method: 'DELETE', + credentials: 'include' + }); + if (!response.ok) { + const error = await response.json().catch(() => ({})); + throw new Error(error.detail || 'Kunne ikke afvise forslag'); + } + await loadCaseTagSuggestions(); + } catch (error) { + showCaseFeedback('Fejl: ' + error.message); + } + } + async function applySuggestedCaseTag(tagId) { if (!tagId) return; try { @@ -8561,6 +8580,12 @@ throw new Error(`Kunne ikke slette tag (${genericError || 'generic fail'} / ${legacyError || 'legacy fail'})`); } + for (let index = initialCaseTagsSnapshot.length - 1; index >= 0; index--) { + const tag = initialCaseTagsSnapshot[index]; + if (Number(tag.id || tag.tag_id) === numericTagId) { + initialCaseTagsSnapshot.splice(index, 1); + } + } await syncCaseTagsUi(); } @@ -8815,6 +8840,7 @@ window.syncCaseTagsUi = syncCaseTagsUi; window.loadTodoSteps = loadTodoSteps; window.removeCaseTagAndSync = removeCaseTagAndSync; + window.dismissSuggestedCaseTag = dismissSuggestedCaseTag; let todoUserId = null; diff --git a/app/tags/backend/router.py b/app/tags/backend/router.py index 2599176..1bf9502 100644 --- a/app/tags/backend/router.py +++ b/app/tags/backend/router.py @@ -1,7 +1,7 @@ """ Tag system API endpoints """ -from fastapi import APIRouter, HTTPException, Query +from fastapi import APIRouter, HTTPException, Query, Depends from typing import List, Optional import json import re @@ -13,6 +13,7 @@ from app.tags.backend.models import ( TagGroup, TagGroupCreate ) from app.core.database import execute_query, execute_query_single, execute_update +from app.core.auth_dependencies import require_any_permission router = APIRouter(prefix="/tags") @@ -608,6 +609,9 @@ async def suggest_entity_tags(entity_type: str, entity_id: int): (entity_type, entity_id), ) or [] existing_tag_ids = {int(row.get("tag_id")) for row in existing_rows if row.get("tag_id") is not None} + proposal_schema = execute_query_single("SELECT to_regclass('public.workflow_tag_proposals') AS name") or {} + dismissed_rows = (execute_query("SELECT tag_id FROM workflow_tag_proposals WHERE entity_type=%s AND entity_id=%s AND source='manual_dismissal' AND status='rejected'", (entity_type, entity_id)) or []) if proposal_schema.get('name') else [] + dismissed_tag_ids = {int(row['tag_id']) for row in dismissed_rows} candidate_rows = execute_query( """ @@ -633,7 +637,7 @@ async def suggest_entity_tags(entity_type: str, entity_id: int): suggestions = [] for row in candidate_rows: tag_id = int(row.get("id")) - if tag_id in existing_tag_ids: + if tag_id in existing_tag_ids or tag_id in dismissed_tag_ids: continue catch_words = _normalize_catch_words(row.get("catch_words")) @@ -655,6 +659,32 @@ async def suggest_entity_tags(entity_type: str, entity_id: int): suggestions.sort(key=lambda item: (-item["score"], item["tag"]["type"], item["tag"]["name"])) return suggestions + +@router.delete('/entity/case/{entity_id}/suggestions/{tag_id}') +def dismiss_case_tag_suggestion(entity_id: int, tag_id: int, + actor=Depends(require_any_permission('cases.edit', 'tickets.edit'))): + """Reject a computed suggestion on this case, preserving the tag catalogue.""" + from app.workflows.service import transaction, emit_event + with transaction() as cursor: + cursor.execute('SELECT id FROM sag_sager WHERE id=%s AND deleted_at IS NULL', (entity_id,)) + if not cursor.fetchone(): + raise HTTPException(404, 'Sag ikke fundet') + cursor.execute('SELECT id FROM tags WHERE id=%s', (tag_id,)) + if not cursor.fetchone(): + raise HTTPException(404, 'Tag ikke fundet') + cursor.execute("SELECT id FROM tmodule_times WHERE sag_id=%s AND status NOT IN ('approved','billed') AND economy_order_draft_id IS NULL ORDER BY id FOR UPDATE", (entity_id,)) + time_ids = [row['id'] for row in cursor.fetchall()] + cursor.execute('SELECT pg_advisory_xact_lock(hashtext(%s))', (f'tag:case:{entity_id}',)) + cursor.execute("SELECT id FROM workflow_tag_proposals WHERE entity_type='case' AND entity_id=%s AND tag_id=%s AND source='manual_dismissal' AND status='rejected'", (entity_id, tag_id)) + if not cursor.fetchone(): + cursor.execute("INSERT INTO workflow_tag_proposals(entity_type,entity_id,tag_id,source,status,actor_id,reviewed_at) VALUES('case',%s,%s,'manual_dismissal','rejected',%s,now())", (entity_id, tag_id, actor['id'])) + cursor.execute("INSERT INTO workflow_tag_history(entity_type,entity_id,tag_id,action,source,actor_id) VALUES('case',%s,%s,'reject','manual_dismissal',%s)", (entity_id, tag_id, actor['id'])) + if time_ids: + cursor.execute('UPDATE tmodule_times SET workflow_revision=workflow_revision+1 WHERE id=ANY(%s) RETURNING id,workflow_revision', (time_ids,)) + for row in cursor.fetchall(): + emit_event(cursor, 'time.updated', 'time_entry', row['id'], row['workflow_revision']) + return {'message': 'Tagforslag afvist'} + @router.get("/search") async def search_tags(q: str, type: Optional[TagType] = None): """Search tags by name (fuzzy search)""" diff --git a/app/workflows/classification.py b/app/workflows/classification.py index dada411..8c9d4c4 100644 --- a/app/workflows/classification.py +++ b/app/workflows/classification.py @@ -109,6 +109,11 @@ def enrich_entity_context(cursor, context, entity_type, entity_id): parents={r['id']:r['parent_id'] for r in cursor.fetchall()} cursor.execute('SELECT k.* FROM workflow_tag_keywords k JOIN tags t ON t.id=k.tag_id WHERE k.active AND t.is_active') matches=match_keywords(classification_input(context),cursor.fetchall()) + if entity_type in ('case', 'time_entry'): + case_id = entity_id if entity_type == 'case' else context.get('case_id') + cursor.execute("SELECT tag_id FROM workflow_tag_proposals WHERE source='manual_dismissal' AND status='rejected' AND ((entity_type=%s AND entity_id=%s) OR (entity_type='case' AND entity_id=%s))", (entity_type, entity_id, case_id)) + dismissed = {row['tag_id'] for row in cursor.fetchall()} + matches = [match for match in matches if match['tag_id'] not in dismissed] existing=set(context.get('concrete_tag_ids',context.get('tag_ids',context.get('tags',[])))) cursor.execute('SELECT t.id,t.tag_group_id,tg.behavior FROM tags t LEFT JOIN tag_groups tg ON tg.id=t.tag_group_id') groups={r['id']:r for r in cursor.fetchall()} diff --git a/docs/workflow-manuel-testplan.md b/docs/workflow-manuel-testplan.md index 64cb506..b3fade4 100644 --- a/docs/workflow-manuel-testplan.md +++ b/docs/workflow-manuel-testplan.md @@ -383,3 +383,12 @@ Automatisk SLA/eskalering, aftaleudløb samt eventkilder til `reminder.due` og ` 7. Kontrollér tom gemt dækning (alle tags), under-tags og en særskilt Faktura-regel med forrang. Slå kunden fra: nye tider får ingen aktiv beslutning. **Forventet:** gemt dækning vedligeholder den automatiske regel, mens aktivering, rettigheder og økonomisk godkendelse fortsat gælder. + +### T24 – Fjern og afvis tagforslag + +1. Fjern sagens sidste tag med krydset: listen skal blive tom uden at gendanne sidens gamle tags. +2. Hvis tagget foreslås igen på grund af teksten, vælg krydset under Forslag. Genindlæs sagen: forslaget skal stadig være væk. +3. Registrér ny tid: afvist tag må ikke genindsættes automatisk af keyword-regler eller bruges som beregnet aftaledækning. +4. Tilføj tagget manuelt fra tagvælgeren: det skal igen tælle som et eksplicit tag. Andre sagers forslag skal være uændrede. + +**Forventet:** + tilføjer; × afviser kun på den konkrete sag. Afvisningen gemmes med historik. diff --git a/tests/test_case_tag_suggestion_dismissal.py b/tests/test_case_tag_suggestion_dismissal.py new file mode 100644 index 0000000..990a7ca --- /dev/null +++ b/tests/test_case_tag_suggestion_dismissal.py @@ -0,0 +1,36 @@ +import asyncio +from contextlib import contextmanager +from unittest.mock import MagicMock +from app.tags.backend import router +from app.workflows import service + + +def test_dismissed_computed_tag_is_not_suggested(monkeypatch): + monkeypatch.setattr(router,'execute_query_single',lambda *args:{'id':196,'titel':'ABA support','beskrivelse':'','name':'workflow_tag_proposals'}) + def query(sql,params=None): + if 'FROM entity_tags' in sql:return [] + if 'FROM workflow_tag_proposals' in sql:return [{'tag_id':7}] + return [{'id':7,'name':'ABA','type':'type','catch_words':['aba'],'is_active':True}] + monkeypatch.setattr(router,'execute_query',query) + assert asyncio.run(router.suggest_entity_tags('case',196))==[] + + +def test_dismissal_is_audited_and_never_deletes_tag_or_assignment(monkeypatch): + cursor=MagicMock();cursor.fetchone.side_effect=[{'id':196},{'id':7},None];cursor.fetchall.return_value=[] + @contextmanager + def transaction():yield cursor + monkeypatch.setattr(service,'transaction',transaction) + assert router.dismiss_case_tag_suggestion(196,7,{'id':4})['message']=='Tagforslag afvist' + statements=[c.args[0] for c in cursor.execute.call_args_list] + assert any('INSERT INTO workflow_tag_proposals' in sql for sql in statements) + assert any('INSERT INTO workflow_tag_history' in sql for sql in statements) + assert not any(sql.startswith('DELETE') for sql in statements) + + +def test_repeated_dismissal_does_not_duplicate_history(monkeypatch): + cursor=MagicMock();cursor.fetchone.side_effect=[{'id':196},{'id':7},{'id':90}];cursor.fetchall.return_value=[] + @contextmanager + def transaction():yield cursor + monkeypatch.setattr(service,'transaction',transaction) + router.dismiss_case_tag_suggestion(196,7,{'id':4}) + assert not any(c.args[0].startswith('INSERT') for c in cursor.execute.call_args_list) diff --git a/tests/test_case_tag_suggestion_ui.py b/tests/test_case_tag_suggestion_ui.py new file mode 100644 index 0000000..dd883c8 --- /dev/null +++ b/tests/test_case_tag_suggestion_ui.py @@ -0,0 +1,109 @@ +from pathlib import Path +import shutil +import subprocess + +import pytest + + +SOURCE = Path('app/modules/sag/templates/detail_v4.html').read_text() + + +def function_between(start, end): + return SOURCE.split(start, 1)[1].split(end, 1)[0] + + +def run_node(script): + if not shutil.which('node'): + pytest.skip('Node required') + result = subprocess.run(['node', '-e', script], capture_output=True, text=True) + assert result.returncode == 0, result.stderr + + +def test_dismiss_suggestion_waits_for_success_and_reports_failure(): + body = function_between(' async function dismissSuggestedCaseTag(tagId) {', + ' async function applySuggestedCaseTag') + run_node(''' +const assert = require('node:assert/strict'); +const caseId = 196; +let reloads = 0, errors = [], succeed = false; +const loadCaseTagSuggestions = async () => { reloads++; }; +const showCaseFeedback = text => errors.push(text); +const fetch = async (url, options) => { + assert.equal(url, '/api/v1/tags/entity/case/196/suggestions/8'); + assert.equal(options.method, 'DELETE'); + return {ok: succeed, json: async () => ({detail: 'Ingen adgang'})}; +}; +async function dismissSuggestedCaseTag(tagId) { BODY +(async () => { + await dismissSuggestedCaseTag(8); + assert.equal(reloads, 0); + assert.deepEqual(errors, ['Fejl: Ingen adgang']); + succeed = true; + await dismissSuggestedCaseTag(8); + assert.equal(reloads, 1); +})().catch(error => { console.error(error); process.exitCode = 1; }); +'''.replace('BODY', body)) + + +def test_empty_authoritative_tags_do_not_resurrect_initial_snapshot(): + body = function_between(' async function loadCaseTagsModule() {', + ' async function loadCaseTagSuggestions') + run_node(''' +const assert = require('node:assert/strict'); +const caseId = 196, initialCaseTagsSnapshot = [{id: 8, name: 'ABA'}]; +const container = {innerHTML: ''}; +const document = {getElementById: () => container}; +let hasContent = null, requests = 0; +const setModuleContentState = (name, value) => { hasContent = value; }; +const fetch = async () => {requests++; return {ok: true, json: async () => []};}; +async function loadCaseTagsModule() { BODY +(async () => { + await loadCaseTagsModule(); + assert.equal(requests, 2); + assert.equal(hasContent, false); + assert.match(container.innerHTML, /Ingen tags/); + assert.doesNotMatch(container.innerHTML, /ABA/); +})().catch(error => { console.error(error); process.exitCode = 1; }); +'''.replace('BODY', body)) + + +def test_removing_tag_updates_snapshot_only_after_success(): + body = function_between(' async function removeCaseTagAndSync(tagId) {', + ' async function ') + run_node(''' +const assert = require('node:assert/strict'); +const caseId = 196, initialCaseTagsSnapshot = [{tag_id: '8'}, {id: 9}]; +const confirm = () => true; +let success = false, syncs = 0; +const fetch = async () => ({ok: success, status: 403}); +const syncCaseTagsUi = async () => { syncs++; }; +async function removeCaseTagAndSync(tagId) { BODY +(async () => { + await assert.rejects(removeCaseTagAndSync(8), /Kunne ikke slette tag/); + assert.equal(initialCaseTagsSnapshot.length, 2); + assert.equal(syncs, 0); + success = true; + await removeCaseTagAndSync(8); + assert.deepEqual(initialCaseTagsSnapshot, [{id: 9}]); + assert.equal(syncs, 1); +})().catch(error => { console.error(error); process.exitCode = 1; }); +'''.replace('BODY', body)) + + +def test_suggestion_has_separate_add_and_dismiss_controls(): + body = function_between(' async function loadCaseTagSuggestions() {', + ' async function dismissSuggestedCaseTag') + run_node(r''' +const assert = require('node:assert/strict'); +const caseId = 196, container = {innerHTML: ''}; +const document = {getElementById: () => container}; +const escapeHtml = value => value; +const fetch = async () => ({ok: true, json: async () => [{tag: {id: 8, name: 'ABA'}}]}); +async function loadCaseTagSuggestions() { BODY +(async () => { + await loadCaseTagSuggestions(); + assert.match(container.innerHTML, /applySuggestedCaseTag\(8\)/); + assert.match(container.innerHTML, /dismissSuggestedCaseTag\(8\)/); + assert.match(container.innerHTML, /Afvis forslag: ABA/); +})().catch(error => { console.error(error); process.exitCode = 1; }); +'''.replace('BODY', body)) diff --git a/tests/test_workflow_dismissed_keywords.py b/tests/test_workflow_dismissed_keywords.py new file mode 100644 index 0000000..2c061fe --- /dev/null +++ b/tests/test_workflow_dismissed_keywords.py @@ -0,0 +1,55 @@ +"""Manual dismissal wins over keyword rediscovery, including case time entries.""" +import pytest + +from app.workflows.classification import enrich_entity_context, apply_entity_keyword_matches + + +class Cursor: + def __init__(self, dismissed): + self.dismissed = dismissed + self.queries = [] + self.rows = [] + + def execute(self, sql, args=None): + self.queries.append((sql, args)) + if 'SELECT id,parent_id' in sql: + self.rows = [{'id': 1, 'parent_id': None}, {'id': 2, 'parent_id': 1}] + elif 'workflow_tag_keywords' in sql: + self.rows = [{'tag_id': 2, 'alternatives': ['ABA']}] + elif "source='manual_dismissal'" in sql: + self.rows = [{'tag_id': tag} for tag in self.dismissed] + elif 'SELECT t.id,t.tag_group_id' in sql: + self.rows = [] + else: + self.rows = [] + + def fetchall(self): + return self.rows + + +@pytest.mark.parametrize('entity_type,entity_id', [('case', 20), ('time_entry', 30)]) +def test_dismissed_keyword_not_reintroduced(entity_type, entity_id): + cursor = Cursor({2}) + context = {'case_id': 20, 'entity_type': entity_type, 'description': 'ABA service', 'concrete_tag_ids': []} + result = enrich_entity_context(cursor, context, entity_type, entity_id) + apply_entity_keyword_matches(cursor, entity_type, entity_id, result) + assert result['keyword_matches'] == [] + assert result['tag_ids'] == [] + assert not any('INSERT INTO entity_tags' in sql for sql, _ in cursor.queries) + dismissal_args = next(args for sql, args in cursor.queries if "source='manual_dismissal'" in sql) + assert dismissal_args == (entity_type, entity_id, 20) + + +def test_explicit_manual_readdition_still_counts_for_billing(): + cursor = Cursor({2}) + result = enrich_entity_context(cursor, {'case_id': 20, 'entity_type': 'case', 'subject': 'ABA service', 'concrete_tag_ids': [2]}, 'case', 20) + assert result['keyword_matches'] == [] + assert result['concrete_tag_ids'] == [2] + assert result['tag_ids'] == [1, 2] + + +def test_undismissed_keyword_still_added(): + cursor = Cursor(set()) + result = enrich_entity_context(cursor, {'case_id': 20, 'entity_type': 'case', 'subject': 'ABA service', 'concrete_tag_ids': []}, 'case', 20) + assert result['keyword_matches'][0]['tag_id'] == 2 + assert result['tag_ids'] == [1, 2] diff --git a/tests/test_workflow_tags.py b/tests/test_workflow_tags.py index aed5f22..31f38e7 100644 --- a/tests/test_workflow_tags.py +++ b/tests/test_workflow_tags.py @@ -58,7 +58,7 @@ def test_enrichment_preserves_manual_single_group(): class Cursor: def __init__(self): self.rows=iter([ [{'id':1,'parent_id':None},{'id':2,'parent_id':None}], - [{'tag_id':2,'alternatives':['Excel'],'mode':'contains'}], + [{'tag_id':2,'alternatives':['Excel'],'mode':'contains'}], [], [{'id':1,'tag_group_id':4,'behavior':'single'},{'id':2,'tag_group_id':4,'behavior':'single'}],[],[]]) def execute(self,*args): pass def fetchall(self): return next(self.rows) @@ -72,7 +72,7 @@ def test_keyword_single_group_multiple_matches_block(): class Cursor: def __init__(self): self.rows=iter([ [{'id':1,'parent_id':None},{'id':2,'parent_id':None}], - [{'tag_id':1,'alternatives':['Excel']},{'tag_id':2,'alternatives':['Excel']}], + [{'tag_id':1,'alternatives':['Excel']},{'tag_id':2,'alternatives':['Excel']}], [], [{'id':1,'tag_group_id':4,'behavior':'single'},{'id':2,'tag_group_id':4,'behavior':'single'}],[],[]]) def execute(self,*args): pass def fetchall(self): return next(self.rows) @@ -86,7 +86,7 @@ def test_case_keywords_title_description_and_parent(): class Cursor: def __init__(self): self.rows=iter([ [{'id':1,'parent_id':None},{'id':2,'parent_id':1}], - [{'tag_id':2,'alternatives':['Excel']}], + [{'tag_id':2,'alternatives':['Excel']}], [], [{'id':1,'tag_group_id':None,'behavior':None},{'id':2,'tag_group_id':None,'behavior':None}],[]]) def execute(self,*args): pass def fetchall(self): return next(self.rows)