[BB-6825] Add option to backfill tabs without publish

[pkulkark]

Description

This PR adds an option to the backfill_course_tabs management command to skip course publish. Currently, running the command triggers course publish for every single course. This could be a very resource-heavy operation in case there are a lot of older courses. The changes introduced here, updates the default tabs in the CourseBlock object and exits without publishing the course. It also updates the CourseOverview object.
Note: This change is only needed for the nutmeg release since Dates tab is still included. For olive, the Dates tab would be completely dropped.

Supporting information

• Sandbox URL: https://maple-nutmeg-dates.opencraft.hosting/ (you'll need django shell access)
• Private ref: Jira ticket - BB-6825

Testing instructions

1. Deploy this branch on your devstack or use the sandbox mentioned above for testing.
2. Import a course to studio from the previous release (i.e. maple).
3. Verify that the "Dates" tab is not visible in the course page in LMS.
4. Run the management command to backfill default course tabs with the option to skip course publish, as shown below:

python manage.py cms backfill_course_tabs --no-publish

5. Verify that the "Dates" tab is now visible in the course page in LMS and that no course publish was triggered.

Reviewers

• @kaustavb12

[kaustavb12]

@pkulkark The changes overall looks good to me. But I have a few concerns with the current approach.

1. I did not realize this earlier that the tab changes are not persisted in the CourseBlock. What are the implications of this, especially in terms of persistence? Will the tab information be persisted when doing a new deployment or during release upgrade ?
2. I already verified that course re-runs do not contain the tab changes and so this command needs to be run everytime we have course reruns. This means this PR cannot be a temporary change, which can be dropped during the Olive release. We would have to maintain this change through the subsequent releases, as I don't think this can be upstreamed.

I was exploring and testing some additional approaches.

The store.update_item() calls this method

Here bulk_operations method is called, which triggers the course_published signal. However, these signals can be suppressed here with emit_signals=False.

What if we add an additional keyword argument no_signals=False here and set emit_signals to False, if no_signals is True. Here's what the changed method will look like:

    def update_item(self, descriptor, user_id, allow_not_found=False, force=False, asides=None, no_signals=False, **kwargs):  # lint-amnesty, pylint: disable=arguments-differ
        old_descriptor_locn = descriptor.location
        descriptor.location = self._map_revision_to_branch(old_descriptor_locn)
        emit_signals = descriptor.location.branch == ModuleStoreEnum.BranchName.published \
            or descriptor.location.block_type in DIRECT_ONLY_CATEGORIES

        if no_signals:
            emit_signals=False

        with self.bulk_operations(descriptor.location.course_key, emit_signals=emit_signals):
            item = super().update_item(
                descriptor,
                user_id,
                allow_not_found=allow_not_found,
                force=force,
                asides=asides,
                **kwargs
            )
            self._auto_publish_no_children(item.location, item.location.block_type, user_id, **kwargs)
            descriptor.location = old_descriptor_locn
            return item

What do you think about this approach ? Do you forsee any unwanted side-effects of this approach ?

This approach would persist the tab information in the CourseBlock without triggering the course_published signal. Also, once we run this command for all our clients, we can drop this change during the next release branch preparation and would not have to keep maintaining it.

[kaustavb12]

I don't fully understand this change. Why was this change necessary ?

[pkulkark]

This was to fix the failing CI tests.

[pkulkark]

@kaustavb12

I did not realize this earlier that the tab changes are not persisted in the CourseBlock. What are the implications of this, especially in terms of persistence? Will the tab information be persisted when doing a new deployment or during release upgrade?

No the tab information will not be persisted across new deployments or release upgrades. Basically this does not eliminate the need for running the backfill_course_tabs command, but rather adds an option to make it less resource-heavy. So we will still require to run the command in all the scenarios as before. Perhaps we can add it in the deployment scripts behind a flag, to ensure it's run after each deployment? What do you think?

I already verified that course re-runs do not contain the tab changes and so this command needs to be run everytime we have course reruns. This means this PR cannot be a temporary change, which can be dropped during the Olive release. We would have to maintain this change through the subsequent releases, as I don't think this can be upstreamed.

As we discovered in BB-6763, the dates tab is not actually related to the legacy learning experience, but rather depends on whether the course was created before nutmeg. So we would have to run this command irrespective of the release, if we plan to continue supporting older courses across releases. One option could be to setup cron jobs to run this command (with the --no-publish option of course) often on the instances. We could also try to upstream this, since we're not changing the original command but just adding an optional flag. I don't know if it will be accepted though.

Here bulk_operations method is called, which triggers the course_published signal. However, these signals can be suppressed here with emit_signals=False.

Oh wow! I did not know that option existed.

What do you think about this approach ? Do you forsee any unwanted side-effects of this approach ?
This approach would persist the tab information in the CourseBlock without triggering the course_published signal. Also, once we run this command for all our clients, we can drop this change during the next release branch preparation and would not have to keep maintaining it.

The only problem I see with this approach, is that the CourseOverview wouldn't get updated with the new tabs information. I don't quite know what effects that might have though - I don't know if CourseOverview is used anywhere to fetch tabs information.

[kaustavb12]

@pkulkark

The only problem I see with this approach, is that the CourseOverview wouldn't get updated with the new tabs information. I don't quite know what effects that might have though - I don't know if CourseOverview is used anywhere to fetch tabs information.

We could just call CourseOverview.load_from_module_store after updating the modulestore with the course with updated tab information, if the no-publish flag is present, just like we are doing currently.

In fact, we would not need to the changes in the CourseOverview model either, as we could call load_from_module_store, since the content block in the module store will now have the updated tabs information.

[pkulkark]

@kaustavb12 Makes sense. I'll make the required changes and let you know.

[kaustavb12]

👍

• I tested this: I tested the changes in devstack as well as in sandbox
• I read through the code
• I checked for accessibility issues
• Includes documentation
• I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.