Compare commits

...

3 Commits

Author SHA1 Message Date
6945b17117 # Optimizations and fixes
* drops identifier handling on all edit-forms (identifier editing has been disabled on the frontend for a while now)
* updates test cases
* updates form caption for checking and recording action (less intimidating)
* optimizes district column width
* fixes bug on frontend parcel fetching on detail view
* adds extended tooltip for title column on index tables
* retraslates 'Law' to 'Rechtsgrundlage'
2024-02-08 07:31:19 +01:00
d639a4e530 # Geom parcel performance improvement
* refactors parcel calculating, resulting in 1.3-1.6x better performance
* optimizes parcel fetching view
2024-01-17 11:22:21 +01:00
523e338b1b # WIP: Performance boost parcel calculation
* improves handling of parcel calculation (speed up by ~30%)
* ToDo: Clean up code
2024-01-16 07:57:29 +01:00
23 changed files with 180 additions and 92 deletions

View File

@ -213,7 +213,6 @@ class EditCompensationForm(NewCompensationForm):
action = UserActionLogEntry.get_edited_action(user)
# Fetch data from cleaned POST values
identifier = self.cleaned_data.get("identifier", None)
title = self.cleaned_data.get("title", None)
intervention = self.cleaned_data.get("intervention", None)
is_cef = self.cleaned_data.get("is_cef", None)
@ -221,7 +220,6 @@ class EditCompensationForm(NewCompensationForm):
is_pik = self.cleaned_data.get("is_pik", None)
comment = self.cleaned_data.get("comment", None)
self.instance.identifier = identifier
self.instance.title = title
self.instance.intervention = intervention
self.instance.is_cef = is_cef

View File

@ -192,7 +192,6 @@ class EditEcoAccountForm(NewEcoAccountForm):
def save(self, user: User, geom_form: SimpleGeomForm):
with transaction.atomic():
# Fetch data from cleaned POST values
identifier = self.cleaned_data.get("identifier", None)
title = self.cleaned_data.get("title", None)
registration_date = self.cleaned_data.get("registration_date", None)
handler_type = self.cleaned_data.get("handler_type", None)
@ -219,7 +218,6 @@ class EditEcoAccountForm(NewEcoAccountForm):
self.instance.legal.save()
# Update main oject data
self.instance.identifier = identifier
self.instance.title = title
self.instance.deductable_surface = surface
self.instance.comment = comment

View File

@ -315,7 +315,6 @@ class Compensation(AbstractCompensation, CEFMixin, CoherenceMixin, PikMixin):
def get_detail_url_absolute(self):
return BASE_URL + self.get_detail_url()
def save(self, *args, **kwargs):
if self.identifier is None or len(self.identifier) == 0:
# Create new identifier is none was given

View File

