Add Submission Reviews

[moshthepitt]

Submission Reviews

This is a new feature that will allow admins to approve and/or reject submissions made to any of their forms.

This document describes the structure and implementation of a minimal submission review feature and process for Ona.io. That is, it represents the minimum that has to be done in order to support submission reviews for our most immediate use-case - the ILRI Kaznet project.

The general idea is that this is an extensible starting point for submission reviews in Ona - much more can be done to make it more feature-rich and useful for all our clients, but that will be done at a later time.

Structure of a submission review

A submission review has two parts:

Review status

The admin chooses from a number of review statuses:

• Approved - the submission is approved

• Rejected - the submission is rejected (if this status is chosen then the admin must write a review comment that will explain why the submission is rejected)

• Pending Review - the default status. It means that the admin has not yet started a submission review or is still in the review process.

Review comment

This is an optional comment about the submission. It is mandatory to provide a comment if a submission status is Rejected. You can also make a comment if Approved.

Who can do a submission review?

Users who are admins (i.e. have the owner role) of a form can review submissions made to that form.

Can a submission be reviewed more than once?

Yes. A submission review history will be maintained. The submission review status will be that of the very last submission review made. e.g. if there are two reviews, one approving and one rejecting, and the one rejecting is the last one made then the status of the submission will be rejected.

Can a submission reviews be edited?

Yes. The user who created it can edit it. This may have the effect of changing the submission review status.

Can submission reviews be deleted?

Yes. Admins can delete submission reviews. This may have the effect of changing the submission review status.

Changes to the Onadata API

In order to support the submission review feature, the Onadata API will need to change in the following ways:

1. Support for submission reviews

The API will need to accept and store submission reviews for each submission. The submission review will have the following fields:

• the submission id
• the user who is doing the review
• the date and time of the review
• the review status
• the review comment

Onadata already has various things that may be used to support the above. This includes submission Notes and Tags.

2. Support for filtering submissions

The API will need to support filtering a form's submissions by the review status.

3. Support for filtering exports

The API will need to support generation of exports filtered by the review status.

Notification to data collection agents

Once a submission has been reviewed, a notification should be sent back to the person who submitted the review. This will be done through the messaging application.

---

This is based on the discussion here: https://github.com/onaio/zebra/issues/5363

Aha! Link: https://ona.aha.io/features/PROD-339

[moshthepitt]

Implementation Plan A

We need the following:

• a review status for each Instance object
• an optional review comment for each Instance object
• a review history for each Instance object. i.e. we should keep a record of all reviews made to an Instance object.

Additionally, a complete review of an Instance is considered to be composed of
two parts:

1. A review status
2. A review comment

Existing Functionality

Currently we have Note objects (onadata.apps.logger.models.Note) which have the following fields:

class Note(models.Model):
    note = models.TextField()
    instance = models.ForeignKey(
        'logger.Instance', related_name='notes')
    instance_field = models.TextField(null=True, blank=True)
    created_by = models.ForeignKey(settings.AUTH_USER_MODEL, null=True,
                                   blank=True)
    date_created = models.DateTimeField(auto_now_add=True)
    date_modified = models.DateTimeField(auto_now=True)

Note objects can be used to add comments to Instance objects, and also to individual fields on an Instance object.

I think we should keep using Notes to provide the review comment part of our Submission review needs.

Why?

• Note objects already exist and have been used elsewhere
• They conveniently provide what we need as far as review comments are concerned
• Using Note objects allows us the flexibility to add "per-question" reviews on Instances

The Plan

1. Add a SubmissionReview model

class SubmissionReview(models.Model):
    """
    Model class for Submission Reviews
    """

    APPROVED = '1'
    REJECTED = '2'
    PENDING = '3'

    STATUS_CHOICES = (
        (APPROVED, 'Approved'),
        (PENDING, 'Pending Review'),
        (REJECTED, 'Rejected')
    )

    instance = models.ForeignKey('logger.Instance', related_name='reviews',
                                 on_delete=models.CASCADE)
    note = models.ForeignKey('logger.Note', related_name='notes', blank=True,
                             null=True, default=None, on_delete=models.SET_NULL)
    status = models.CharField('Status', max_length=1, choices=STATUS_CHOICES,
                              default=PENDING, db_index=True)
    created_by = models.ForeignKey(settings.AUTH_USER_MODEL, null=True,
                                   blank=True, on_delete=models.CASCADE)
    date_created = models.DateTimeField(auto_now_add=True)
    date_modified = models.DateTimeField(auto_now=True)

This model holds the complete information of a review of a single Instance done by a certain user at a certain time.

It references a Note object to hold the optional review comment.

2. Add a review_status field to Instance model

    review_status = models.CharField(
        'Status', max_length=1, choices=SubmissionReview.STATUS_CHOICES,
        default=SubmissionReview.PENDING, db_index=True)

This field is used to hold the current review status of the Instance. It should be updated to match the status of the very last SubmissionReview that references the Instance in question.

Additionally expose review_status as a field on Instance serializer(s) and make it possible to filter the Instance API endpoints using review_status.

Why add a field to Instance?

Adding a field is better than any other solution because:

• it makes it easier to do queries against the field
• I feel that review_status is a legitimate first class field on the Instance model

3. An API endpoint for SubmissionReview objects

Add both a SubmissionReview serializer and SubmissionReview viewset that allow admins to perform CRUD tasks on SubmissionReviews.

All other user roles can only read SubmissionReview objects.

The SubmissionReview serializer fields will be:

