search

goinnn / django-multiselectfield

A Multiple Choice model field
GNU Lesser General Public License v3.0
449 stars 208 forks source link

GET method does not return selected choices with Django REST Framework #73

Open stevepiercy opened 7 years ago

stevepiercy commented 7 years ago

In Django REST Framework, I noticed that PUT, PATCH, and OPTIONS methods work just fine. But when I GET an instance, I receive a list of values that are single characters. I expected to receive a list of the previously PUT or PATCH'ed choices.

I also looked in the database, and the values in there are exactly what was PUT or PATCH'ed as comma-separated values.

I'm not sure where to begin to troubleshoot, but at least I wanted to see if anyone else had this issue. It might be in my own code.

I can make a reproducible example using the DRF tutorial application as the base, if that would help.

See also #55.

blag commented 7 years ago

Are you using the MultipleChoiceField from DRF?

I can make a reproducible example using the DRF tutorial application as the base, if that would help.

That would be great!

stevepiercy commented 7 years ago

I found additional weirdness with the GET method.

First here's how to replicate the issues.

git clone https://github.com/stevepiercy/rest-framework-tutorial.git
cd rest-framework-tutorial
git checkout issue-73-multiple-choice-field
python3 -m venv env
source env/bin/activate
pip install --upgrade pip setuptools
pip install django djangorestframework pygments django-multiselectfield coreapi
python manage.py migrate
python manage.py createsuperuser
python manage.py runserver
# ignore warning:
# WARNINGS:
# ?: (rest_framework.W001) You have specified a default PAGE_SIZE pagination rest_framework setting,without specifying also a DEFAULT_PAGINATION_CLASS.
#         HINT: The default for DEFAULT_PAGINATION_CLASS is None. In previous versions this was # PageNumberPagination. If you wish to define PAGE_SIZE globally whilst defining pagination_class on a per-view basis you may silence this check.
# follow prompts
python manage.py runserver

In a web browser, visit http://127.0.0.1:8000/snippets/1/ Select all values for Multiselect field. Click PUT. Finally click the GET button. Instead of all values being selected, only the values False, True, Two, and Three are selected.

The weirdness manifests when the choices are not True, False, an integer between 0 and 9, or a string with length of one character. Integers 10 or greater and strings of length 2 or longer pose issues. Additionally integers 0 and 1 are interpreted as False and True.

Let me know if you need further information to reproduce.

I'm not sure whether I'm doing choices right, or whether this is in fact a bug either in code or documentation somewhere in DRF, Django, or this package. Thanks for your help.

stevepiercy commented 6 years ago

ping @blag or @goinnn. I'm revisiting this issue in hopes of resolving it. Please let me know whether you need anything further from me. I'm not sure how to troubleshoot, and I'd be grateful for any direction you can provide. Thank you!

blag commented 6 years ago

Sorry, I didn't mean to ignore you for two months. I've been busier than normal lately.

I managed to reproduce your issue once.

You are correct - the data is saved in the database as: {0, 1, 2, 3, 3.5, 10, 'WA', 'CA', 'OR'} (using SQLite as in your example).

DRF emits the JSON as:

{
    "url": "http://127.0.0.1:8000/snippets/1/",
    "id": 1,
    "highlight": "http://127.0.0.1:8000/snippets/1/highlight/",
    "owner": "blag",
    "title": "asdf",
    "code": "fdsa",
    "linenos": "True",
    "language": "abap",
    "style": "abap",
    "multiselect": [
        0,
        1,
        2,
        ",",
        3,
        ".",
        "C",
        "}",
        "W",
        "O",
        " ",
        "5",
        "{",
        "'",
        "R",
        "A"
    ]
}

Unforunately, every time I tried to reproduce it after that it worked just fine. DRF emitted this (correct) JSON:

{
    "url": "http://127.0.0.1:8000/snippets/1/",
    "id": 1,
    "highlight": "http://127.0.0.1:8000/snippets/1/highlight/",
    "owner": "blag",
    "title": "adsfasdfasdf",
    "code": "fdsa",
    "linenos": "True",
    "language": "abap",
    "style": "abap",
    "multiselect": [
        0,
        1,
        2,
        3,
        3.5,
        10,
        "WA",
        "OR",
        "CA"
    ]
}

If you can reliably reproduce this bug, please try to troubleshoot it a bit more. You can manually "install" django-multiselectfield and django-rest-framework by cloning them into your project repository with git and symlinking the appropriate directories, then not installing them with pip:

