Serialize CompletedWork flags in CompletedWorkSerializer

[Charl1996]

Product Description

No user-facing changes, only API data update.

Technical Summary

Ticket

This PR makes sure to serialize the CompletedWork records' flags as part of the CompletedWorkSerializer in a dictionary style with slug as key and description as value. For example, the jsonified response would look like this:

{
  ...
  "deliveries": [
    {
      ...
      "flags": {
          "duration": "The form was completed too quickly."
        }
    }
  ]
}

Additional (unexpected) update

The test_delivery_progress_endpoint test asserts that the serialized delivery keys be similar to the UserVisitSerializer, but that's not the correct serializer class that's being used in the get_deliveries method of the DeliveryProgressSerializer.

I have updated the test to test against the CompletedWorkSerializer, but was just wondering if this was a previous oversight?

Safety Assurance

Safety story

Updated test_delivery_progress_endpoint test to reflect new changes.

Automated test coverage

No new tests, only updated an existing test

QA Plan

QA not planned

Labels & Review

• The set of people pinged as reviewers is appropriate for the level of risk of the change

[Charl1996]

I think I'm going to try Copilot for reviewing this quickly.

[Copilot]

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

commcare_connect/opportunity/api/serializers.py:271

• There is an assumption that 'visit' is a dictionary containing a 'flags' key. Consider adding a type check or exception handling to ensure that if 'visit' is not a dict, the code fails gracefully.

for slug, reason in visit.get("flags", []):

[calellowitz]

looks great. left a comment on the test change, also good to add a test that explicitly checks the correctness of the new field (that the flags get added in the right format)

[calellowitz]

This is a good catch. It was orginally like that because we switched serializers and wanted to ensure compatibility, but I agree that now that we are making additions, we should use the correct one. That said, I am not sure this assertion is buying us much, and would prefer the test to check expected values of keys we care about in the fields rather than just the presence of the keys.

[Charl1996]

True true. I'll update!

[Charl1996]

Now that I think about it, is this the expected format? Would it not make more sense to have it like this:

[
    {
        "duration": "The form was completed too quickly.",
        "attachment_missing": "Form was submitted without attachments.",
    },
]

[Charl1996]

@calellowitz Actually, I'm going to update it as above. Feel free to let me know if that's not in fact what was envisioned. So now a single visit's flags will be consolidated into this one dictionary structure.

See c384c9e

[calellowitz]

Oops yes. This is definitely what was intended. Thanks for the fix (and one more benefit of tests, making this stuff explicit).

[calellowitz]

Actually, shouldn't this not be a list at all. If this is always a list with a single dict, I think it can just be a dict.

[Charl1996]

26b953d