@ -125,10 +125,16 @@ class CompensationWorkflowTestCase(BaseWorkflowTestCase):
self.compensation = self.fill_out_compensation(self.compensation)
pre_edit_log_count = self.compensation.log.count()
self.assertTrue(self.compensation.is_shared_with(self.superuser))
old_identifier = self.compensation.identifier
new_title = self.create_dummy_string()
new_identifier = self.create_dummy_string()
new_comment = self.create_dummy_string()
new_geometry = MultiPolygon(srid=4326) # Create an empty geometry
new_geometry = MultiPolygon(
self.compensation.geometry.geom.buffer(10),
srid=self.compensation.geometry.geom.srid
) # Create a geometry which differs from the stored one
geojson = self.create_geojson(new_geometry)
check_on_elements = {
@ -151,19 +157,21 @@ class CompensationWorkflowTestCase(BaseWorkflowTestCase):
check_on_elements = {
self.compensation.title: new_title,
self.compensation.identifier: new_identifier,
self.compensation.comment: new_comment,
}
for k, v in check_on_elements.items():
self.assertEqual(k, v)
self.assert_equal_geometries(self.compensation.geometry.geom, new_geometry)
# Expect identifier to not be editable
self.assertEqual(self.compensation.identifier, old_identifier, msg="Identifier was editable!")
# Expect logs to be set
self.assertEqual(pre_edit_log_count + 1, self.compensation.log.count())
self.assertEqual(self.compensation.log.first().action, UserAction.EDITED)
self.assert_equal_geometries(self.compensation.geometry.geom, new_geometry)
def test_checkability(self):
"""
This tests if the checkability of the compensation (which is defined by the linked intervention's checked

View File

@ -82,6 +82,7 @@ class EcoAccountWorkflowTestCase(BaseWorkflowTestCase):
url = reverse("compensation:acc:edit", args=(self.eco_account.id,))
pre_edit_log_count = self.eco_account.log.count()
old_identifier = self.eco_account.identifier
new_title = self.create_dummy_string()
new_identifier = self.create_dummy_string()
new_comment = self.create_dummy_string()
@ -114,7 +115,6 @@ class EcoAccountWorkflowTestCase(BaseWorkflowTestCase):
check_on_elements = {
self.eco_account.title: new_title,
self.eco_account.identifier: new_identifier,
self.eco_account.deductable_surface: test_deductable_surface,
self.eco_account.deductable_rest: test_deductable_surface - deductions_surface,
self.eco_account.comment: new_comment,
@ -123,6 +123,7 @@ class EcoAccountWorkflowTestCase(BaseWorkflowTestCase):
for k, v in check_on_elements.items():
self.assertEqual(k, v)
self.assertEqual(self.eco_account.identifier, old_identifier)
self.assert_equal_geometries(self.eco_account.geometry.geom, new_geometry)
# Expect logs to be set

View File

@ -133,7 +133,6 @@ class EditEmaForm(NewEmaForm):
def save(self, user: User, geom_form: SimpleGeomForm):
with transaction.atomic():
# Fetch data from cleaned POST values
identifier = self.cleaned_data.get("identifier", None)
title = self.cleaned_data.get("title", None)
handler_type = self.cleaned_data.get("handler_type", None)
handler_detail = self.cleaned_data.get("handler_detail", None)
@ -154,7 +153,6 @@ class EditEmaForm(NewEmaForm):
self.instance.responsible.save()
# Update main oject data
self.instance.identifier = identifier
self.instance.title = title
self.instance.comment = comment
self.instance.is_pik = is_pik

View File

@ -80,6 +80,7 @@ class EmaWorkflowTestCase(BaseWorkflowTestCase):
self.ema = self.fill_out_ema(self.ema)
pre_edit_log_count = self.ema.log.count()
old_identifier = self.ema.identifier
new_title = self.create_dummy_string()
new_identifier = self.create_dummy_string()
new_comment = self.create_dummy_string()
@ -106,13 +107,13 @@ class EmaWorkflowTestCase(BaseWorkflowTestCase):
check_on_elements = {
self.ema.title: new_title,
self.ema.identifier: new_identifier,
self.ema.comment: new_comment,
}
for k, v in check_on_elements.items():
self.assertEqual(k, v)
self.assertEqual(self.ema.identifier, old_identifier)
self.assert_equal_geometries(self.ema.geometry.geom, new_geometry)
# Expect logs to be set

View File

@ -130,7 +130,7 @@ class EditEmaFormTestCase(BaseTestCase):
self.assertIsNotNone(obj.responsible.handler)
self.assertEqual(obj.responsible.conservation_office, data["conservation_office"])
self.assertEqual(obj.responsible.conservation_file_number, data["conservation_file_number"])
self.assertEqual(obj.identifier, data["identifier"])
self.assertNotEqual(obj.identifier, data["identifier"], msg="Identifier editable via form!")
self.assertEqual(obj.comment, data["comment"])
last_log = obj.log.first()

View File

@ -345,7 +345,6 @@ class EditInterventionForm(NewInterventionForm):
"""
with transaction.atomic():
identifier = self.cleaned_data.get("identifier", None)
title = self.cleaned_data.get("title", None)
process_type = self.cleaned_data.get("type", None)
laws = self.cleaned_data.get("laws", None)
@ -379,7 +378,6 @@ class EditInterventionForm(NewInterventionForm):
self.instance.log.add(user_action)
self.instance.identifier = identifier
self.instance.title = title
self.instance.comment = comment
self.instance.modified = user_action

View File

@ -33,7 +33,7 @@ class CheckModalForm(BaseModalForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.form_title = _("Run check")
self.form_caption = _("I, {} {}, confirm that all necessary control steps have been performed by myself.").format(self.user.first_name, self.user.last_name)
self.form_caption = _("The necessary control steps have been performed:").format(self.user.first_name, self.user.last_name)
self.valid = False
def _are_deductions_valid(self):

View File

@ -33,6 +33,11 @@ class InterventionTable(BaseTable, TableRenderMixin, TableOrderMixin):
verbose_name=_("Parcel gmrkng"),
orderable=False,
accessor="geometry",
attrs={
"th": {
"class": "w-25",
}
}
)
c = tables.Column(
verbose_name=_("Checked"),

View File

@ -39,7 +39,7 @@ def index_view(request: HttpRequest):
"""
template = "generic_index.html"
# Filtering by user access is performed in table filter inside of InterventionTableFilter class
# Filtering by user access is performed in table filter inside InterventionTableFilter class
interventions = Intervention.objects.filter(
deleted=None, # not deleted
).select_related(

View File

@ -27,7 +27,7 @@ class RecordModalForm(BaseModalForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.form_title = _("Record data")
self.form_caption = _("I, {} {}, confirm that all necessary control steps have been performed by myself.").format(self.user.first_name, self.user.last_name)
self.form_caption = _("The necessary control steps have been performed:").format(self.user.first_name, self.user.last_name)
# Disable automatic w-100 setting for this type of modal form. Looks kinda strange
self.fields["confirm"].widget.attrs["class"] = ""

View File

@ -8,6 +8,7 @@ Created on: 15.11.21
import json
from django.contrib.gis.db.models import MultiPolygonField
from django.core.exceptions import ObjectDoesNotExist
from django.db import models, transaction
from django.utils import timezone
@ -147,69 +148,113 @@ class Geometry(BaseResource):
"""
Performs the main logic of parcel updating.
"""
from konova.models import Parcel, District, ParcelIntersection, Municipal, ParcelGroup
from konova.models import Parcel, District, Municipal, ParcelGroup
parcel_fetcher = ParcelFetcher(
geometry=self
)
fetched_parcels = parcel_fetcher.get_parcels()
_now = timezone.now()
underlying_parcels = []
districts = {}
municipals = {}
parcel_groups = {}
parcels_to_update = []
parcels_to_create = []
for result in fetched_parcels:
with transaction.atomic():
# There could be parcels which include the word 'Flur',
# which needs to be deleted and just keep the numerical values
## THIS CAN BE REMOVED IN THE FUTURE, WHEN 'Flur' WON'T OCCUR ANYMORE!
flr_val = result["flur"].replace("Flur ", "")
# There could be parcels which include the word 'Flur',
# which needs to be deleted and just keep the numerical values
## THIS CAN BE REMOVED IN THE FUTURE, WHEN 'Flur' WON'T OCCUR ANYMORE!
flr_val = result["flur"].replace("Flur ", "")
# Get district (cache in dict)
try:
district = districts["kreisschl"]
except KeyError:
district = District.objects.get_or_create(
key=result["kreisschl"],
name=result["kreis"],
)[0]
districts[district.key] = district
# Get municipal (cache in dict)
try:
municipal = municipals["gmdschl"]
except KeyError:
municipal = Municipal.objects.get_or_create(
key=result["gmdschl"],
name=result["gemeinde"],
district=district,
)[0]
municipals[municipal.key] = municipal
# Get parcel group (cache in dict)
try:
parcel_group = parcel_groups["gemaschl"]
except KeyError:
parcel_group = ParcelGroup.objects.get_or_create(
key=result["gemaschl"],
name=result["gemarkung"],
municipal=municipal,
)[0]
flrstck_nnr = result['flstnrnen']
if not flrstck_nnr:
parcel_groups[parcel_group.key] = parcel_group
# Preprocess parcel data
flrstck_nnr = result['flstnrnen']
match flrstck_nnr:
case "":
flrstck_nnr = None
flrstck_zhlr = result['flstnrzae']
if not flrstck_zhlr:
flrstck_zhlr = result['flstnrzae']
match flrstck_zhlr:
case "":
flrstck_zhlr = None
parcel_obj = Parcel.objects.get_or_create(
try:
# Try to fetch parcel from db. If it already exists, just update timestamp.
parcel_obj = Parcel.objects.get(
district=district,
municipal=municipal,
parcel_group=parcel_group,
flr=flr_val,
flrstck_nnr=flrstck_nnr,
flrstck_zhlr=flrstck_zhlr,
)[0]
parcel_obj.district = district
)
parcel_obj.updated_on = _now
parcel_obj.save()
underlying_parcels.append(parcel_obj)
parcels_to_update.append(parcel_obj)
except ObjectDoesNotExist:
# If not existing, create object but do not commit, yet
parcel_obj = Parcel(
district=district,
municipal=municipal,
parcel_group=parcel_group,
flr=flr_val,
flrstck_nnr=flrstck_nnr,
flrstck_zhlr=flrstck_zhlr,
updated_on=_now,
)
parcels_to_create.append(parcel_obj)
# Update the linked parcels
self.parcels.clear()
# Create new parcels
Parcel.objects.bulk_create(
parcels_to_create,
batch_size=500
)
# Update existing parcels
Parcel.objects.bulk_update(
parcels_to_update,
[
"updated_on"
],
batch_size=500
)
# Update linking to geometry
parcel_ids = [x.id for x in parcels_to_update] + [x.id for x in parcels_to_create]
underlying_parcels = Parcel.objects.filter(id__in=parcel_ids)
self.parcels.set(underlying_parcels)
# Set the calculated_on intermediate field, so this related data will be found on lookups
intersections_without_ts = self.parcelintersection_set.filter(
parcel__in=self.parcels.all(),
calculated_on__isnull=True,
)
for entry in intersections_without_ts:
entry.calculated_on = _now
ParcelIntersection.objects.bulk_update(
intersections_without_ts,
["calculated_on"]
)
@transaction.atomic
def _set_parcel_update_start_time(self):
"""
@ -233,9 +278,7 @@ class Geometry(BaseResource):
Returns:
parcels (QuerySet): The related parcels as queryset
"""
parcels = self.parcels.filter(
parcelintersection__calculated_on__isnull=False,
).prefetch_related(
parcels = self.parcels.prefetch_related(
"district",
"municipal",
).order_by(
@ -305,6 +348,36 @@ class Geometry(BaseResource):
}
return geojson
@property
def complexity_factor(self) -> float:
""" Calculates a factor to estimate the complexity of a Geometry
0 = very low complexity
1 = very high complexity
ASSUMPTION:
The envelope is the bounding box of a geometry. If the geometry's area is similar to the area of it's bounding
box, it is considered as rather simple, since it seems to be a closer shape like a simple box.
If the geometry has a very big bounding box, but the geometry's own area is rather small,
compared to the one of the bounding box, the complexity can be higher.
Example:
geometry area similar to bounding box --> geometry / bounding_box ~ 1
geometry area far smaller than bb --> geometry / bounding_box ~ 0
Result is being inverted for better understanding of 'low' and 'high' complexity.
Returns:
complexity_factor (float): The estimated complexity
"""
if self.geom.empty:
return 0
geom_envelope = self.geom.envelope
diff = geom_envelope - self.geom
complexity_factor = 1 - self.geom.area / diff.area
return complexity_factor
class GeometryConflict(UuidModel):
"""

View File

@ -49,5 +49,5 @@ ETS_GROUP = "Conservation office"
# GEOMETRY
## Max number of allowed vertices. Geometries larger will be simplified until they reach this threshold
GEOM_MAX_VERTICES = 10000
## Max seconds to wait for a parcel calculation result before a new request will be started (default: 5 minutes)
GEOM_THRESHOLD_RECALCULATION_SECONDS = 300
## Max seconds to wait for a parcel calculation result before a new request will be started (default: 30 minutes)
GEOM_THRESHOLD_RECALCULATION_SECONDS = 60 * 30

View File

@ -135,6 +135,7 @@ DATABASES = {
'USER': 'postgres',
'HOST': '127.0.0.1',
'PORT': '5432',
'CONN_MAX_AGE': 120,
}
}
DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField"

View File

@ -10,13 +10,14 @@ def celery_update_parcels(geometry_id: str, recheck: bool = True):
from konova.models import Geometry, ParcelIntersection
try:
geom = Geometry.objects.get(id=geometry_id)
objs = geom.parcelintersection_set.all()
for obj in objs:
obj.calculated_on = None
ParcelIntersection.objects.bulk_update(
objs,
["calculated_on"]
)
geom.parcels.clear()
#objs = geom.parcelintersection_set.all()
#for obj in objs:
# obj.calculated_on = None
#ParcelIntersection.objects.bulk_update(
# objs,
# ["calculated_on"]
#)
geom.update_parcels()
except ObjectDoesNotExist:

View File

@ -146,7 +146,6 @@ class BaseTestCase(TestCase):
geometry = Geometry.objects.create()
# Finally create main object, holding the other objects
intervention = Intervention.objects.create(
identifier="TEST",
title="Test_title",
responsible=responsibility_data,
legal=legal_data,
@ -174,7 +173,6 @@ class BaseTestCase(TestCase):
geometry = Geometry.objects.create()
# Finally create main object, holding the other objects
compensation = Compensation.objects.create(
identifier="TEST",
title="Test_title",
intervention=interv,
created=action,
@ -200,10 +198,8 @@ class BaseTestCase(TestCase):
responsible_data.handler = handler
responsible_data.save()
identifier = EcoAccount().generate_new_identifier()
# Finally create main object, holding the other objects
eco_account = EcoAccount.objects.create(
identifier=identifier,
title="Test_title",
deductable_surface=500,
legal=lega_data,
@ -230,7 +226,6 @@ class BaseTestCase(TestCase):
responsible_data.save()
# Finally create main object, holding the other objects
ema = Ema.objects.create(
identifier="TEST",
title="Test_title",
responsible=responsible_data,
created=action,
@ -474,7 +469,7 @@ class BaseTestCase(TestCase):
eco_account.save()
return eco_account
def assert_equal_geometries(self, geom1: MultiPolygon, geom2: MultiPolygon):
def assert_equal_geometries(self, geom1: MultiPolygon, geom2: MultiPolygon, tolerance = 0.001):
""" Assert for geometries to be equal
Transforms the geometries to matching srids before checking
@ -491,7 +486,6 @@ class BaseTestCase(TestCase):
self.assertTrue(True)
return
tolerance = 0.001
if geom1.srid != geom2.srid:
# Due to prior possible transformation of any of these geometries, we need to make sure there exists a
# transformation from one coordinate system into the other, which is valid

View File

@ -152,7 +152,7 @@ class RecordModalFormTestCase(BaseTestCase):
)
self.assertEqual(form.form_title, str(_("Record data")))
self.assertEqual(form.form_caption, str(
_("I, {} {}, confirm that all necessary control steps have been performed by myself.").format(
_("The necessary control steps have been performed:").format(
self.user.first_name,
self.user.last_name
)

View File

@ -173,9 +173,13 @@ class TableRenderMixin:
Returns:
"""
value_orig = value
max_length = 75
if len(value) > max_length:
value = f"{value[:max_length]}..."
value = format_html(
f'<div title="{value_orig}">{value}</div>'
)
return value
def render_d(self, value, record: GeoReferencedMixin):

View File

@ -33,34 +33,43 @@ class GeomParcelsView(LoginRequiredMixin, View):
Returns:
A rendered piece of HTML
"""
# HTTP code 286 states that the HTMX should stop polling for updates
# https://htmx.org/docs/#polling
status_code = 286
template = "konova/includes/parcels/parcel_table_frame.html"
geom = get_object_or_404(Geometry, id=id)
parcels = geom.get_underlying_parcels()
geos_geom = geom.geom or MultiPolygon(srid=DEFAULT_SRID_RLP)
geometry_exists = not geos_geom.empty and geos_geom.area > 0
geom_parcel_update_started = geom.parcel_update_start is not None
geom_parcel_update_finished = geom.parcel_update_end is not None
parcels = geom.get_underlying_parcels()
parcels_are_available = len(parcels) > 0
waiting_too_long = self._check_waiting_too_long(geom)
geometry_exists = not geos_geom.empty and geos_geom.area > 0
parcels_are_currently_calculated = (
geometry_exists and
geom.parcel_update_start and
not geom.parcel_update_end
)
parcels_available = len(parcels) > 0
if geometry_exists and not parcels_are_available and waiting_too_long:
# Trigger calculation again - process may have failed silently
celery_update_parcels.delay(geom.id)
parcels_are_currently_calculated = True
else:
parcels_are_currently_calculated = (
geometry_exists and
not parcels_are_available and
geom_parcel_update_started and
not geom_parcel_update_finished
)
if parcels_are_currently_calculated:
# Parcels are being calculated right now. Change the status code, so polling stays active for fetching
# results after the calculation
status_code = 200
else:
# HTTP code 286 states that the HTMX should stop polling for updates
# https://htmx.org/docs/#polling
status_code = 286
if waiting_too_long:
# Trigger calculation again
celery_update_parcels.delay(geom.id)
if parcels_available or not geometry_exists:
if parcels_are_available or not geometry_exists:
# Default case: Parcels are calculated or there is no geometry at all
# (so there will be no parcels to expect)
municipals = geom.get_underlying_municipals(parcels)
rpp = 100
@ -88,13 +97,15 @@ class GeomParcelsView(LoginRequiredMixin, View):
Depending on the geometry's modified attribute
"""
# Scale time to wait longer with increasing geometry complexity
complexity_factor = geom.complexity_factor + 1
wait_for_seconds = int(GEOM_THRESHOLD_RECALCULATION_SECONDS * complexity_factor)
try:
pcs_diff = (timezone.now() - geom.parcel_update_start).seconds
except TypeError:
pcs_diff = GEOM_THRESHOLD_RECALCULATION_SECONDS
pcs_diff = wait_for_seconds
calculation_not_finished = geom.parcel_update_end is None
waiting_too_long = (pcs_diff >= GEOM_THRESHOLD_RECALCULATION_SECONDS) and calculation_not_finished
waiting_too_long = (pcs_diff >= wait_for_seconds)
return waiting_too_long

Binary file not shown.

View File

@ -321,7 +321,7 @@ msgstr ""
#: intervention/templates/intervention/detail/view.html:39
#: intervention/templates/intervention/report/report.html:20
msgid "Law"
msgstr "Gesetz"
msgstr "Rechtsgrundlage"
#: analysis/templates/analysis/reports/includes/old_data/amount.html:17
#: compensation/templates/compensation/detail/compensation/includes/deadlines.html:33
@ -1452,11 +1452,9 @@ msgstr "Prüfung vornehmen"
#: intervention/forms/modals/check.py:36 konova/forms/modals/record_form.py:30
#: konova/tests/unit/test_forms.py:155
msgid ""
"I, {} {}, confirm that all necessary control steps have been performed by "
"myself."
"The necessary control steps have been performed:"
msgstr ""
"Ich, {} {}, bestätige, dass die notwendigen Kontrollschritte durchgeführt "
"wurden:"
"Die notwendigen Kontrollschritte wurden durchgeführt:"
#: intervention/forms/modals/deduction.py:33
msgid "Only recorded accounts can be selected for deductions"