git clone https://github.com/stevepiercy/rest-framework-tutorial.git
cd rest-framework-tutorial
# vvvv
git clone https://github.com/goinnn/django-multiselectfield.git
ln -s django-multiselectfield/multiselectfield multiselectfield
git clone https://github.com/encode/django-rest-framework.git
ln -s django-rest-framework/rest_framework rest_framework
# ^^^^
git checkout issue-73-multiple-choice-field
python3 -m venv env
source env/bin/activate
pip install --upgrade pip setuptools
# DON'T install django-multiselectfield or django-rest-framework here
pip install django pygments coreapi
python manage.py migrate
python manage.py createsuperuser
python manage.py runserver
# ignore warning:
# WARNINGS:
# ?: (rest_framework.W001) You have specified a default PAGE_SIZE pagination rest_framework setting,without specifying also a DEFAULT_PAGINATION_CLASS.
#         HINT: The default for DEFAULT_PAGINATION_CLASS is None. In previous versions this was # PageNumberPagination. If you wish to define PAGE_SIZE globally whilst defining pagination_class on a per-view basis you may silence this check.
# follow prompts
python manage.py runserver

That should allow you to play with django-multiselectfield or DRF to see if that's causing the issue.

It's a little kludgy, but you can sprinkle print() statements in the various codebases to see what data they're being passed and how they are handling it.

Please keep in mind that this project is kind of a hack similar to Django's CommaSeparatedIntegerField, so it will have some limitations. Querying into the MultiSelectField will be very difficult to do correctly. I would recommend either using PostgreSQL's ARRAY field via django.contrib.postgres.fields.ArrayField or using Django's normal many to many relation.

Let me know if you run into any issues troubleshooting and I'll do what I can to help you. Thanks!

stevepiercy commented 6 years ago

After following your installation instructions, I've been able to reliably produce the issue, just like with my original issue report.

Can you suggest where I ought to put some print statements? I think the first place to look would be wherever DRF makes a query to the database and to view the result. Then step through how DRF parses that result and converts it to JSON.

FTR, I created two snippets, then did a GET for them, which returned the following JSON.

HTTP 200 OK
Allow: GET, POST, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

[
    {
        "url": "http://127.0.0.1:8000/snippets/1/",
        "id": 1,
        "highlight": "http://127.0.0.1:8000/snippets/1/highlight/",
        "owner": "stevepiercy",
        "title": "title",
        "code": "print('hello world')",
        "linenos": "True",
        "language": "python3",
        "style": "default",
        "multiselect": [
            0,
            1,
            2,
            "{",
            3,
            "5",
            "C",
            ",",
            "'",
            ".",
            " ",
            "W",
            "O",
            "R",
            "A",
            "}"
        ]
    },
    {
        "url": "http://127.0.0.1:8000/snippets/2/",
        "id": 2,
        "highlight": "http://127.0.0.1:8000/snippets/2/highlight/",
        "owner": "stevepiercy",
        "title": "title",
        "code": "print('hello world')",
        "linenos": "True",
        "language": "python3",
        "style": "default",
        "multiselect": [
            0,
            1,
            2,
            "{",
            3,
            "5",
            "C",
            ",",
            "'",
            ".",
            " ",
            "W",
            "O",
            "R",
            "A",
            "}"
        ]
    }
]
blag commented 6 years ago

Hm, that's interesting. I have more questions for you then.

  1. What OS? What version (be as specific as possible)?
  2. What version of Python (x.y.z)?
  3. What version of Django (x.y.z)? Use pip freeze | grep -i django to find it.
  4. What version of DRF (x.y.z)? Use pip freeze | grep -i rest | grep -i framework to find it.

I assume you're using a virtualenv, so find the rest_framework/fields.py file in your virtualenv and add print stuff out in MultiSelectField, particularly in get_value, to_internal_value, and to_representation. You're going to have to find the code path in DRF that is used and print that out, so don't be afraid to experiment with it. If you screw anything in DRF up just uninstall it with pip and reinstall it with pip.

stevepiercy commented 6 years ago

Shoot. I didn't pin Django, so I had 2.0.1 installed. I fixed that by uninstalling, then reinstalling 1.11.9. Here are my current versions:

  1. macOS 10.12.6
  2. Python 3.6.1
  3. Django 1.11.9
  4. DRF 3.7.7

DRF is symlinked to the cloned repo as you previously instructed. So is multiselectfield.

Nonetheless, now with these versions, I still experience the issue. When I do a PUT and look at the response, the correct items are selected in the multiselect. (Is that what you mean when you said it worked for you just once, perhaps?) But for every GET, I get the first character of each choice instead of the complete choice.

There is no MultiSelectField but there is a MultipleChoiceField in rest_framework/fields.py. Assuming that is what you meant, I added some print statements to the functions and I think I found the issue in to_representation. Here's the code and the result of a GET request to /snippets/1/. It does not look right to me, as I expect to see the complete choice.

What should I try next?

    def to_representation(self, value):
        for item in value:
            print('to_representation - value ' + str(
            self.choice_strings_to_values.get(six.text_type(item), item)))
        return {
            self.choice_strings_to_values.get(six.text_type(item), item) for item in value
        }
