TP2000-1387 Async Bulk Edit - Python 3.12 #1286
Conversation
…ular import issues
LaurenMullally
changed the title
| @@ -0,0 +1,112 @@ | |||
| from typing import Dict | |||
There was a problem hiding this comment.
This is the class that now handles editing the measures
| @@ -781,7 +781,7 @@ def __init__(self, *args, **kwargs): | |||
| ) | |||
|
|
|||
|
|
|||
| class MeasureStartDateForm(forms.Form): | |||
| class MeasureStartDateForm(forms.Form, SerializableFormMixin): | |||
There was a problem hiding this comment.
Throughout this file, I add some functions to each form, as well as the mixin, all of which aid the serializing and deserializing required for passing the form data to the celery task
| @@ -414,3 +415,164 @@ def _log_form_errors(self, form_class, form_or_formset) -> None: | |||
| for form_errors in errors: | |||
| for error_key, error_values in form_errors.items(): | |||
| logger.error(f"{error_key}: {error_values}") | |||
|
|
|||
There was a problem hiding this comment.
This is the model that we use to store the serialized form data, kwargs, selected measures, and other info. We store it in the db so that we can drag it out within the celery task as the celery task only likes being passed simple data types.
measures/tests/conftest.py
Outdated
| @@ -324,12 +324,33 @@ def mock_request(rf, valid_user, valid_user_client): | |||
| request.requests_session = requests.Session() | |||
There was a problem hiding this comment.
Making some fixtures in here. The tests I use them in are parameterised, but you have to do some special trick in order to get fixtures used in a parameterised test.. this meant writing a fixture for some really simple stuff, so not sure whether this the best approach or not.
| simple_measures_bulk_editor, | ||
| mocked_edit_schedule_apply_async, | ||
| ): | ||
| """Test that calling MeasuresBulkCreator.schedule() correctly schedules a |
There was a problem hiding this comment.
Need to change this doc string 😬 but this is a test to check that the task gets scheduled as expected
| @@ -58,11 +58,11 @@ | |||
|
|
|||
| @pytest.fixture() | |||
| def mocked_diff_components(): | |||
| """Mocks `diff_components()` inside `update_measure_components()` in | |||
| """Mocks `diff_components()` inside `update_measure_components()` that is called in | |||
| `MeasureEditWizard` to prevent parsing errors where test measures lack a | |||
There was a problem hiding this comment.
Need to change this, she's not called in the Measure Edit Wizard any more!!
| @@ -2038,7 +2038,7 @@ def test_multiple_measure_edit_single_form_functionality( | |||
| assert measure.update_type == UpdateType.UPDATE | |||
| assert reduce(getattr, updated_attribute.split("."), measure) == expected_data | |||
|
|
|||
|
|
|||
| @override_settings(MEASURES_ASYNC_EDIT=False) | |||
There was a problem hiding this comment.
These tests were preexisting, and checked that the form data provided did actually end up editing things as expected. Nice as it checks that the edit function works as it should, but I need to add this settings override so that it tests it through the synchronous route, or it complains that Celery isn't running! Considering that both routes use the same edit function, I felt this was okay.. but let me know if I'm wrong!
measures/util.py
Outdated
| @@ -49,6 +56,10 @@ def diff_components( | |||
| """ | |||
| from measures.parsers import DutySentenceParser | |||
|
|
|||
| # We add in the component output type here as otherwise we run into circular import issues. | |||
There was a problem hiding this comment.
Oh this was annoying!!! I'm not sure what I did, I think it's the making of the BulkEditor class pehaps?? but whatever I did gave me circular import errors which were such a pain. The workaround (Big thanks Paul) was to assign the type for the component output as none, then assign it correctly when it came to actually using the function!
| ) | ||
|
|
||
|
|
||
| def update_measure_condition_components( |
There was a problem hiding this comment.
These all lived in the MeasureEditWizard, but considering they're now shared across two routes, I think it makes sense to keep them here. I guess these could live in the measure editor class?? but idk
measures/views/mixins.py
Outdated
| @@ -50,3 +51,72 @@ def get_queryset(self): | |||
| """Get the queryset for measures that are candidates for | |||
| editing/deletion.""" | |||
| return models.Measure.objects.filter(pk__in=self.measure_selections) | |||
|
|
|||
|
|
|||
| class MeasureSerializableWizardMixin(): | |||
There was a problem hiding this comment.
Moved these functions into a mixin that can now be shared across wizards..
TP2000-1387 Async Bulk Edit
Why
The moment you've all been waiting for!! Sad it only amounts to 15 file changes oof.
As part of efforts to make things quicker in TAP, we're utilising celery tasks to take some of the processing work away from the web worker. Previously we changed over the MeasureBulkCreate journey, this PR Is for the MeasureBulkEdit journey. It's pretty much a replica process just applied to the edits.
What
MEASURE_ASYNC_EDITswitch is on.MeasuresBulkEditorobject, then a task is scheduled. The task is run asynchronously, and handles setting the state of the task, although we don't utilise it like we do in measures as the design hasn't changed this time, and then handles calling theMeasureEditorclass'edit_measures()function.edit_measures()function above, just all within the web worker.Checklist