feat: add generate report button in admin

[Alec4r]

Description:

This PR adds a new field to SurveyReport model and adds add a button in the django model admin to generate a new report.

SurveyReport model (#31183).
SurveyReport Admin Model (#31323).

ISSUE:
openedx/axim-engineering#349

APPROVED DESIGN:
https://docs.google.com/document/d/1tm1O5BQsAwekphI5IN9pBn8behQDZigXePLGqCl5he8

DECISIONS:

• We decided add a new celery task to generate the report async, because we won't want block the user experience when this decide generate a new report and the generating time depends of how much information the user have.

How to test

With a dev environment running, you should go to the django admin /admin/survey_report/surveyreport/ and use the button "Generate Report" to generate a new survey report.

You should see a new report with state "Processing" if you reload the page so few seconds later you should see the same report with state "Generated" or "Error".

[openedx-webhooks]

Thanks for the pull request, @Alec4r! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[Alec4r]

@jalondonot @felipemontoya @ormsbee @jmakowski1123 @mariajgrimaldi We would appreciated your comments in this PR.

[ormsbee]

I think I get the intention here, but I believe you'd be better off writing to the model only from the async task instead of writing the skeleton entry in the view and filling in the pieces in the celery task. You can save multiple times within that task if you want to show incremental progress. But having generate_reports take a boolean flag that really makes it do no querying is confusing, and in rare cases this split can cause weird race condition issues. For example:

If the celery task starts before the view returns (because of some randomly slow middleware), it's possible that it could start without seeing the survey report referenced by survey_report_id, particularly if the celery worker isn't properly configured and is accessing MySQL using "repeatable read" as its transaction isolation level. You then enter this weird state where the celery worker can't read the row (it didn't exist at the start of its transaction), but you can't write with that row's primary key because that causes an integrity error.

FWIW, I am not creative enough to come up with that example on my own. it isn't likely, but I've seen it happen, and it's really obnoxious to debug when it does.

[felipemontoya]

I think I have an example of this. It involves an exception instead of a simple slow middleware, but I think it holds:

survey_report_id = generate_report(defaults=True)

    # SQL: with transaction;
    # SQL: insert `value` in generated_reports;

generate_survey_report.delay(survey_report_id) # -> new thread in a different server
return redirect("admin:survey_report_surveyreport_changelist")

    middleware 1
    middleware 2 -> exception
    middleware 3

    ## somewhere at the end of the middlewares
        if not exception:
            # SQL: commit;
        else:
            # SQL: rollback;

In the async server, since the connection is different we have no context of the transaction and the ID might not even exist.

[Alec4r]

@ormsbee I've already moved all to the generate_report method, what do you think? the we will have the same problem with these changes or are enough?

[ormsbee]

Yeah, I think it's fine now, thank you. Please just squash your commits and add the context from your PR message to your commit message. Thank you!

[mphilbrick211]

Hi @ormsbee - flagging the question for you above.

[ProductRyan]

@jmakowski1123 @Alec4r - does this survey have a method for opt-out?

[ormsbee]

@ProductRyan: Do you mean the instance opting out (which is the default, since this needs to be manually run), or do you mean individual courses/orgs opting out of having their stats collected in the aggregate counts of enrollments, etc.?

[felipemontoya]

Coming back to this I think we are ready to move forward.

There are some final touches in the way se send the report to the API, but nothing there changes the generation of the report itself.

[felipemontoya]

I was seeing this more from the code perspective. Sorry I missed your comment @ProductRyan.

As Dave said right now the generation of the survey needs to be run manually. We will work on a way to make it easy to program with a frequency of a couple times a year. In that work one of the hard requirements is that we make it easy for site operators to opt out. So definitely yes, opting out will be present and easy.

[felipemontoya]

@ProductRyan do you still have a blocking concern over this PR? I'd like to merge this PR tomorrow before it gets stalled. Please let me know if you need us to hold it for a while.

[ProductRyan]

@felipemontoya reached out tome to confirm that there is a method for instances to opt-out.

@mphilbrick211 I approve from a Product POV.

[openedx-webhooks]

@Alec4r 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.