Delete/Edit issues with public tunes

[jwjacobson]

Right now the delete view deletes both the Tune and RepertoireTune object (lines 137-8 of views.py). This is a problem because if the user takes a tune from the public database and then deletes it from their repertoire, the tune is also deleted from the public database. On the other hand, the user should be able to remove these tunes from their repertoire, so simply forbidding the user from deleting them will not work. I see two possible ways of addressing this:

1. Create a copy of the tune object, which is then added to the user's repertoire. Then it can be deleted with no impact on the public tune. This would involve removing the unique constraint from tune titles but I don't think that's really a big issue; each tune still has its id as a unique identifier. The potential drawback to this approach is that it could be inefficient spacewise with many duplicates of the same tunes being made in a multiuser scenario.
2. Change the delete view so only the RepertoireTune is deleted, while the tune object remains. This should work since only tunes in a user's repertoire ever appear to the user, but it means that if a user creates their own tunes then deletes them those tunes will uselessly persist in the database, of no use to anybody.

As I write this I realize this will also be an issue with edit and so the first option seems to make more sense, as the second option doesn't prevent the user from editing a public tune.

[bbelderbos]

Might not be the case? Can you verify this please?

[jwjacobson]

Well, right now the code deletes both: tune.delete() followed by rep_tune.delete(). But even if only the rep_tune were deleted, editing still requires that the original tune be duplicated when it is taken from the public repository, since the user is able to edit all fields of the tune (title, key, form, etc.) but this shouldn't affect the public tune.

Another thing to consider is that deleting only the rep_tune and leaving the tune in existence is a form of "archiving" for the user; since their tune still exists it might be possible for them to get it back...

[bbelderbos]

OK good points, thanks. So if the user can edit the tune it needs to be a copy.I like the archived state that copied tune can be in. So we just delete the rep_tune relation which can be reestablished at a later time, right? Are you clear how to move forward on this one then?

[ryaustin]

Interesting thread. I'm not sure if you've settled on an approach but it does seem like creating a user specific copy is the cleanest route here.

[bbelderbos]

I think so too, thanks Ryan. Jeff I did some cleanup of old code, I am shooting you a PR now.

[bbelderbos]

@jwjacobson so here it would make the copy right?

@login_required
def tune_take(request, pk):
    tune = get_object_or_404(Tune, pk=pk)

    if request.method == "POST":
        RepertoireTune.objects.create(tune=tune, player=request.user)
        # rep_tune.save()
        # messages.success(
        #     request,
        #     f"Tune {rep_tune.tune.id}: {rep_tune.tune.title} copied to repertoire.",
        # )
        # return redirect("tune:tune_browse")

    return render(request, "tune/browse.html", {"tune": tune})

As triggered from tune/templates/tune/browse.html ?

[jwjacobson]

Yes, exactly. Make the copy there and then make the RepertoireTune pointing to the copy

[bbelderbos]

Cool, done today: https://github.com/jwjacobson/jazz_repertoire/pull/163

Btw I just saw in the docs that _state.adding should be set to True as well: https://docs.djangoproject.com/en/5.0/topics/db/queries/#copying-model-instances

But checking with ChatGPT it seems optional:

[bbelderbos]

Also posted a tweet to ask others ... https://twitter.com/bbelderbos/status/1748007175916503042

[jwjacobson]

I believe most recent PR solves this (forgot to try putting the issue in the commit message)

[bbelderbos]

Probably fine as is, but you can also abstract it on the model:

import copy

class Blog(models.Model):
    # your fields here

    def clone(self, save=False):
        """
        Creates a copy of the current instance. Optionally saves the copy.  
        """
        copy_obj = copy.deepcopy(self)
        copy_obj.pk = None
        if save:
            copy_obj.save()
        return copy_obj

# Usage
blog = Blog.objects.get(id=1)
new_blog = blog.clone(save=True)