1242984 - 500 error when setting history_current to null

Status: RESOLVED FIXED

(developer.mozilla.org Graveyard :: BrowserCompat, defect)

Description

[John Whitlock [:jwhitlock]]

What did you do?
================
1. PATCH /api/v2/browsers/1 with: 

{"data": {
  "relationships": {
    "history_current": {
      "data": null
    }
  }
}
2. 
3. 

What happened?
==============
A 500 error is raised, with the traceback:

  File "/web-platform-compat/webplatformcompat/serializers.py", line 122, in to_internal_value
    history_id = int(data['history_current'])
TypeError: int() argument must be a string or a number, not 'NoneType'



What should have happened?
==========================
A 400 error should be returned, with a JSON path to /data/relationships/history_current/data, explaining that null is not allowed.

Is there anything else we should know?
======================================
This applies to all resources that create historical resources on changes, and a single field validators should fix them all.

Comment 1

[Anthony Maton [:anthony]]

Hello,

I'm interested by taking a look at that bug.
Would you have time to guide me a bit to find the good place to implement that ?

Comment 2

[John Whitlock [:jwhitlock]]

First step would be a failing test that reproduces the error, in webplatformcompat/tests/test_serializers.py

Next would be a change to HistoricalModelSerializer.to_internal_value in webplatformcompat/serializers.py so that a null value doesn't trigger the exception but instead raises a ValidationError, making the test pass.

Then, ensure that unit tests and API tests run cleanly with the change.

Comment 3

[MDN Team (:mdn-dev)]

Commits pushed to master at https://github.com/mdn/browsercompat

https://github.com/mdn/browsercompat/commit/5230cac0affedf3b8f5924e10e8a9a0f695e4909
bug 1242984 Add a unitary test + a test in serializers.py

https://github.com/mdn/browsercompat/commit/e4379be054be5f4e813bd2fa800ed26a27867bdc
Merge pull request #117 from MatonAnthony/1242984

Bug 1242984 500 error when setting history_current to null

Comment 4

[John Whitlock [:jwhitlock]]

Confirmed as fixed, which is hard due to bug 1230584, but that is literally another issue.