From 0ca71e418b7d82f02abd590ba867d242aba0114f Mon Sep 17 00:00:00 2001 From: John Nagro Date: Thu, 18 Jan 2024 19:02:58 +0000 Subject: [PATCH 1/2] fix: framing up the revert --- .../migrations/0018_auto_20240110_1301.py | 28 +++++++----- .../migrations/0019_auto_20240118_1822.py | 44 +++++++++++++++++++ .../migrations/0034_auto_20240118_1822.py | 21 +++++++++ ...tatransmissionaudit_transmission_status.py | 17 +++++++ ...tatransmissionaudit_transmission_status.py | 17 +++++++ ...tatransmissionaudit_transmission_status.py | 17 +++++++ ...tatransmissionaudit_transmission_status.py | 17 +++++++ .../integrated_channel/models.py | 16 ------- .../transmitters/learner_data.py | 1 - ...tatransmissionaudit_transmission_status.py | 17 +++++++ ...tatransmissionaudit_transmission_status.py | 17 +++++++ ...tatransmissionaudit_transmission_status.py | 17 +++++++ .../test_transmitters/test_learner_data.py | 20 --------- 13 files changed, 200 insertions(+), 49 deletions(-) create mode 100644 integrated_channels/blackboard/migrations/0019_auto_20240118_1822.py create mode 100644 integrated_channels/canvas/migrations/0034_auto_20240118_1822.py create mode 100644 integrated_channels/cornerstone/migrations/0033_remove_cornerstonelearnerdatatransmissionaudit_transmission_status.py create mode 100644 integrated_channels/degreed/migrations/0033_remove_degreedlearnerdatatransmissionaudit_transmission_status.py create mode 100644 integrated_channels/degreed2/migrations/0025_remove_degreed2learnerdatatransmissionaudit_transmission_status.py create mode 100644 integrated_channels/integrated_channel/migrations/0032_remove_genericlearnerdatatransmissionaudit_transmission_status.py create mode 100644 integrated_channels/moodle/migrations/0033_remove_moodlelearnerdatatransmissionaudit_transmission_status.py create mode 100644 integrated_channels/sap_success_factors/migrations/0016_remove_sapsuccessfactorslearnerdatatransmissionaudit_transmission_status.py create mode 100644 integrated_channels/xapi/migrations/0013_remove_xapilearnerdatatransmissionaudit_transmission_status.py diff --git a/integrated_channels/blackboard/migrations/0018_auto_20240110_1301.py b/integrated_channels/blackboard/migrations/0018_auto_20240110_1301.py index 79c0ef22ba..8065735497 100644 --- a/integrated_channels/blackboard/migrations/0018_auto_20240110_1301.py +++ b/integrated_channels/blackboard/migrations/0018_auto_20240110_1301.py @@ -9,15 +9,19 @@ class Migration(migrations.Migration): ('blackboard', '0017_alter_historicalblackboardenterprisecustomerconfiguration_options'), ] - operations = [ - migrations.AddField( - model_name='blackboardlearnerassessmentdatatransmissionaudit', - name='transmission_status', - field=models.JSONField(blank=True, default=list, null=True), - ), - migrations.AddField( - model_name='blackboardlearnerdatatransmissionaudit', - name='transmission_status', - field=models.JSONField(blank=True, default=list, null=True), - ), - ] + # johnnagro 20240118 DOS-4490 + # Making this migration a no-op to revert a partially applied deploy migration + # operations = [ + # migrations.AddField( + # model_name='blackboardlearnerassessmentdatatransmissionaudit', + # name='transmission_status', + # field=models.JSONField(blank=True, default=list, null=True), + # ), + # migrations.AddField( + # model_name='blackboardlearnerdatatransmissionaudit', + # name='transmission_status', + # field=models.JSONField(blank=True, default=list, null=True), + # ), + # ] + + operations = [] diff --git a/integrated_channels/blackboard/migrations/0019_auto_20240118_1822.py b/integrated_channels/blackboard/migrations/0019_auto_20240118_1822.py new file mode 100644 index 0000000000..21c352992e --- /dev/null +++ b/integrated_channels/blackboard/migrations/0019_auto_20240118_1822.py @@ -0,0 +1,44 @@ +# Generated by Django 3.2.22 on 2024-01-18 18:22 + +from django.apps import apps +from django.db import migrations +from django.core.exceptions import FieldDoesNotExist + + +class Migration(migrations.Migration): + + dependencies = [ + ('blackboard', '0018_auto_20240110_1301'), + ] + + # johnnagro 20240118 DOS-4490 + # we are reverting a partially applied deployment migration + # this migration conditionally cleans up the previous migration operations + # only if they were part of the partially applied ones + + BlackboardLearnerAssessmentDataTransmissionAudit = apps.get_model('blackboard', 'BlackboardLearnerAssessmentDataTransmissionAudit') + BlackboardLearnerDataTransmissionAudit = apps.get_model('blackboard', 'BlackboardLearnerDataTransmissionAudit') + + operations = [] + + try: + if BlackboardLearnerDataTransmissionAudit._meta.get_field('transmission_status'): + operations.append( + migrations.RemoveField( + model_name='blackboardlearnerdatatransmissionaudit', + name='transmission_status', + ) + ) + except FieldDoesNotExist: + pass + + try: + if BlackboardLearnerAssessmentDataTransmissionAudit._meta.get_field('transmission_status'): + operations.append( + migrations.RemoveField( + model_name='blackboardlearnerassessmentdatatransmissionaudit', + name='transmission_status', + ) + ) + except FieldDoesNotExist: + pass diff --git a/integrated_channels/canvas/migrations/0034_auto_20240118_1822.py b/integrated_channels/canvas/migrations/0034_auto_20240118_1822.py new file mode 100644 index 0000000000..ecd564e8cc --- /dev/null +++ b/integrated_channels/canvas/migrations/0034_auto_20240118_1822.py @@ -0,0 +1,21 @@ +# Generated by Django 3.2.22 on 2024-01-18 18:22 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('canvas', '0033_auto_20240110_1301'), + ] + + operations = [ + migrations.RemoveField( + model_name='canvaslearnerassessmentdatatransmissionaudit', + name='transmission_status', + ), + migrations.RemoveField( + model_name='canvaslearnerdatatransmissionaudit', + name='transmission_status', + ), + ] diff --git a/integrated_channels/cornerstone/migrations/0033_remove_cornerstonelearnerdatatransmissionaudit_transmission_status.py b/integrated_channels/cornerstone/migrations/0033_remove_cornerstonelearnerdatatransmissionaudit_transmission_status.py new file mode 100644 index 0000000000..5e7601bab5 --- /dev/null +++ b/integrated_channels/cornerstone/migrations/0033_remove_cornerstonelearnerdatatransmissionaudit_transmission_status.py @@ -0,0 +1,17 @@ +# Generated by Django 3.2.22 on 2024-01-18 18:22 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('cornerstone', '0032_cornerstonelearnerdatatransmissionaudit_transmission_status'), + ] + + operations = [ + migrations.RemoveField( + model_name='cornerstonelearnerdatatransmissionaudit', + name='transmission_status', + ), + ] diff --git a/integrated_channels/degreed/migrations/0033_remove_degreedlearnerdatatransmissionaudit_transmission_status.py b/integrated_channels/degreed/migrations/0033_remove_degreedlearnerdatatransmissionaudit_transmission_status.py new file mode 100644 index 0000000000..5ef0c84cc2 --- /dev/null +++ b/integrated_channels/degreed/migrations/0033_remove_degreedlearnerdatatransmissionaudit_transmission_status.py @@ -0,0 +1,17 @@ +# Generated by Django 3.2.22 on 2024-01-18 18:22 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('degreed', '0032_degreedlearnerdatatransmissionaudit_transmission_status'), + ] + + operations = [ + migrations.RemoveField( + model_name='degreedlearnerdatatransmissionaudit', + name='transmission_status', + ), + ] diff --git a/integrated_channels/degreed2/migrations/0025_remove_degreed2learnerdatatransmissionaudit_transmission_status.py b/integrated_channels/degreed2/migrations/0025_remove_degreed2learnerdatatransmissionaudit_transmission_status.py new file mode 100644 index 0000000000..c4b7665a76 --- /dev/null +++ b/integrated_channels/degreed2/migrations/0025_remove_degreed2learnerdatatransmissionaudit_transmission_status.py @@ -0,0 +1,17 @@ +# Generated by Django 3.2.22 on 2024-01-18 18:22 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('degreed2', '0024_degreed2learnerdatatransmissionaudit_transmission_status'), + ] + + operations = [ + migrations.RemoveField( + model_name='degreed2learnerdatatransmissionaudit', + name='transmission_status', + ), + ] diff --git a/integrated_channels/integrated_channel/migrations/0032_remove_genericlearnerdatatransmissionaudit_transmission_status.py b/integrated_channels/integrated_channel/migrations/0032_remove_genericlearnerdatatransmissionaudit_transmission_status.py new file mode 100644 index 0000000000..b4178daf9f --- /dev/null +++ b/integrated_channels/integrated_channel/migrations/0032_remove_genericlearnerdatatransmissionaudit_transmission_status.py @@ -0,0 +1,17 @@ +# Generated by Django 3.2.22 on 2024-01-18 18:22 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('integrated_channel', '0031_genericlearnerdatatransmissionaudit_transmission_status'), + ] + + operations = [ + migrations.RemoveField( + model_name='genericlearnerdatatransmissionaudit', + name='transmission_status', + ), + ] diff --git a/integrated_channels/integrated_channel/models.py b/integrated_channels/integrated_channel/models.py index cc0d60fc64..eecfa7c1f8 100644 --- a/integrated_channels/integrated_channel/models.py +++ b/integrated_channels/integrated_channel/models.py @@ -533,8 +533,6 @@ class LearnerDataTransmissionAudit(TimeStampedModel): help_text=_('Data pertaining to the transmissions API request response.') ) - transmission_status = models.JSONField(default=list, blank=True, null=True) - class Meta: abstract = True app_label = 'integrated_channel' @@ -607,20 +605,6 @@ def _payload_data(self): 'grade': self.grade, } - def add_transmission_status(self, status_code, error_message): - """ - Append the new entry to the list, keeping the list limited to latest three entries. - """ - new_entry = { - 'timestamp': timezone.now().isoformat(), - 'Status_code': status_code, - 'error_message': error_message, - } - - self.transmission_status.append(new_entry) - - self.transmission_status = self.transmission_status[-TRANSMISSION_STATUS_RECORDS_LIMIT:] - class GenericLearnerDataTransmissionAudit(LearnerDataTransmissionAudit): """ diff --git a/integrated_channels/integrated_channel/transmitters/learner_data.py b/integrated_channels/integrated_channel/transmitters/learner_data.py index 23208d49ab..09398ce39b 100644 --- a/integrated_channels/integrated_channel/transmitters/learner_data.py +++ b/integrated_channels/integrated_channel/transmitters/learner_data.py @@ -380,7 +380,6 @@ def transmit(self, payload, **kwargs): # pylint: disable=arguments-differ was_successful = code < 300 learner_data.status = str(code) learner_data.error_message = body if not was_successful else '' - learner_data.add_transmission_status(learner_data.status, learner_data.error_message) learner_data.save() self.enterprise_configuration.update_learner_synced_at(action_happened_at, was_successful) diff --git a/integrated_channels/moodle/migrations/0033_remove_moodlelearnerdatatransmissionaudit_transmission_status.py b/integrated_channels/moodle/migrations/0033_remove_moodlelearnerdatatransmissionaudit_transmission_status.py new file mode 100644 index 0000000000..b366def996 --- /dev/null +++ b/integrated_channels/moodle/migrations/0033_remove_moodlelearnerdatatransmissionaudit_transmission_status.py @@ -0,0 +1,17 @@ +# Generated by Django 3.2.22 on 2024-01-18 18:22 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('moodle', '0032_auto_20240117_1202'), + ] + + operations = [ + migrations.RemoveField( + model_name='moodlelearnerdatatransmissionaudit', + name='transmission_status', + ), + ] diff --git a/integrated_channels/sap_success_factors/migrations/0016_remove_sapsuccessfactorslearnerdatatransmissionaudit_transmission_status.py b/integrated_channels/sap_success_factors/migrations/0016_remove_sapsuccessfactorslearnerdatatransmissionaudit_transmission_status.py new file mode 100644 index 0000000000..86efb5f1f4 --- /dev/null +++ b/integrated_channels/sap_success_factors/migrations/0016_remove_sapsuccessfactorslearnerdatatransmissionaudit_transmission_status.py @@ -0,0 +1,17 @@ +# Generated by Django 3.2.22 on 2024-01-18 18:22 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('sap_success_factors', '0015_sapsuccessfactorslearnerdatatransmissionaudit_transmission_status'), + ] + + operations = [ + migrations.RemoveField( + model_name='sapsuccessfactorslearnerdatatransmissionaudit', + name='transmission_status', + ), + ] diff --git a/integrated_channels/xapi/migrations/0013_remove_xapilearnerdatatransmissionaudit_transmission_status.py b/integrated_channels/xapi/migrations/0013_remove_xapilearnerdatatransmissionaudit_transmission_status.py new file mode 100644 index 0000000000..3ff94c76f5 --- /dev/null +++ b/integrated_channels/xapi/migrations/0013_remove_xapilearnerdatatransmissionaudit_transmission_status.py @@ -0,0 +1,17 @@ +# Generated by Django 3.2.22 on 2024-01-18 18:22 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('xapi', '0012_xapilearnerdatatransmissionaudit_transmission_status'), + ] + + operations = [ + migrations.RemoveField( + model_name='xapilearnerdatatransmissionaudit', + name='transmission_status', + ), + ] diff --git a/tests/test_integrated_channels/test_integrated_channel/test_transmitters/test_learner_data.py b/tests/test_integrated_channels/test_integrated_channel/test_transmitters/test_learner_data.py index a04161a274..5cf359f760 100644 --- a/tests/test_integrated_channels/test_integrated_channel/test_transmitters/test_learner_data.py +++ b/tests/test_integrated_channels/test_integrated_channel/test_transmitters/test_learner_data.py @@ -222,23 +222,3 @@ def test_learner_data_transmission_dry_run_mode(self, already_transmitted_mock, ) # with dry_run_mode_enabled = True we shouldn't be able to call this method assert not self.learner_transmitter.client.create_assessment_reporting.called - - def test_transmission_status_learner_data_transmission(self): - """ - Test that transmission status records three most recent status instances. - """ - self.create_course_completion_mock.return_value = 200, '' - - transmitter = learner_data.SapSuccessFactorsLearnerTransmitter(self.enterprise_config) - for _ in range(TRANSMISSION_STATUS_RECORDS_LIMIT + 1): - if _ == TRANSMISSION_STATUS_RECORDS_LIMIT: - self.create_course_completion_mock.return_value = 400, '{"error":{"code":null,"message":"Invalid value for property \'courseCompleted\'."}}' - transmitter.transmit(self.exporter([self.payload])) - actual_transmission_status = self.payload.transmission_status - - expected_transmission_status = [ - {'timestamp': mock.ANY, 'Status_code': '200', 'error_message': ''}, - {'timestamp': mock.ANY, 'Status_code': '200', 'error_message': ''}, - {'timestamp': mock.ANY, 'Status_code': '400', 'error_message': 'Client create_course_completion failed: {"error":{"code":null,"message":"Invalid value for property \'courseCompleted\'."}}'}, - ] - assert expected_transmission_status == actual_transmission_status From d25c3d0e4bef1126d6bb37cb07e9dff100ea393e Mon Sep 17 00:00:00 2001 From: John Nagro Date: Thu, 18 Jan 2024 20:29:20 +0000 Subject: [PATCH 2/2] fix: attempting to frame up a solution --- .../migrations/0018_auto_20240110_1301.py | 4 +- .../migrations/0019_auto_20240118_1822.py | 45 ++++++++----------- .../integrated_channel/models.py | 2 + 3 files changed, 23 insertions(+), 28 deletions(-) diff --git a/integrated_channels/blackboard/migrations/0018_auto_20240110_1301.py b/integrated_channels/blackboard/migrations/0018_auto_20240110_1301.py index 8065735497..24fbbe5f7c 100644 --- a/integrated_channels/blackboard/migrations/0018_auto_20240110_1301.py +++ b/integrated_channels/blackboard/migrations/0018_auto_20240110_1301.py @@ -24,4 +24,6 @@ class Migration(migrations.Migration): # ), # ] - operations = [] + operations = [ + migrations.RunPython(migrations.RunPython.noop, reverse_code=migrations.RunPython.noop) + ] diff --git a/integrated_channels/blackboard/migrations/0019_auto_20240118_1822.py b/integrated_channels/blackboard/migrations/0019_auto_20240118_1822.py index 21c352992e..03d8e73f19 100644 --- a/integrated_channels/blackboard/migrations/0019_auto_20240118_1822.py +++ b/integrated_channels/blackboard/migrations/0019_auto_20240118_1822.py @@ -1,8 +1,22 @@ # Generated by Django 3.2.22 on 2024-01-18 18:22 -from django.apps import apps from django.db import migrations from django.core.exceptions import FieldDoesNotExist +from integrated_channels.blackboard.models import * + + +def forwards_func(apps, schema_editor): + try: + if field := BlackboardLearnerDataTransmissionAudit._meta.get_field('transmission_status'): + schema_editor.remove_field(BlackboardLearnerDataTransmissionAudit, field) + except FieldDoesNotExist: + pass + + try: + if field := BlackboardLearnerAssessmentDataTransmissionAudit._meta.get_field('transmission_status'): + schema_editor.remove_field(BlackboardLearnerAssessmentDataTransmissionAudit, field) + except FieldDoesNotExist: + pass class Migration(migrations.Migration): @@ -16,29 +30,6 @@ class Migration(migrations.Migration): # this migration conditionally cleans up the previous migration operations # only if they were part of the partially applied ones - BlackboardLearnerAssessmentDataTransmissionAudit = apps.get_model('blackboard', 'BlackboardLearnerAssessmentDataTransmissionAudit') - BlackboardLearnerDataTransmissionAudit = apps.get_model('blackboard', 'BlackboardLearnerDataTransmissionAudit') - - operations = [] - - try: - if BlackboardLearnerDataTransmissionAudit._meta.get_field('transmission_status'): - operations.append( - migrations.RemoveField( - model_name='blackboardlearnerdatatransmissionaudit', - name='transmission_status', - ) - ) - except FieldDoesNotExist: - pass - - try: - if BlackboardLearnerAssessmentDataTransmissionAudit._meta.get_field('transmission_status'): - operations.append( - migrations.RemoveField( - model_name='blackboardlearnerassessmentdatatransmissionaudit', - name='transmission_status', - ) - ) - except FieldDoesNotExist: - pass + operations = [ + migrations.RunPython(forwards_func, reverse_code=migrations.RunPython.noop) + ] diff --git a/integrated_channels/integrated_channel/models.py b/integrated_channels/integrated_channel/models.py index eecfa7c1f8..e3e585246e 100644 --- a/integrated_channels/integrated_channel/models.py +++ b/integrated_channels/integrated_channel/models.py @@ -533,6 +533,8 @@ class LearnerDataTransmissionAudit(TimeStampedModel): help_text=_('Data pertaining to the transmissions API request response.') ) + transmission_status = models.JSONField(default=list, blank=True, null=True) + class Meta: abstract = True app_label = 'integrated_channel'