Display form submission statistics based on logs #5015
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Initial pass at displaying submissions statistics from timeline logs. This doesn't take into account any aggregation yet, but allows for easy filtering of the log records. The plan is to modify the changelist template and to render the table markup manually, using an aggregation queryset to provide the necessary data. I'm a little bit concerned about customizing the admin for these kinds of things again.
This allows filtering the submissions on a date range rather than Django's predefined datetime field filters.
Add the extra snapshot data to existing form submission log records if it's still available, otherwise we can't do anything.
The list view will render the aggregation results, rather than each individual log record, so we override the entire result table and mimic the django markup. Note that the ordering options are not copied over/supported as this looks quite complex in django itself. We might at some point maybe support search fields on form name (or a list filter?) to narrow down results if it becomes too unwieldly.
* Test the results are aggregated * Test the search field searches through the form name * Test the date range filter
Fixed the type annotation errors that were cluttering actual errors, now we should not get any typing regressions anymore in the logevent and models module in the logging app.
sergei-maertens
force-pushed
the
feature/4931-submission-statistics-based-on-logs
branch
from
ea7e4a9 to
6eb13d5
Compare
There's no point anymore in keeping the model around and reacting to submission events via signals - we can delete this code and let the timeline-log-record-based approach replace it.
sergei-maertens
force-pushed
the
feature/4931-submission-statistics-based-on-logs
branch
from
6eb13d5 to
67ebb7e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5015 +/- ##
=======================================
Coverage 96.62% 96.62%
=======================================
Files 761 761
Lines 25961 25989 +28
Branches 3394 3395 +1
=======================================
+ Hits 25084 25112 +28
Misses 611 611
Partials 266 266 ☔ View full report in Codecov by Sentry. |
sergei-maertens
changed the title
vaszig
approved these changes
Jan 14, 2025
|
I shall add a test for the data migration! I forgor |
Attempted this but it's a world of pain - the database can't be flushed because a table is supposed to be deleted but that goes in a later migration, GFK's make everything harder and swapping the order of migrations leads to django/testing tools not being able to resolve the model state in the migration graph :/ I ran it locally and it produced the results I expected. |
This may be too confusing. It was decided with Joeri that we'll remove these columns.
This makes it possible to make the numbers match from the admin page with the aggregated results versus the number of records included in the export file.
The total result count is based on the changelist, but we don't render that, instead we render the aggregated/annotated queryset.
sergei-maertens
force-pushed
the
feature/4931-submission-statistics-based-on-logs
branch
from
a393bea to
950463d
Compare
vaszig
approved these changes
Jan 14, 2025
| @@ -0,0 +1,21 @@ | |||
| {% load i18n static %} | |||
There was a problem hiding this comment.
What is the purpose of overriding this?
There was a problem hiding this comment.
Removing the item result count, as it shows the wrong number. See commit message 719dbb4 :)
sergei-maertens
deleted the
feature/4931-submission-statistics-based-on-logs
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #4931
Changes
I'm a bit concerned about the heavy admin customization again 😒
On top of that - this will lose some historic data if organisations eagerly clean up their submissions - there's a best effort data migration to backfill this data, but it will take some time to get meaningful statistics over time.
Checklist
Check off the items that are completed or not relevant.
Impact on features
Release management
I have updated the translations assets (you do NOT need to provide translations)
./bin/makemessages_js.sh./bin/compilemessages_js.shDockerfile/scripts
./binfolderCommit hygiene