AndrewIngram / django-extra-views

Django's class-based generic views are awesome, let's have more of them.
MIT License
1.39k stars 172 forks source link

[Feature] Use slug fields instead of pk in formset templates #251

Open lorddaedra opened 2 years ago

lorddaedra commented 2 years ago

Problem

Let's assume, we have two models.

models.py:

class Survey(models.Model):
    uid = models.UUIDField(verbose_name='UID', unique=True, default=uuid7, editable=False)
    title = models.CharField(_('title'), max_length=255)
    description = models.TextField('description')
    is_active = models.BooleanField(
        'active',
        default=True,
        help_text='Designates whether this object should be treated as active. Unselect this instead of deleting objects.',
    )
    ...

class Feature(models.Model):
    uid = models.UUIDField(verbose_name='UID', unique=True, default=uuid7, editable=False)
    survey = models.ForeignKey(
        Survey,
        on_delete=models.CASCADE,
        verbose_name='survey',
        related_name='features',
        related_query_name='feature',
    )
    display_name = models.CharField('display name', max_length=255)
    is_active = models.BooleanField(
        'active',
        default=True,
        help_text='Designates whether this object should be treated as active. Unselect this instead of deleting objects.',
    )
    ....

Now we would like to use UpdateWithInlinesView.

Let's view our form html:

...
<input type="hidden" name="features-0-id" value="7" id="id_features-0-id">
...
<input type="hidden" name="features-1-id" value="8" id="id_features-1-id">
...
<input type="hidden" name="features-2-id" value="9" id="id_features-2-id">

We have Feature instance ids there for each of formset members. It's acceptable for most of projects but we may want to use slug fields (uid in this case).

We have motivation to do it if we use integer PK (for performance reasons) and would like to prevent marketing tracking from our competitors (they may create new features every month and compare feature ids to each other to answer questions like how much new surveys do we have in this month and so on).

form html:

...
<input type="hidden" name="features-0-id" value="0183a330-a8d5-77c2-a3d7-166d6f1c1fe7" id="id_features-0-id">
...

Solution

We replace slugs (uids) to ids here during processing POST data. First step: we create uids list. Second step: we load features list of dicts with id and uid data from database based on uids values. Third step: we replace slugs to id if we know how to do it (we read features).

class FeatureUpdateInline(InlineFormSetFactory):  # type: ignore
    ...
    form_class = FeatureInlineForm
    formset_class = BaseFeatureFormSet
    model = Feature
    fields = ('display_name', )
    ...

    @staticmethod
    def process_post_data(obj, post_data):
        object_id = str(obj.id)
        changes = {}
        uids = []
        # 1
        for field_name, value in post_data.items():
            if field_name.endswith('-id'):
                uids.append(value)
        # 2
        features = obj.features.filter(is_active=True, uid__in=uids).values('id', 'uid')
        # 3
        for field_name, value in post_data.items():
            if field_name.endswith('-id'):
                for feature in features:
                    if value == str(feature['uid']):
                        changes[field_name] = str(feature['id'])

        return changes

    def get_formset_kwargs(self):
        kwargs = super().get_formset_kwargs()
        if self.request.method in ("POST", "PUT"):

            kwargs['data'].update(self.process_post_data(obj=self.object, post_data=kwargs['data']))
            print(kwargs['data'].dict())

        return kwargs

class SurveyUpdateView(
    ...
    NamedFormsetsMixin,
    UpdateWithInlinesView,
):
    ...
    form_class = SurveyUpdateForm
    inlines = [FeatureUpdateInline]
    inlines_names = ['Features']
    ...

Suggestions

  1. Do you have suggestions how to refactor this code?
  2. Can we somehow add this feature to django-extra-views?
jonashaag commented 2 years ago

Sounds like a legitimate use case.

I didn't find any code dealing with PKs with a quick search in this repo. Maybe this is coming from Django itself?

You might want to make a custom Field that encrypts its value with a secret when coming from the DB and decrypts it before going to the DB.

lorddaedra commented 2 years ago

Sounds like a legitimate use case.

Yes, IMHO it's also a Django-way. We can open any generic views from Django source code and ensure they have support for slug-based getters as alternative to pk.

I didn't find any code dealing with PKs with a quick search in this repo. Maybe this is coming from Django itself?

Yes, you're right.

Why do fieldsets do not support pk to slug replacements? I think, the answer is simple here. Django maintainers do not use fieldsets in "frontend applications", they use them only in admin panel, it's a thing like "back office". So if someone already has access to admin panel, he/she may investigate our system in details, so we already do not have big motivation to hide real integer pk of our objects or something like that.

That's why this feature may be will never implemented in Django itself. (At least until we will see any generic views with fieldsets usage inside in Django source code.)

But it can be a good idea to improve it on django-extra-views side.

Maybe this is coming from Django itself?

Just replace

{{ Features }}

to

{{ Features.management_form }}
{% for feature_form in Features %}
    {{ feature_form.display_name }}
    {{ feature_form.display_name.errors }}
    <input type="hidden" name="{{ feature_form.prefix }}-id" value="{{ feature_form.instance.uid }}" id="id_{{ feature_form.prefix }}-id">
{% endfor %}

in template and that's all. (Plus get_formset_kwargs changes from my first message)

So, yes, {{ Features }} will add two hidden input fields per each inline form (pk of feature and fk to survey but it's safe to omit it, fk is needed only for + Add new feature new inline forms).

About fk hidden fields: it's possible to use slugs (uid) for + Add forms here too and remove hidden fk fields completely from any forms for existed inline objects.

    @staticmethod
    def process_post_data(obj, post_data):
        object_id = str(obj.id)
        changes = {}
        ...
        for field_name, value in post_data.items():
            ...
            if field_name.endswith('-display_name'):
                changes[field_name.replace('-display_name', '-survey')] = object_id
        ...

This is similar to

<input type="hidden" name="{{ feature_form.prefix }}-survey" value="{{ feature_form.instance.id }}" id="id_{{ feature_form.prefix }}-survey">
lorddaedra commented 2 years ago

You might want to make a custom Field that encrypts its value with a secret when coming from the DB and decrypts it before going to the DB.

If we will create additional form field, we need somehow prevent updating value of that field (may be del it before formset.save()). We should not allow to change value of uid field, for example.