feat: standardize bulk-revoke endpoint

license_manager/apps/api/v1/tests/test_views.py

@@ -2930,9 +2930,8 @@ def test_bulk_revoke_happy_path(self, mock_revoke_license, mock_execute_post_rev
         mock_revoke_license.side_effect = [revoke_alice_license_result, revoke_bob_license_result]
 
         response = self.api_client.post(self.bulk_revoke_license_url, request_payload)
-        assert response.status_code == status.HTTP_200_OK
-        assert response.json()['revocation_results'][0]['user_email'] == '[email protected]'
-        assert response.json()['error_messages'] == []
+
+        assert response.status_code == status.HTTP_207_MULTI_STATUS
         # Since alice has multiple licenses, we should only revoke her assigned one.
         mock_revoke_license.assert_has_calls([
             mock.call(alice_assigned_license),
@@ -2981,7 +2980,7 @@ def test_bulk_revoke_set_custom_tags(
         }
         # Verify that set_tags util was called with right arguments
         mock_set_tags_util.assert_called_with(tags_dict)
-        assert response.status_code == status.HTTP_200_OK
+        assert response.status_code == status.HTTP_207_MULTI_STATUS
 
     @mock.patch('license_manager.apps.api.v1.views.execute_post_revocation_tasks')
     @mock.patch('license_manager.apps.api.v1.views.revoke_license')
@@ -3013,7 +3012,7 @@ def test_bulk_revoke_multiple_activated_same_email(self, mock_revoke_license, mo
 
         response = self.api_client.post(self.bulk_revoke_license_url, request_payload)
 
-        assert response.status_code == status.HTTP_200_OK
+        assert response.status_code == status.HTTP_207_MULTI_STATUS
         # Since alice has multiple licenses, we should only revoke the first one (which is arbitrarily
         # the one with the smallest uuid).
         mock_revoke_license.assert_has_calls([
@@ -3057,7 +3056,7 @@ def test_bulk_revoke_with_filters_happy_path(
         }
 
         response = self.api_client.post(self.bulk_revoke_license_url, request_payload)
-        assert response.status_code == status.HTTP_200_OK
+        assert response.status_code == status.HTTP_207_MULTI_STATUS
 
         revoked_emails = [call_arg[0][0].user_email for call_arg in mock_revoke_license.call_args_list]
         assert sorted(revoked_emails) == expected_revoked_emails
@@ -3112,7 +3111,7 @@ def test_bulk_revoke_no_valid_subscription_plan_superuser(self, mock_revoke_lice
         response = self.api_client.post(request_url, request_payload)
 
         assert response.status_code == status.HTTP_404_NOT_FOUND
-        expected_response_message = 'No SubscriptionPlan identified by {} exists'.format(non_existent_uuid)
+        expected_response_message = {'error_messages': [{'error': 'No SubscriptionPlan identified by {} exists'.format(non_existent_uuid)}]}
         self.assertEqual(expected_response_message, response.json())
         self.assertFalse(mock_revoke_license.called)
 
@@ -3143,7 +3142,7 @@ def test_bulk_revoke_not_enough_revocations_remaining(self, mock_revoke_license)
         response = self.api_client.post(request_url, request_payload)
 
         assert response.status_code == status.HTTP_400_BAD_REQUEST
-        expected_response_message = 'Plan does not have enough revocations remaining.'
+        expected_response_message = {'error_messages': [{'error': 'Plan does not have enough revocations remaining.'}]}
         self.assertEqual(expected_response_message, response.json())
         self.assertFalse(mock_revoke_license.called)
 
@@ -3166,27 +3165,26 @@ def test_bulk_revoke_license_not_found(self, mock_revoke_license, mock_execute_p
             'user_emails': [
                 '[email protected]',
                 '[email protected]',  # There's no license for bob
-                '[email protected]',  # There's no license for john
             ],
         }
-        response = self.api_client.post(
-            self.bulk_revoke_license_url, request_payload)
-        assert response.status_code == status.HTTP_200_OK
+        response = self.api_client.post(self.bulk_revoke_license_url, request_payload)
+
+        assert response.status_code == status.HTTP_207_MULTI_STATUS
         expected_error_msg = (
-            "No license for email {} exists in plan "
-            "{} with a status in ['activated', 'assigned']. user_email: {}"
+            "No license for email [email protected] exists in plan "
+            "{} with a status in ['activated', 'assigned']. user_email: [email protected]".format(
+                self.subscription_plan.uuid)
         )
-        self.assertEqual(expected_error_msg.format(
-            '[email protected]',
-            self.subscription_plan.uuid,
-            '[email protected]'
-        ), response.json()
-            ['error_messages'][0]['error'])
-        self.assertEqual(expected_error_msg.format(
-            '[email protected]',
-            self.subscription_plan.uuid,
-            '[email protected]'
-        ), response.json()['error_messages'][1]['error'])
+        response_data = response.json()
+        self.assertIn('revocation_results', response_data)
+        self.assertIn('error_messages', response_data)
+
+        self.assertEqual(len(response_data['revocation_results']), 1)
+        self.assertIsInstance(response_data['revocation_results'][0]['user_email'], str)
+
+        self.assertEqual(len(response_data['error_messages']), 1)
+        self.assertIsInstance(response_data['error_messages'][0]['user_email'], str)
+        self.assertEqual(response_data['error_messages'][0]['error'], expected_error_msg)
         mock_revoke_license.assert_called_once_with(alice_license)
 
     @mock.patch('license_manager.apps.api.v1.views.revoke_license')
@@ -3213,11 +3211,15 @@ def test_bulk_revoke_license_revocation_error(self, mock_revoke_license):
         response = self.api_client.post(self.bulk_revoke_license_url, request_payload)
 
         assert response.status_code == status.HTTP_400_BAD_REQUEST
-        expected_error_msg = "Action: license revocation failed for license: {} because: {}".format(
-            alice_license.uuid,
-            'floor is lava. user_email: [email protected]',
-        )
-        self.assertEqual(expected_error_msg, response.json()['error_messages'][0]['error'])
+        expected_error_msg = {'error_messages': [{
+            'error': "Action: license revocation failed for license: {} because: {}".format(
+                alice_license.uuid,
+                'floor is lava. user_email: [email protected]',
+            ),
+            'error_response_status': 400,
+            'user_email': '[email protected]'
+        }]}
+        self.assertEqual(expected_error_msg, response.json())
         mock_revoke_license.assert_called_once_with(alice_license)
 
     def test_revoke_all_no_valid_subscription_plan_superuser(self):
@@ -3293,7 +3295,7 @@ def test_assign_after_license_revoke_end_to_end(
         self.subscription_plan.save()
 
         response = self.api_client.post(self.bulk_revoke_license_url, {'user_emails': [self.test_email]})
-        assert response.status_code == status.HTTP_200_OK
+        assert response.status_code == status.HTTP_207_MULTI_STATUS
         mock_revoke_course_enrollments_for_user_task.assert_called()
         if is_revocation_cap_enabled:
             mock_send_revocation_cap_notification_email_task.assert_called_with(
@@ -3354,7 +3356,7 @@ def test_revoke_total_and_allocated_count_end_to_end(
 
         # Revoke the activated license and verify the counts change appropriately
         revoke_response = self.api_client.post(self.bulk_revoke_license_url, {'user_emails': [self.test_email]})
-        assert revoke_response.status_code == status.HTTP_200_OK
+        assert revoke_response.status_code == status.HTTP_207_MULTI_STATUS
         mock_revoke_course_enrollments_for_user_task.assert_called()
 
         second_detail_response = _subscriptions_detail_request(

license_manager/apps/api/v1/views.py

@@ -1358,9 +1358,10 @@ def bulk_revoke(self, request, subscription_uuid=None):
 
         if len(user_emails) > subscription_plan.num_revocations_remaining:
             error_message = 'Plan does not have enough revocations remaining.'
-            return Response({'error_messages': [
-                {'error': error_message}
-            ]
+            return Response({
+                'error_messages': [
+                    {'error': error_message}
+                ]
             }, status=status.HTTP_400_BAD_REQUEST)
 
         revocation_results = []
@@ -1389,7 +1390,9 @@ def bulk_revoke(self, request, subscription_uuid=None):
         if error_response_status and not revocation_results:
             return Response({
                 'error_messages': error_messages
-            }, status=error_response_status)
+            },
+                status=error_response_status
+            )
 
         # Case 2: if all or few revocations succeded; return error messages list & the succeeeded revocations list
         revocation_succeeded = []