Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(experimental) Allow using proxy models and change the pk with them. #345

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

PetrDlouhy
Copy link
Contributor

@PetrDlouhy PetrDlouhy commented Sep 30, 2021

I would like to use UUIDs instead of IDs as mentioned in #179.
Since I don't have UUID set as primary key in my app and it would be very hard and dangerous to change migrate it, I would like to change the PK somehow for django-comments-xtd.

I was able to achieve thet with few changes to django-comments-xtd (the for_concrete_model parameters can be made configurable from settings) and quite extensive hacking of the Proxy model and related objects:

from django_comments_xtd.models import XtdComment
from django.contrib.contenttypes.models import ContentType

from ..assets.models import Asset


class UUIDAsset(Asset):
    
    def _get_pk_val(self):
        return self.version_uuid

    class Meta:
        proxy = True


def uuid_get(*args, **kwargs):
    if 'pk' in kwargs:
        return UUIDAsset.objects._get(*args, version_uuid=kwargs['pk'])
    return UUIDAsset.objects._get(*args, **kwargs)
    

@property
def uuid_content_object(comment):
    if comment.content_type.model == 'uuidasset':
        asset_uuid = comment.object_pk
        return UUIDAsset.objects.get(version_uuid=asset_uuid)
    return comment._content_object
    
    
def uuid_get_object_for_this_type(content_type, **kwargs):
    if content_type.model == 'uuidasset':
        asset_uuid = kwargs['pk']
        return UUIDAsset.objects.get(version_uuid=asset_uuid)
    return content_type._get_object_for_this_type(**kwargs)
    
    
UUIDAsset.version_uuid.primary_key = True
UUIDAsset.objects._get = UUIDAsset.objects.get
UUIDAsset.objects.get = uuid_get
UUIDAsset._meta.pk = UUIDAsset._meta.get_field('asset_uuid')

XtdComment._content_object = XtdComment.content_object
XtdComment.content_object = uuid_content_object

ContentType._get_object_for_this_type = ContentType.get_object_for_this_type
ContentType.get_object_for_this_type = uuid_get_object_for_this_type

I am posting this here for further discussion.

I was even trying to make it that the user could set id_field for the model in COMMENTS_XTD_APP_MODEL_OPTIONS, which would be much easier the user to use, but also would require some of the hacking be inside of django-contribs-xtd (unfortunately there is no support for such things in django.contrib.contenttypes).

@PetrDlouhy
Copy link
Contributor Author

I have updated the code, so that it uses COMMENTS_FOR_CONCRETE_MODEL setting to pass to ContentType framework.
I made similar Pull request to django-contrib-comments, which is also needed to run hacks: django/django-contrib-comments#177

Updated hack code is here:

from django.contrib.contenttypes.models import ContentType
from django.db import models
from django_comments_xtd.models import XtdComment

from ..assets.models import Asset


class UUIDAssetQuerySet(models.query.QuerySet):
    def get(self, **kwargs):
        if 'pk' in kwargs:
            return super().get(asset_uuid=kwargs['pk'])
        return super().get(**kwargs)


class UUIDAsset(Asset):
    objects = models.Manager.from_queryset(UUIDAssetQuerySet)()

    def _get_pk_val(self):
        return self.asset_uuid

    class Meta:
        proxy = True
        app_label = 'assets'


def uuid_content_object(comment):
    if comment.content_type.model == 'uuidasset':
        asset_uuid = comment.object_pk
        return UUIDAsset.objects.get(asset_uuid=asset_uuid)
    return comment._content_object


_get_object_for_this_type = ContentType.get_object_for_this_type


def uuid_get_object_for_this_type(self: ContentType, **kwargs) -> models.Model:
    if self.model == 'uuidasset':
        asset_uuid = kwargs['pk']
        return UUIDAsset.objects.get(asset_uuid=asset_uuid)
    return _get_object_for_this_type(self, **kwargs)


UUIDAsset.asset_uuid.primary_key = True
UUIDAsset._meta.pk = UUIDAsset._meta.get_field('asset_uuid')

XtdComment._content_object_tmp = XtdComment.content_object
XtdComment.content_object = uuid_content_object

ContentType.get_object_for_this_type = uuid_get_object_for_this_type

@danirus
Copy link
Owner

danirus commented Oct 20, 2021

Thanks @PetrDlouhy, the PR code looks good (thanks for the additional fixes).
On the other hand the example code seems a bit like a hack, which is a pity given the achievement.
To display the full picture, could you please add the part of the code that belongs to the Asset model?
With a little bit of make-up it could be used as an example to add to the use cases page of the docs.

@PetrDlouhy PetrDlouhy marked this pull request as draft July 8, 2022 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants