mirror of
https://github.com/DRYTRIX/TimeTracker.git
synced 2026-05-08 05:19:48 -05:00
feat: Add client count tracking and cleanup for custom field definitions
- Add count_clients_with_value() method to CustomFieldDefinition model to track how many clients have values for each custom field - Display client count in custom field definitions list view - Automatically remove custom field values from all clients when a custom field definition is deleted - Show user-friendly confirmation message indicating how many clients were affected when deleting a field definition - Update client view to use custom field definitions for friendly field names instead of raw field keys - Add comprehensive test suite for custom field definitions including model creation, client count functionality, deletion cleanup, and edge cases - Update templates to display client counts and improve delete confirmation dialogs
This commit is contained in:
@@ -55,3 +55,19 @@ class CustomFieldDefinition(db.Model):
|
||||
def get_by_key(cls, field_key):
|
||||
"""Get a custom field definition by its key"""
|
||||
return cls.query.filter_by(field_key=field_key, is_active=True).first()
|
||||
|
||||
def count_clients_with_value(self):
|
||||
"""Count how many clients have a value for this custom field"""
|
||||
from app.models import Client
|
||||
from sqlalchemy import func
|
||||
|
||||
# Query clients that have this field key in their custom_fields JSON
|
||||
# This works for both SQLite and PostgreSQL
|
||||
count = 0
|
||||
for client in Client.query.all():
|
||||
if client.custom_fields and self.field_key in client.custom_fields:
|
||||
value = client.custom_fields.get(self.field_key)
|
||||
# Count only if value is not empty
|
||||
if value and str(value).strip():
|
||||
count += 1
|
||||
return count
|
||||
+16
-4
@@ -279,9 +279,6 @@ def view_client(client_id):
|
||||
"remaining_hours": float(remaining_hours),
|
||||
}
|
||||
|
||||
# Get rendered links from link templates
|
||||
rendered_links = client.get_rendered_links()
|
||||
|
||||
# Get link templates for custom fields (for clickable values)
|
||||
from app.models import LinkTemplate
|
||||
from sqlalchemy.exc import ProgrammingError
|
||||
@@ -299,6 +296,21 @@ def view_client(client_id):
|
||||
else:
|
||||
raise
|
||||
|
||||
# Get custom field definitions for friendly names
|
||||
custom_field_definitions_by_key = {}
|
||||
try:
|
||||
for definition in CustomFieldDefinition.get_active_definitions():
|
||||
custom_field_definitions_by_key[definition.field_key] = definition
|
||||
except ProgrammingError as e:
|
||||
# Handle case where custom_field_definitions table doesn't exist (migration not run)
|
||||
if "does not exist" in str(e.orig) or "relation" in str(e.orig).lower():
|
||||
current_app.logger.warning(
|
||||
"custom_field_definitions table does not exist. Run migration: flask db upgrade"
|
||||
)
|
||||
custom_field_definitions_by_key = {}
|
||||
else:
|
||||
raise
|
||||
|
||||
# Get recent time entries for this client
|
||||
# Include entries directly linked to client and entries through projects
|
||||
project_ids = [p.id for p in projects]
|
||||
@@ -330,9 +342,9 @@ def view_client(client_id):
|
||||
contacts=contacts,
|
||||
primary_contact=primary_contact,
|
||||
prepaid_overview=prepaid_overview,
|
||||
rendered_links=rendered_links,
|
||||
recent_time_entries=recent_time_entries,
|
||||
link_templates_by_field=link_templates_by_field,
|
||||
custom_field_definitions_by_key=custom_field_definitions_by_key,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -18,6 +18,9 @@ custom_field_definitions_bp = Blueprint("custom_field_definitions", __name__)
|
||||
def list_custom_field_definitions():
|
||||
"""List all custom field definitions"""
|
||||
definitions = CustomFieldDefinition.query.order_by(CustomFieldDefinition.order, CustomFieldDefinition.label).all()
|
||||
# Add client count for each definition
|
||||
for definition in definitions:
|
||||
definition.client_count = definition.count_clients_with_value()
|
||||
return render_template("admin/custom_field_definitions/list.html", definitions=definitions)
|
||||
|
||||
|
||||
@@ -138,12 +141,42 @@ def edit_custom_field_definition(definition_id):
|
||||
@admin_or_permission_required("manage_settings")
|
||||
def delete_custom_field_definition(definition_id):
|
||||
"""Delete a custom field definition"""
|
||||
from app.models import Client
|
||||
|
||||
definition = CustomFieldDefinition.query.get_or_404(definition_id)
|
||||
|
||||
field_key = definition.field_key
|
||||
|
||||
# Count clients that have a value for this custom field
|
||||
clients_with_value = []
|
||||
for client in Client.query.all():
|
||||
if client.custom_fields and field_key in client.custom_fields:
|
||||
value = client.custom_fields.get(field_key)
|
||||
if value and str(value).strip():
|
||||
clients_with_value.append(client)
|
||||
|
||||
client_count = len(clients_with_value)
|
||||
|
||||
# Remove the custom field from all clients that have it
|
||||
if client_count > 0:
|
||||
for client in clients_with_value:
|
||||
client.remove_custom_field(field_key)
|
||||
|
||||
# Commit the client updates before deleting the definition
|
||||
if not safe_commit("remove_custom_field_from_clients", {"field_key": field_key, "client_count": client_count}):
|
||||
flash(_("Could not remove custom field from clients due to a database error."), "error")
|
||||
return redirect(url_for("custom_field_definitions.list_custom_field_definitions"))
|
||||
|
||||
# Now delete the definition
|
||||
db.session.delete(definition)
|
||||
if not safe_commit("delete_custom_field_definition", {"definition_id": definition.id}):
|
||||
flash(_("Could not delete custom field definition due to a database error."), "error")
|
||||
else:
|
||||
flash(_("Custom field definition deleted successfully"), "success")
|
||||
if client_count > 0:
|
||||
flash(
|
||||
_("Custom field definition deleted successfully. The field was removed from %(count)d client(s).", count=client_count),
|
||||
"success"
|
||||
)
|
||||
else:
|
||||
flash(_("Custom field definition deleted successfully"), "success")
|
||||
|
||||
return redirect(url_for("custom_field_definitions.list_custom_field_definitions"))
|
||||
|
||||
@@ -61,7 +61,7 @@
|
||||
<a href="{{ url_for('custom_field_definitions.edit_custom_field_definition', definition_id=definition.id) }}" class="text-primary hover:underline">
|
||||
<i class="fas fa-edit"></i> {{ _('Edit') }}
|
||||
</a>
|
||||
<form method="POST" action="{{ url_for('custom_field_definitions.delete_custom_field_definition', definition_id=definition.id) }}" class="inline" onsubmit="return confirm('{{ _('Are you sure you want to delete this custom field definition?') }}');">
|
||||
<form method="POST" action="{{ url_for('custom_field_definitions.delete_custom_field_definition', definition_id=definition.id) }}" class="inline delete-field-form" data-client-count="{{ definition.client_count }}" data-field-label="{{ definition.label|e }}">
|
||||
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
|
||||
<button type="submit" class="text-red-600 dark:text-red-400 hover:underline">
|
||||
<i class="fas fa-trash"></i> {{ _('Delete') }}
|
||||
@@ -105,4 +105,30 @@
|
||||
</ul>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<script>
|
||||
document.addEventListener('DOMContentLoaded', function() {
|
||||
const deleteForms = document.querySelectorAll('.delete-field-form');
|
||||
deleteForms.forEach(function(form) {
|
||||
form.addEventListener('submit', function(e) {
|
||||
const clientCount = parseInt(form.dataset.clientCount) || 0;
|
||||
const fieldLabel = form.dataset.fieldLabel || '';
|
||||
|
||||
let message;
|
||||
if (clientCount > 0) {
|
||||
message = '{{ _("Warning: %(count)d client(s) have a value for this custom field. Deleting this field definition will remove the field value from all %(count)d client(s). Are you sure you want to delete the custom field definition \"%(label)s\"?") }}'
|
||||
.replace(/%(count)d/g, clientCount)
|
||||
.replace('%(label)s', fieldLabel);
|
||||
} else {
|
||||
message = '{{ _("Are you sure you want to delete this custom field definition?") }}';
|
||||
}
|
||||
|
||||
if (!confirm(message)) {
|
||||
e.preventDefault();
|
||||
return false;
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
</script>
|
||||
{% endblock %}
|
||||
|
||||
@@ -274,7 +274,7 @@
|
||||
<i class="fas fa-chevron-down ml-auto sidebar-label"></i>
|
||||
</button>
|
||||
<ul id="workDropdown" class="{% if not work_open %}hidden {% endif %}mt-2 space-y-2 ml-6">
|
||||
{% set nav_active_timer = ep.startswith('timer.') %}
|
||||
{% set nav_active_timer = ep.startswith('timer.manual') %}
|
||||
{% set nav_active_time_entries = ep == 'timer.time_entries_overview' %}
|
||||
{% set nav_active_projects = ep.startswith('projects.') %}
|
||||
{% set nav_active_clients = ep.startswith('clients.') %}
|
||||
|
||||
@@ -112,7 +112,11 @@
|
||||
<div class="space-y-3">
|
||||
{% for key, value in client.custom_fields.items() %}
|
||||
<div>
|
||||
<h3 class="text-sm font-medium text-text-muted-light dark:text-text-muted-dark">{{ key }}</h3>
|
||||
{% set field_defs = custom_field_definitions_by_key|default({}) %}
|
||||
{% set field_definition = field_defs[key] if key in field_defs else None %}
|
||||
<h3 class="text-sm font-medium text-text-muted-light dark:text-text-muted-dark">
|
||||
{% if field_definition %}{{ field_definition.label }}{% else %}{{ key }}{% endif %}
|
||||
</h3>
|
||||
<p class="text-text-light dark:text-text-dark">
|
||||
{% set link_template = link_templates_by_field.get(key) if link_templates_by_field else None %}
|
||||
{% if link_template and value %}
|
||||
@@ -145,24 +149,6 @@
|
||||
</div>
|
||||
{% endif %}
|
||||
|
||||
{% if rendered_links %}
|
||||
<div class="bg-card-light dark:bg-card-dark p-6 rounded-lg shadow">
|
||||
<h2 class="text-lg font-semibold mb-4">{{ _('Quick Links') }}</h2>
|
||||
<div class="space-y-2">
|
||||
{% for link in rendered_links %}
|
||||
<a href="{{ link.url }}" target="_blank" rel="noopener noreferrer" class="flex items-center gap-2 px-3 py-2 bg-primary/10 hover:bg-primary/20 text-primary rounded-lg transition">
|
||||
{% if link.icon %}
|
||||
<i class="{{ link.icon }}"></i>
|
||||
{% else %}
|
||||
<i class="fas fa-external-link-alt"></i>
|
||||
{% endif %}
|
||||
<span>{{ link.name }}</span>
|
||||
</a>
|
||||
{% endfor %}
|
||||
</div>
|
||||
</div>
|
||||
{% endif %}
|
||||
|
||||
{% if prepaid_overview %}
|
||||
<div class="bg-card-light dark:bg-card-dark p-6 rounded-lg shadow">
|
||||
<h2 class="text-lg font-semibold mb-4">{{ _('Prepaid Hours') }}</h2>
|
||||
|
||||
@@ -7,7 +7,7 @@ from setuptools import setup, find_packages
|
||||
|
||||
setup(
|
||||
name='timetracker',
|
||||
version='4.3.0',
|
||||
version='4.3.1',
|
||||
packages=find_packages(),
|
||||
include_package_data=True,
|
||||
install_requires=[
|
||||
|
||||
@@ -0,0 +1,304 @@
|
||||
"""
|
||||
Test suite for Custom Field Definitions.
|
||||
Tests model creation, deletion, and client field cleanup.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from datetime import datetime
|
||||
from app.models import CustomFieldDefinition, Client, User
|
||||
from app import db
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# CustomFieldDefinition Model Tests
|
||||
# ============================================================================
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
@pytest.mark.models
|
||||
@pytest.mark.smoke
|
||||
def test_custom_field_definition_creation(app, admin_user):
|
||||
"""Test basic custom field definition creation."""
|
||||
with app.app_context():
|
||||
definition = CustomFieldDefinition(
|
||||
field_key="debtor_number",
|
||||
label="Debtor Number",
|
||||
description="Client's debtor number in ERP system",
|
||||
is_mandatory=False,
|
||||
is_active=True,
|
||||
order=1,
|
||||
created_by=admin_user.id,
|
||||
)
|
||||
db.session.add(definition)
|
||||
db.session.commit()
|
||||
|
||||
assert definition.id is not None
|
||||
assert definition.field_key == "debtor_number"
|
||||
assert definition.label == "Debtor Number"
|
||||
assert definition.is_mandatory is False
|
||||
assert definition.is_active is True
|
||||
assert definition.order == 1
|
||||
assert definition.created_at is not None
|
||||
assert definition.updated_at is not None
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
@pytest.mark.models
|
||||
def test_count_clients_with_value_no_clients(app, admin_user):
|
||||
"""Test counting clients with value when no clients have the field."""
|
||||
with app.app_context():
|
||||
definition = CustomFieldDefinition(
|
||||
field_key="test_field",
|
||||
label="Test Field",
|
||||
created_by=admin_user.id,
|
||||
)
|
||||
db.session.add(definition)
|
||||
db.session.commit()
|
||||
|
||||
count = definition.count_clients_with_value()
|
||||
assert count == 0
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
@pytest.mark.models
|
||||
def test_count_clients_with_value_with_clients(app, admin_user, test_client):
|
||||
"""Test counting clients with value when clients have the field."""
|
||||
with app.app_context():
|
||||
definition = CustomFieldDefinition(
|
||||
field_key="test_field",
|
||||
label="Test Field",
|
||||
created_by=admin_user.id,
|
||||
)
|
||||
db.session.add(definition)
|
||||
db.session.commit()
|
||||
|
||||
# Set custom field value for client
|
||||
test_client.set_custom_field("test_field", "test_value")
|
||||
db.session.commit()
|
||||
|
||||
count = definition.count_clients_with_value()
|
||||
assert count == 1
|
||||
|
||||
# Add another client with the field
|
||||
client2 = Client(name="Test Client 2")
|
||||
db.session.add(client2)
|
||||
client2.set_custom_field("test_field", "another_value")
|
||||
db.session.commit()
|
||||
|
||||
count = definition.count_clients_with_value()
|
||||
assert count == 2
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
@pytest.mark.models
|
||||
def test_count_clients_with_value_ignores_empty(app, admin_user, test_client):
|
||||
"""Test that empty values are not counted."""
|
||||
with app.app_context():
|
||||
definition = CustomFieldDefinition(
|
||||
field_key="test_field",
|
||||
label="Test Field",
|
||||
created_by=admin_user.id,
|
||||
)
|
||||
db.session.add(definition)
|
||||
db.session.commit()
|
||||
|
||||
# Set empty value
|
||||
test_client.set_custom_field("test_field", "")
|
||||
db.session.commit()
|
||||
|
||||
count = definition.count_clients_with_value()
|
||||
assert count == 0
|
||||
|
||||
# Set whitespace-only value
|
||||
test_client.set_custom_field("test_field", " ")
|
||||
db.session.commit()
|
||||
|
||||
count = definition.count_clients_with_value()
|
||||
assert count == 0
|
||||
|
||||
# Set actual value
|
||||
test_client.set_custom_field("test_field", "valid_value")
|
||||
db.session.commit()
|
||||
|
||||
count = definition.count_clients_with_value()
|
||||
assert count == 1
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
@pytest.mark.models
|
||||
def test_count_clients_with_value_ignores_other_fields(app, admin_user, test_client):
|
||||
"""Test that only the specific field is counted."""
|
||||
with app.app_context():
|
||||
definition = CustomFieldDefinition(
|
||||
field_key="test_field",
|
||||
label="Test Field",
|
||||
created_by=admin_user.id,
|
||||
)
|
||||
db.session.add(definition)
|
||||
db.session.commit()
|
||||
|
||||
# Set a different field
|
||||
test_client.set_custom_field("other_field", "value")
|
||||
db.session.commit()
|
||||
|
||||
count = definition.count_clients_with_value()
|
||||
assert count == 0
|
||||
|
||||
# Set the correct field
|
||||
test_client.set_custom_field("test_field", "value")
|
||||
db.session.commit()
|
||||
|
||||
count = definition.count_clients_with_value()
|
||||
assert count == 1
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# Custom Field Definition Deletion Tests
|
||||
# ============================================================================
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.database
|
||||
def test_delete_custom_field_removes_from_clients(app, admin_user, test_client, admin_authenticated_client):
|
||||
"""Test that deleting a custom field definition removes it from all clients."""
|
||||
with app.app_context():
|
||||
# Create custom field definition
|
||||
definition = CustomFieldDefinition(
|
||||
field_key="debtor_number",
|
||||
label="Debtor Number",
|
||||
created_by=admin_user.id,
|
||||
)
|
||||
db.session.add(definition)
|
||||
db.session.commit()
|
||||
|
||||
# Set custom field value for client
|
||||
test_client.set_custom_field("debtor_number", "12345")
|
||||
db.session.commit()
|
||||
|
||||
# Verify client has the field
|
||||
assert test_client.get_custom_field("debtor_number") == "12345"
|
||||
|
||||
# Delete the definition via route
|
||||
response = admin_authenticated_client.post(
|
||||
f"/admin/custom-field-definitions/{definition.id}/delete",
|
||||
follow_redirects=True,
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
# Verify definition is deleted
|
||||
deleted_definition = CustomFieldDefinition.query.get(definition.id)
|
||||
assert deleted_definition is None
|
||||
|
||||
# Verify field is removed from client
|
||||
db.session.refresh(test_client)
|
||||
assert test_client.get_custom_field("debtor_number") is None
|
||||
assert "debtor_number" not in (test_client.custom_fields or {})
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.database
|
||||
def test_delete_custom_field_multiple_clients(app, admin_user, test_client, admin_authenticated_client):
|
||||
"""Test that deleting a custom field removes it from multiple clients."""
|
||||
with app.app_context():
|
||||
# Create custom field definition
|
||||
definition = CustomFieldDefinition(
|
||||
field_key="erp_id",
|
||||
label="ERP ID",
|
||||
created_by=admin_user.id,
|
||||
)
|
||||
db.session.add(definition)
|
||||
db.session.commit()
|
||||
|
||||
# Create multiple clients with the field
|
||||
client1 = Client(name="Client 1")
|
||||
client2 = Client(name="Client 2")
|
||||
client3 = Client(name="Client 3")
|
||||
db.session.add_all([client1, client2, client3])
|
||||
client1.set_custom_field("erp_id", "ERP001")
|
||||
client2.set_custom_field("erp_id", "ERP002")
|
||||
client3.set_custom_field("erp_id", "ERP003")
|
||||
db.session.commit()
|
||||
|
||||
# Delete the definition
|
||||
response = admin_authenticated_client.post(
|
||||
f"/admin/custom-field-definitions/{definition.id}/delete",
|
||||
follow_redirects=True,
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
# Verify field is removed from all clients
|
||||
db.session.refresh(client1)
|
||||
db.session.refresh(client2)
|
||||
db.session.refresh(client3)
|
||||
|
||||
assert client1.get_custom_field("erp_id") is None
|
||||
assert client2.get_custom_field("erp_id") is None
|
||||
assert client3.get_custom_field("erp_id") is None
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.database
|
||||
def test_delete_custom_field_no_clients_affected(app, admin_user, admin_authenticated_client):
|
||||
"""Test deleting a custom field when no clients have values."""
|
||||
with app.app_context():
|
||||
# Create custom field definition
|
||||
definition = CustomFieldDefinition(
|
||||
field_key="unused_field",
|
||||
label="Unused Field",
|
||||
created_by=admin_user.id,
|
||||
)
|
||||
db.session.add(definition)
|
||||
db.session.commit()
|
||||
|
||||
# Delete the definition
|
||||
response = admin_authenticated_client.post(
|
||||
f"/admin/custom-field-definitions/{definition.id}/delete",
|
||||
follow_redirects=True,
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
# Verify definition is deleted
|
||||
deleted_definition = CustomFieldDefinition.query.get(definition.id)
|
||||
assert deleted_definition is None
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.database
|
||||
def test_delete_custom_field_preserves_other_fields(app, admin_user, test_client, admin_authenticated_client):
|
||||
"""Test that deleting one custom field doesn't affect other custom fields."""
|
||||
with app.app_context():
|
||||
# Create two custom field definitions
|
||||
definition1 = CustomFieldDefinition(
|
||||
field_key="field1",
|
||||
label="Field 1",
|
||||
created_by=admin_user.id,
|
||||
)
|
||||
definition2 = CustomFieldDefinition(
|
||||
field_key="field2",
|
||||
label="Field 2",
|
||||
created_by=admin_user.id,
|
||||
)
|
||||
db.session.add_all([definition1, definition2])
|
||||
db.session.commit()
|
||||
|
||||
# Set both fields for client
|
||||
test_client.set_custom_field("field1", "value1")
|
||||
test_client.set_custom_field("field2", "value2")
|
||||
db.session.commit()
|
||||
|
||||
# Delete only definition1
|
||||
response = admin_authenticated_client.post(
|
||||
f"/admin/custom-field-definitions/{definition1.id}/delete",
|
||||
follow_redirects=True,
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
# Verify field1 is removed but field2 remains
|
||||
db.session.refresh(test_client)
|
||||
assert test_client.get_custom_field("field1") is None
|
||||
assert test_client.get_custom_field("field2") == "value2"
|
||||
|
||||
Reference in New Issue
Block a user