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

Refresh from DB when updating nested object #122

Merged
merged 2 commits into from
Oct 29, 2020

Conversation

pcarn
Copy link
Contributor

@pcarn pcarn commented Oct 9, 2020

When I make a PUT or PATCH request on an object that includes changes to a nested model, the response does not include the changes.
But when I do a GET on the object after that, it shows the changes. It is making the change, but not returning it in the response.

I've confirmed that adding a refresh_from_db() in the update serializer fixes this. It responds with the changes I made, and behaves like I'd expect.

Example:
Models:

class Question(BaseModel):
    text = models.CharField(max_length=50)

class Option(BaseModel):
    question = models.ForeignKey(
        "Question", related_name="options", on_delete=models.CASCADE,
    )
    text = models.CharField(max_length=50)

Serializers:

class OptionSerializer(DynamicFieldsModelSerializer):
    class Meta:
        model = models.Option
        fields = (
            "id",
            "text",
        )

class QuestionSerializer(WritableNestedModelSerializer):
    options = OptionSerializer(many=True)

    class Meta:
        model = models.Question
        fields = (
            "id",
            "text",
            "options",
        )

Before these changes:

GET /api/events/questions/cc464c6e-79f8-496d-9aad-d98a7fff11a5/
Response:
{
    "id": "cc464c6e-79f8-496d-9aad-d98a7fff11a5",
    "text": "what is the best cheese?",
    "options": [
        {
            "id": "8cd27b47-129d-4093-8132-0bbcfc17a45a",
            "text": "cheddar"
        },
        {
            "id": "8d53d35c-9694-40d2-9e7c-e1ad83f99bb5",
            "text": "gouda"
        }
    ]
}

PATCH /api/events/questions/cc464c6e-79f8-496d-9aad-d98a7fff11a5/
Body:
{
    "options": [
        {
            "id": "8cd27b47-129d-4093-8132-0bbcfc17a45a",
            "text": "cheddar"
        },
        {
            "id": "8d53d35c-9694-40d2-9e7c-e1ad83f99bb5",
            "text": "gouda"
        },
        {
            "text": "swiss"
        }
    ]
}
Response:
{
    "id": "cc464c6e-79f8-496d-9aad-d98a7fff11a5",
    "text": "what is the best cheese?",
    "options": [
        {
            "id": "8cd27b47-129d-4093-8132-0bbcfc17a45a",
            "text": "cheddar"
        },
        {
            "id": "8d53d35c-9694-40d2-9e7c-e1ad83f99bb5",
            "text": "gouda"
        }
    ]
}

GET /api/events/questions/cc464c6e-79f8-496d-9aad-d98a7fff11a5/
Response:
{
    "id": "cc464c6e-79f8-496d-9aad-d98a7fff11a5",
    "text": "what is the best cheese?",
    "options": [
        {
            "id": "8cd27b47-129d-4093-8132-0bbcfc17a45a",
            "text": "cheddar"
        },
        {
            "id": "8d53d35c-9694-40d2-9e7c-e1ad83f99bb5",
            "text": "gouda"
        },
        {
            "id": "6b33788e-298d-4408-b6ea-dd7dcd1c8232",
            "text": "swiss"
        }
    ]
}

After these changes:

GET /api/events/questions/cc464c6e-79f8-496d-9aad-d98a7fff11a5/
Response:
{
    "id": "cc464c6e-79f8-496d-9aad-d98a7fff11a5",
    "text": "what is the best cheese?",
    "options": [
        {
            "id": "8cd27b47-129d-4093-8132-0bbcfc17a45a",
            "text": "cheddar"
        },
        {
            "id": "8d53d35c-9694-40d2-9e7c-e1ad83f99bb5",
            "text": "gouda"
        }
    ]
}

PATCH /api/events/questions/cc464c6e-79f8-496d-9aad-d98a7fff11a5/
Body:
{
    "options": [
        {
            "id": "8cd27b47-129d-4093-8132-0bbcfc17a45a",
            "text": "cheddar"
        },
        {
            "id": "8d53d35c-9694-40d2-9e7c-e1ad83f99bb5",
            "text": "gouda"
        },
        {
            "text": "swiss"
        }
    ]
}
Response:
{
    "id": "cc464c6e-79f8-496d-9aad-d98a7fff11a5",
    "text": "what is the best cheese?",
    "options": [
        {
            "id": "8cd27b47-129d-4093-8132-0bbcfc17a45a",
            "text": "cheddar"
        },
        {
            "id": "8d53d35c-9694-40d2-9e7c-e1ad83f99bb5",
            "text": "gouda"
        },
        {
            "id": "6b33788e-298d-4408-b6ea-dd7dcd1c8232",
            "text": "swiss"
        }
    ]
}

GET /api/events/questions/cc464c6e-79f8-496d-9aad-d98a7fff11a5/
Response:
{
    "id": "cc464c6e-79f8-496d-9aad-d98a7fff11a5",
    "text": "what is the best cheese?",
    "options": [
        {
            "id": "8cd27b47-129d-4093-8132-0bbcfc17a45a",
            "text": "cheddar"
        },
        {
            "id": "8d53d35c-9694-40d2-9e7c-e1ad83f99bb5",
            "text": "gouda"
        },
        {
            "id": "6b33788e-298d-4408-b6ea-dd7dcd1c8232",
            "text": "swiss"
        }
    ]
}

@codecov-io
Copy link

codecov-io commented Oct 9, 2020

Codecov Report

Merging #122 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #122   +/-   ##
=======================================
  Coverage   98.13%   98.13%           
=======================================
  Files           3        3           
  Lines         214      215    +1     
=======================================
+ Hits          210      211    +1     
  Misses          4        4           
Impacted Files Coverage Δ
drf_writable_nested/mixins.py 98.02% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4084624...d0e6571. Read the comment docs.

@pcarn
Copy link
Contributor Author

pcarn commented Oct 9, 2020

I can use a workaround by adding this to QuestionSerializer:

    def update(self, instance, validated_data):
        updated_instance = super().update(instance, validated_data)
        updated_instance.refresh_from_db()
        return updated_instance

But this seems like something that should be default behavior, and it would be nice to not need a workaround.

@ruscoder
Copy link
Member

Hello @pcarn! Thanks for your contribution. Could you please add tests covering your case (tests that fail without this PR)?

@ruscoder
Copy link
Member

It seems like a good improvement, I'm ready to merge it to the master.

@pcarn
Copy link
Contributor Author

pcarn commented Oct 19, 2020

@ruscoder Thanks! Added tests

@pcarn
Copy link
Contributor Author

pcarn commented Oct 29, 2020

@ruscoder Can this be merged so I can start using it?

@ruscoder ruscoder merged commit 3e38d12 into beda-software:master Oct 29, 2020
@ruscoder
Copy link
Member

@pcarn Thanks! I've just published version 0.6.2 which includes your fix.

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

Successfully merging this pull request may close these issues.

4 participants