• instance - the Instance
• user - the User
• date_created - the datetime of creation
• date_modified - the datetime of last modification
• status - the review status
• note - the text of the related Note object

For the SubmissionReview viewset, it will be possible to filter by instance, user and status.

When a SubmissionReview object is created, it should update the review_status field on the related Instance object.

Only admins can Create, Update or Delete SubmissionReview objects.

4. Notifications

Whenever a new SubmissionReview object is created, a message should be sent through the Onadata messaging (onadata.apps.messaging) application.

The message is sent to the user and its content is the serialized SubmissionReview object.

[pld]

I think we should consider alternative approaches as well, here are two:

Use an additional meta column on the XForm

• Add a _review_status metadata field
• Use notes with linked to the submission with _review_status in instance_field to determine that they're the comments
• You'd get versioning via the submission history
• You'd get filtering etc. with filtered viewsets -- this is pretty important regardless of implementation given the plan to handle filtering of views and exports via filtered datasets
• You wouldn't need more (any?) changes to the API

Use MetaData

• add a metadata_set (like we have on XForms already) to instances and use this instead of a new SubmissionReview model
  • use data_type of submission_review and data_value of an enum that is the current review status
  • sort by data_created per instance to get the current review, and keep the history

[ukanga]

You'd get versioning via the submission history

I don't think this is true, submission history get's updated only when the XML submission changes, not when any field in the Instance model changes.

[ukanga]

With the addition of a Submission Review model, I don't think we will need to use the note's model; we should use a text field to store the review comments. Otherwise, I feel there will be the need to differentiate between a regular comment/note with a review comment/note on the notes endpoint. How do you propose to show that?

[ukanga]

How do we address the issue about which forms will have a review status, which ones would not? Or is it something that will be enabled by default for all forms? I do recall that this is only enabled on specific forms.

[ukanga]

I am more in favour of the separate model - could be a separate app - that provides the functionality.

[moshthepitt]

I still feel that going with a separate model is a cleaner and probably better way to do this.

I do agree with @ukanga that we probably do not need the Notes model if we do it that way.

[pld]

You and Dickson should provide evidence and reasoning for those opinions

[moshthepitt]

Competing implementation plans

Implementation Plan A

• Add a review_status field on Instance objects that holds the current review status
• Add a submission_review model that holds the submission details:
  • a pointer to the Instance
  • a pointer to a Note object (for the comment)
  • the review status

Sample query to get approved instances

SELECT "logger_instance"."id" FROM "logger_instance" WHERE "logger_instance"."review_status" = "approved"

• not necessarily 100% accurate

Why?

1. ...

Why not?

1. Adds a lot of new stuff to the database
2. Adds much more code than the other implementation plans

Implementation Plan B

• Add a _review_status metadata field
• Use Note objects with linked to the submission with _review_status in instance_field to determine that they're the instance review comments
• Modify the way submission history works so that we get submission reviews as part of that history

Sample query to get approved instances

SELECT "logger_instance"."id" FROM "logger_instance" WHERE ("logger_instance"."json" -> \'review_status\') = \'"approved"\'

• not necessarily 100% accurate

Why?

1. We do not have to modify the models or the database
2. We re-use already existing, tested code

Why not?

1. ...

Implementation Plan C

• Use the Metadata model (onadata.apps.main.models.MetaData) to store the submission reviews
• use data_type of "submission_review" and data_value of an enum that is the current review status
• sort by data_created per instance to get the current review, and keep the history

Sample query to get approved instances

SELECT "logger_instance"."id",
"main_metadata"."data_type",
"main_metadata"."data_value",
"main_metadata"."content_type",
"main_metadata"."object_id"
FROM "logger_instance"
LEFT JOIN "main_metadata" ON
"main_metadata"."object_id" = "logger_instance"."id" AND
"main_metadata"."content_type" = "the instance contenttype"
WHERE
"main_metadata"."data_type" = "submission_review" AND
"main_metadata"."data_value" = "approved"

• not necessarily 100% accurate

Why

1. We do not have to modify the models or the database
2. We re-use already existing, tested code

Why not?

1. Possibly "overloading" the Metadata model?
2. More complex queries

[pld]

B makes the most sense to me, this is the expected use-case for the Notes model and how we've used it for other projects

[Wambere]

I would say B as well for this particular case. But if we are ever to extend this feature then it might not be as efficient

[moshthepitt]

@Wambere what would make it less efficient?

[ukanga]

I will have to agree to B as well but Notes is the other model that we can use without creating a new one. the complexity is maintaining the _review_status fields between edits, which should not be that tough.

[moshthepitt]

@pld @ukanga @Wambere

I'm wondering how we'll tie a Note object to a particular review_status?

It is possible to have reviews that do not include comments. So, for example, we may have an Instance that has received three reviews as such:

• approved with no comment
• not approved with a comment
• approved with a comment

The final status of this Instance will be "approved" and we can tell that it has had two comments, but we cannot easily tell which particular review_status was paired with a particular comment.

Any ideas on how to do this? Would be be forced to add another field to the Instance metadata that identifies the Note object?

[ukanga]

I think in this case we can then simply create a new SubmissionReview model with the functionality we need. That way we can isolate the review behaviour and keep the Notes for general purposes.

[pld]

And the comments/notes would still be in the notes model?

[moshthepitt]

hmm, ok.

@ukanga @pld I'll have the SubmissionReview model hold the submission status and a relation to the associated Note object.

[pld]

That doesn't sound unreasonable