Adding placeholders to a template after objects already saved breaks things in a bad way.

[mrmachine]

We are using fluent-contents (without fluent-pages). We are using the "CMS system" style with dynamic layout switching. A particular layout's template initially had only one placeholder defined in it. Objects using this layout have already been saved to the database and it is working as expected.

We need to add a second placeholder to the existing layout's template. When we do, the new placeholder is detected and rendered in the form, but when we save the form, we get a ValidationError: '' value must be an integer." originating in fluent_contents.admin.genericextensions.BaseInitialGenericInlineFormSet._construct_form.

While digging through the code, we noticed what looks like a related bug in BaseInitialGenericInlineFormSet.__get_form_instance(). The code assumes that the number of forms in the formset equals the number of Placeholder objects already saved in the database and that their sequence is a match. We fixed that problem with the following code:

    def __get_form_instance(self, i):
        instance = None
        try:
            # Editing existing object. Make sure the ID is passed.
            instance = self.get_queryset().filter(slot=self._initial[i]['slot'])[0]
        except IndexError:
            try:
                # Adding new object, pass initial values
                # TODO: initial should be connected to proper instance ordering.
                # currently this works, because the client handles all details for layout switching.
                queryset_count = self.get_queryset().count()
                values = self.__initial_minus_queryset()[i - queryset_count]

                values[self.ct_field.name] = ContentType.objects.get_for_model(self.instance)
                values[self.ct_fk_field.name] = self.instance.pk
                instance = self.model(**values)
                instance.save()
            except IndexError:
                pass

        return instance

This code does two things differently:

1. It gets the Placeholder by slot name instead of by index, in case they are out of order (e.g. the template is updated and their positions are reversed).

2. It calls instance.save() to persist the new Placeholder object to the database as soon as it is needed by the frontend, when the change form page is rendered.

This fixed the ValidationError, but on save we got a different error, instead...

IntegrityError: duplicate key value violates unique constraint "fluent_contents_placeholde_parent_type_id_451c85966d08dedf_uniq"
DETAIL:  Key (parent_type_id, parent_id, slot)=(10, 100607, main) already exists.

It seems that fluent-contents is trying to create a Placeholder that already exists. Perhaps because we already created it earlier in __get_form_instance()?

@sjdines and I spent a couple hours trying to debug this, and it seems pretty complex. Perhaps this would be an easy fix for you, @vdboor, or you could shed some light on it for us.

Should we be trying to create and save a Placeholder object as soon as the frontend needs one, or only when content items are saved to a placeholder?

Or should we be trying to fix the ValidationError another way?

[mrmachine]

Ping @vdboor, any feedback on how we should approach this?

[vdboor]

Hi!

Thanks for getting back on this issue. I was away for 2 weeks (holiday), sorry I didn't notice it before.

Regarding the raised points:

1. thanks a lot, this seems like a good idea. I'd recommend doing a Python loop to walk through the queryset, so the cache is read instead.
2. I'm not sure about calling instance.save(). Is this really needed? I'd prefer to save the Placeholder when the data will actually be saved.

[mrmachine]

Thanks for the feedback. I've done some more work on this and opened a WIP pull request so we can discuss more easily.

#63