Copy feature overwrites instance rather than creating new one

[danhayden]

First I'd like to thank you for creating this app, it's really helped me out :smile:

One issue I have noticed is in the admin section when I click the copy button of a campaign it overwrites the current instance rather than duplicating.

Inspecting the database verifies that no new instance was created and the existing campaign is amended. Also if the campaign has been sent it resets the status.

Perhaps there is an issue with the copy method logic? https://github.com/neon-jungle/wagtail-birdsong/blob/e51508bec00d47fa77fe0f1974dda24bb2b58aa8/birdsong/options.py#L161-L169

Does this work as expected for you?

Let me know if you need any further info.

Thanks

[seb-b]

I used this method from the Django docs: https://docs.djangoproject.com/en/2.2/topics/db/queries/#copying-model-instances

But it looks like there's a slightly updated method in 3.0+ where you need to add instance._state.adding = True, what version of Django are you using?

[danhayden]

Thanks for looking in to this, really appreciate it.

I'm using Django 3.1.8 and have just tested the copy feature after adding instance._state.adding = True, but that did not fix the issue I have been experiencing..

I have however noticed that this is happening due to having subclassed Campaign then creating campaigns from that subclass rather than Campaign directly.

class BaseCampaign(Campaign):
    # ... shared logic

class Campaign1(BaseCampaign):
    # ... campaign specific logic

class Campaign2(BaseCampaign):
    # ... campaign specific logic

The copy feature works as expected for BaseCampaign but as I described previously for Campaign1 and Campaign2.

---

Is there a better way you are aware of for creating campaigns with shared logic or is it possible for the copy feature to work with subclasses? Otherwise would it be best simply to duplicate the logic for each campaign and subclass Campaign directly?

Thanks

[seb-b]

Oh yep, that would cause it. The docs mention setting the id to None for subclasses, as well as the pk, that might fix it?

If you have shared behaviour, and BaseCampaign does not need to be created and used directly, you can make that campaign abstract and it should work as expected. e.g.

class BaseCampaign(Campaign):
    # ... shared logic
    class Meta:
        abstract = True

class Campaign1(BaseCampaign):
    # ... campaign specific logic

class Campaign2(BaseCampaign):
    # ... campaign specific logic

If BaseCampaign does need to be used directly, perhaps moving the shared logic into a mixin would work e.g:

class CampaignMixin(object):
   # ... shared logic

class BaseCampaign(Campaign, CampaignMixin):
    # ... base campaign specifics

class Campaign1(Campaign, CampaignMixin):
    # ... campaign specific logic

class Campaign2(Campaign, CampaignMixin):
    # ... campaign specific logic

[danhayden]

Making BaseCampaign abstract, that did the trick! Thanks for the suggestion :smile:

Do you think it would be worth adding a note about shared campaign logic to the docs?

[seb-b]

Excellent, glad to hear its working. It probably wouldn't hurt to mention that the campaign model needs to be concrete subclass, open to a pr for that :)

But for now I'll close this issue as it looks like the original problem is fixed