Add a management command to remove stale content types

[Kay]

This patch creates a new method remove_contenttypes that, when called from a migration, will programmatically remove the stale content type.

Diff: ​https://github.com/django/django/compare/master...Protosac:removalUtility

[Tim Graham]

Could you please send a pull request with tests and documentation? You may find the ​patch review checklist helpful.

[Kay]

Replying to timgraham:

Could you please send a pull request with tests and documentation? You may find the ​patch review checklist helpful.

I've looked the specs over and I'm not sure this warrants a test. The current Django tests for update_contenttypes already test for interactive. All this change does is automatically respond to it's prompt in the very same way the test_interactive_true does.

def test_interactive_true(self):
        """
        interactive mode of update_contenttypes() (the default) should delete
        stale contenttypes.
        """
        management.input = lambda x: force_str("yes")
        with captured_stdout() as stdout:
            management.update_contenttypes(self.app_config)
        self.assertIn("Deleting stale content type", stdout.getvalue())
        self.assertEqual(ContentType.objects.count(), self.before_count)

As for documentation I was considering placing it in the contenttypes.txt and adding a Management section to describe the force_remove argument and remove_contenttypes method. If that's an inappropriate place, please make a recommendation. Thanks again.

[Tim Graham]

Tests would ensure that the remove_contenttypes() function performs the job it's expected to do (deleting content types for the given app).

For docs, I'd suggest to add a "Utilities" section which documents remove_contenttypes(). No need to document the update_contenttypes() method -- that should remain a private function in my opinion.

[Markus Holtermann]

I think remove_contenttypes() should remain private as well. It's way too easy to loose data when using that function. I'd put the respective documentation for that method into the function's docstring.

[Tim Graham]

Markus, can you elaborate on the data loss concerns? I guess it's about the possibility of delete cascades which is why interactive prompts were originally added in #12339. I don't think there's much value in completing this ticket if it isn't a public API. I'd think documentation that explains the data-loss considerations would do more to educate users than keeping the function private would. If you have another idea on how to address the use case in #24820 I'm open to alternatives.

[Markus Holtermann]

Tim, neither patch is guaranteed to work when a user has not applied all contenttypes migrations, as the model used inside the update_contenttypes is from the global apps, not from the latest applied migration state (see #24100 for that).

Anyway, I'm ok with making it a public API if we put a .. warning:: box around the docs and state the potential data loss due to cascading.

[Tim Graham]

Markus and I chatted in IRC and came up with the idea of moving stale content types deletion out of the post_migrate signal update_contenttypes function and into a separate remove_stale_contenttypes management command. This would default to interactive=True but could be called with call_command('remove_stale_contenttypes', interactive=False) to delete without a prompt.

How does this sound?

[Tim Graham]

I started working on the management command described in the previous comment: ​PR

[Tim Graham]

I'd welcome someone continuing where I left off here.

[Amos Onn]

I think that the other side of not removing stale content types for fear of data loss is leaving stale GenericForeignKeys, which will break when accessed (see #25931). Hard to say which is worse.
I think perhaps adding a switch to migrate is better: --remove_stale_contenttypes or --keep_stale_contenttypes, and if none are present then default to current interactive behaviour.

[Tim Graham]

I doubt it's acceptable to add a contrib app specific option to migrate.

[Tim Graham]

I completed my ​old PR.

[Tim Graham <timograham@…>]

In 6a2af014:

Fixed #24865 -- Added remove_stale_contenttypes management command.

Thanks Simon Charette for the review.