to_representation - value {
to_representation - value 0
to_representation - value ,
to_representation - value
to_representation - value 1
to_representation - value ,
to_representation - value
to_representation - value 2
to_representation - value ,
to_representation - value
to_representation - value 3
to_representation - value ,
to_representation - value
to_representation - value 3
to_representation - value .
to_representation - value 5
to_representation - value ,
to_representation - value
to_representation - value '
to_representation - value W
to_representation - value A
to_representation - value '
to_representation - value ,
to_representation - value
to_representation - value 1
to_representation - value 0
to_representation - value ,
to_representation - value
to_representation - value '
to_representation - value C
to_representation - value A
to_representation - value '
to_representation - value ,
to_representation - value
to_representation - value '
to_representation - value O
to_representation - value R
to_representation - value '
to_representation - value }
to_representation - value {
to_representation - value 0
to_representation - value ,
to_representation - value
to_representation - value 1
to_representation - value ,
to_representation - value
to_representation - value 2
to_representation - value ,
to_representation - value
to_representation - value 3
to_representation - value ,
to_representation - value
to_representation - value 3
to_representation - value .
to_representation - value 5
to_representation - value ,
to_representation - value
to_representation - value '
to_representation - value W
to_representation - value A
to_representation - value '
to_representation - value ,
to_representation - value
to_representation - value 1
to_representation - value 0
to_representation - value ,
to_representation - value
to_representation - value '
to_representation - value C
to_representation - value A
to_representation - value '
to_representation - value ,
to_representation - value
to_representation - value '
to_representation - value O
to_representation - value R
to_representation - value '
to_representation - value }
to_representation - value {
to_representation - value 0
to_representation - value ,
to_representation - value
to_representation - value 1
to_representation - value ,
to_representation - value
to_representation - value 2
to_representation - value ,
to_representation - value
to_representation - value 3
to_representation - value ,
to_representation - value
to_representation - value 3
to_representation - value .
to_representation - value 5
to_representation - value ,
to_representation - value
to_representation - value '
to_representation - value W
to_representation - value A
to_representation - value '
to_representation - value ,
to_representation - value
to_representation - value 1
to_representation - value 0
to_representation - value ,
to_representation - value
to_representation - value '
to_representation - value C
to_representation - value A
to_representation - value '
to_representation - value ,
to_representation - value
to_representation - value '
to_representation - value O
to_representation - value R
to_representation - value '
to_representation - value }
to_representation - value {
to_representation - value 0
to_representation - value ,
to_representation - value
to_representation - value 1
to_representation - value ,
to_representation - value
to_representation - value 2
to_representation - value ,
to_representation - value
to_representation - value 3
to_representation - value ,
to_representation - value
to_representation - value 3
to_representation - value .
to_representation - value 5
to_representation - value ,
to_representation - value
to_representation - value '
to_representation - value W
to_representation - value A
to_representation - value '
to_representation - value ,
to_representation - value
to_representation - value 1
to_representation - value 0
to_representation - value ,
to_representation - value
to_representation - value '
to_representation - value C
to_representation - value A
to_representation - value '
to_representation - value ,
to_representation - value
to_representation - value '
to_representation - value O
to_representation - value R
to_representation - value '
to_representation - value }
[15/Jan/2018 08:22:28] "GET /snippets/1/ HTTP/1.1" 200 57631
stevepiercy commented 6 years ago

I've also tried Django's ManyToManyField, but either the field does not display in the DRF web UI, or I get other errors. I've scoured both DRF and Django docs, Stack Overflow, and exhausted all my Google Fu. Does anyone have a working example of a ManyToManyField that I can reproduce?

blag commented 6 years ago

@stevepiercy Thanks for the additional information! The printed data there indicates that to_representation is being called over every character in the string. I suspect this is because some piece of code is calling to_representation with every element of an iterable, when it should be calling to_representation with every element of an iterable unless that iterable is a string. However, that might be a limitation in Django or DRF due to the way CommaSeparatedIntegerField is implemented behind the scenes (as a string that is .split() on commas).

I suspect that this isn't going to be solved quickly, and it looks like you will someday try to filter on the MultiSelectField, which will be difficult and always be error-prone if you use this package.

I think the appropriate relationship is a proper M2M for you. Not only will it be less buggy, it will give you control over how to display those relations to the user, and be well supported by Django. Unfortunately there aren't super easy ways to handle that with DRF. It does support this though - look into the DRF documentation on nested relationships and either nested routes or nested routers.

stevepiercy commented 6 years ago

@blag thank you. The M2M relationship option is what I am pursuing. I haven't figured out whether DRF can render such relationships in its web UI, and if it can, where I can find such an example that is complete. Stack Overflow examples lack completeness.

Jcat7414 commented 4 years ago

I am curious to know if you ever managed to find a fix for this? I am encountering the same issue, using a MultipleChoiceField. The data displays correctly on a POST or PUT, but not for GET.

stevepiercy commented 4 years ago

Yup, I went with Pyramid, SQLAlchemy, and Deform. Probably not the answer you wanted, but it gives me the control I need.