ponder-lab / Imperative-DL-Study-Web-App

0 stars 0 forks source link

Foreign key problems with categorization form #53

Closed khatchad closed 3 years ago

khatchad commented 3 years ago

I replaced the integers for problems categories, fixes, causes... with words in the Categorization model. However, even though this worked well in the Categorizations table, it raised an error when I tried to add a new categorization using the form. This is because Tatiana used the model instances to build the form (it's redundant to redefine all these model fields again as form fields). I fixed the problem locally but since Tatiana is working on the Categorization form, we need to fix this together since there are multiple changes to be made to the form fields.

Originally posted by @mZneit in https://github.com/ponder-lab/Imperative-DL-Study-Web-App/issues/22#issuecomment-796497518

khatchad commented 3 years ago

https://docs.djangoproject.com/en/3.1/topics/forms/modelforms/

khatchad commented 3 years ago

Reported by @mZneit. Please with work with @tatianacv to fix it if it is a problem. Please also add as much info as you can so that we can understand the problem.

mZneit commented 3 years ago

So in the Categorization table, we're trying to map the integer values of the columns: Problem category, Problem cause, Problem symptom, and Problem fix, to their text values. These text values can be found in their corresponding tables. So the Categorization model has a relation with 4 other models, and in order to map the integer values to these text values, we need to change the above fields to ForeignKeys in models.py to express that relation. Changing to ForeignKey made things work and the texts were displayed in place of integers. But since @tatianacv is creating the Categorization form using the Categorization model, the ForeignKeys should be treated in a special way. In forms.py, they should be defined as ModelChoiceField. In this link, you can see the mapping between model types and form types:

https://docs.djangoproject.com/en/3.1/topics/forms/modelforms/#field-types

Notice that the relational fields are mapped differently in the forms. Quoting the documentation:

"As you might expect, the ForeignKey and ManyToManyField model field types are special cases: ForeignKey is represented by django.forms.ModelChoiceField, which is a ChoiceField whose choices are a model QuerySet."

mZneit commented 3 years ago

I fixed this problem locally, but we need to make changes to the form fields that @tatianacv already built. I will send her the changes that I made.

khatchad commented 3 years ago

@mZneit, instead of "sending" the changes, I would create a new branch with the changes and issue a pull request. Then, request @tatianacv's review for the pull request. https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-requests

mZneit commented 3 years ago

Ok np.

khatchad commented 3 years ago

That way, we can precisely see the difference between your proposed fix and the current code base. Thanks, Manal!

mZneit commented 3 years ago

Thanks. Done.

khatchad commented 3 years ago

@mZneit, can you update this issue with the PR you opened in response to https://github.com/ponder-lab/Imperative-DL-Study-Web-App/issues/53#issuecomment-796999809?

khatchad commented 3 years ago

As discussed with @tatianacv, she needs to do this one since somehow it is interfering with the form submission.

khatchad commented 3 years ago

@tatianacv and I found another article here: https://docs.djangoproject.com/en/3.1/topics/db/examples/many_to_one/

khatchad commented 3 years ago

So, in summary, either we need to hack the table view to work with the current way categorizations are inserted OR we need to change the way the insertion works using ModelFields?

khatchad commented 3 years ago

The problem is that we can't verify, in the web app, the categories we are creating (we can do it on the backend, of course).

khatchad commented 3 years ago

By ModelField, we mean ManyToOne relationships.

khatchad commented 3 years ago

I think that you need to tie in model field to your widget:

khatchad commented 3 years ago

So, it's not as simple as just changing the field type, unfortunately.