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

Editing model with many-to-one relationship #110

Open
agseaton opened this issue Jan 31, 2024 · 15 comments
Open

Editing model with many-to-one relationship #110

agseaton opened this issue Jan 31, 2024 · 15 comments

Comments

@agseaton
Copy link

The documentation describes how to set up a form collection for creating a new entry with a one-to-many relationship (https://django-formset.fly.dev/model-collections/#one-to-many-relations).

However, it does not describe how to use a FormCollection to edit an existing entry. Would you be able to provide an example of how to do this (ideally similar to https://django-formset.fly.dev/model-form/#complete-crud-view)?

@jrief
Copy link
Owner

jrief commented Feb 1, 2024

I'm unsure if I understand your question, but forms can be created from models as described here:
https://django-formset.fly.dev/model-collections/#one-to-one-relations
this also applies to one-to-many-relations

The important method to remember is model_to_dict which recursively traverses the the tree of related models and builds a dictionary then used to initialize the collection of forms.

@agseaton
Copy link
Author

agseaton commented Feb 2, 2024

Let me give a concrete example. Let's say we have the following pair of models as per the documentation:

class Company(models.Model):
    name = models.CharField(verbose_name="Company name", max_length=50)

class Department(models.Model):
    name = models.CharField(verbose_name="Department name", max_length=50)
    company = models.ForeignKey(Company, on_delete=models.CASCADE)

    class Meta:
        unique_together = ['name', 'company']

The documentation describes how to write a form that will allow the user to create a company and a series of new linked departments.

What I'd like to do instead is to create a form that will not only do this, but that can also be used to edit existing objects. In other words, similar to the CRUD example but for FormCollections. I understand that I need to implement model_to_dict functions, but it isn't immediately obvious to me how to do this, and from what I can see this isn't all that is required.

In particular, how should I represent the child objects? A list of dicts, or a dict containing a list? What should the dict keys be? Which Form/FormCollection classes need to implement these methods? It seems to me that the FormCollectionView needs modifying too, so if that's true, what changes need to be made there?

If you'd be able to provide a complete working example of this, it would help clear up some of these ambiguities. I've been trying to figure this out for some time by referring to the various examples you've provided and by trial and error, but I'm just getting more and more confused!

@jrief
Copy link
Owner

jrief commented Feb 2, 2024

I believe the best idea is that you fork this project and add one of the existing models/collections to your implementation. Then I'll have a look at how that problem can be solved. Btw., I'm always interested in exotic use-cases, so yours seems to play in that league.

It might however take some time, because I'm currently working on other stuff.

@agseaton
Copy link
Author

agseaton commented Feb 7, 2024

Sure, it might take me a little while to find some time to do this, but I'm planning to give it a try. I'll post back here once I have something I can share.

@lsmoker
Copy link

lsmoker commented Mar 7, 2024

I would also like to know how to update/view existing parent/child data using form collections - not just add new objects (database table rows).

@jrief
Copy link
Owner

jrief commented Mar 7, 2024

Did you try the methods construct_instance and model_to_dict. Here is a working demo:
https://django-formset.fly.dev/bootstrap/company

the code to run that demo is provided in testapp.

@lsmoker
Copy link

lsmoker commented Mar 7, 2024

Yes, that's it. My attempts with my own models haven't worked out, but I'll keep at it.

@lsmoker
Copy link

lsmoker commented Mar 8, 2024

I see that the FormCollection classes in company.py only have retrieve_instance methods not construct_instance and model_to_dict. I have tried overriding all 3 of those methods in my child FormCollection but they are not called when the page loads...

@jrief
Copy link
Owner

jrief commented Mar 8, 2024

did you use the formset.views.EditCollectionView?

@lsmoker
Copy link

lsmoker commented Mar 9, 2024

I did. I think the problem I'm having relates to my child model's ForeignKey field - I did not use the related_name parameter which means the reverse relation becomes something like book_set rather than explicitly named as books which your examples do. Monday I'll do more work on it.

@jrief
Copy link
Owner

jrief commented Mar 9, 2024

You may send me the link to your (public) repository and I will try to reproduce it.

@lsmoker
Copy link

lsmoker commented Mar 12, 2024

I did learn that I need to set the related_name on the child model for that form collection to load existing rows (as I mentioned previously). Here is the repo that can demonstrate that -> https://github.com/lsmoker/django-formset

In testapp/models/bundle.py, I have a line commented out that doesn't have related_name specified for the bundle field. In testapp/forms/bundle.py, I have a line commented out that uses opportunity_set (the default manager name if related_name is not used). I was wrongly assuming(!) opportunity_set would work. If you switch these 2 lines, you can see that the Opportunity rows will not be loaded.

Maybe the documentation could make it clear that related_name needs to be specified in this situation and even explain how the "magic" of it works...

@SamuelJennings
Copy link

I encountered this issue as well while trying to create a form collection for a one-to-many relationship. Here's what I discovered. Everything works fine ONLY if the name of the declared holder is equivalent to the reverse accessor of the related field on the parent model. For example

# models.py
class Department(models.Model):
    company = models.ForeignKey(Company, on_delete=models.CASCADE)

# forms.py
class CompanyCollection(FormCollection):
    # this doesn't work!
    # departments = DepartmentCollection()

    # this works!
    department_set = DepartmentCollection()

and also,

# models.py
class Department(models.Model):
    company = models.ForeignKey(Company, on_delete=models.CASCADE, related_name="departments")

# forms.py
class CompanyCollection(FormCollection):
    # this works now!
    departments = DepartmentCollection()

Why?

At the moment, the default implementation of model_to_dict tries to fetch the related manager and queryset like this:

# formset/collection.py 

class BaseFormCollection(HolderMixin, RenderableMixin):

    def model_to_dict(...):
        ...
        if related_manager := getattr(instance, holder._name, None):
            ...

holder._name is automatically set based on the declared attribute on the parent holder. Ergo, the attribute name of the related form collection MUST be the same as the reverse accessor on the instance.

So...

I don't think the current implementation is necessarily a bad thing but it probably needs to be documented. Perhaps something could be changed to allow a little more flexibility with naming and declaring child form collections but I would leave that to you.

@jrief
Copy link
Owner

jrief commented May 24, 2024

Thanks @SamuelJennings for pointing this out. I'll add this to the documentation. If you want, you can create a pull request and write it yourself.

@SamuelJennings
Copy link

SamuelJennings commented May 27, 2024 via email

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

No branches or pull requests

4 participants