Frameworks throws AssertionError saying you cannot set required=True and...

[gdoermann]

... read_only=True on editable=False model fields. We should not make the field required if editable=False.

[carltongibson]

Hi @gdoermann — thanks for the pull-request. Ideally there's be a test case demonstrating the issue — it's hard to see otherwise.

However... :) the serialisers are undergoing a major redesign for the upcoming 3.0 release. I'm going to close this ticket and add it to the 3.0 pre-release reassessment milestone.

If you're able to test 3.0 before release and help verify your issue is addressed that would be awesome.

[tomchristie]

Considering reopening and merging for 2.4, given the limited scope & passing tests.

[gdoermann]

Thanks guys. In the end I just added blank=True to the model which was a
quick band-aid. Either way thanks for all the work you guys do! We love
the framework!

On Thu, Aug 21, 2014 at 4:38 AM, Tom Christie [email protected]
wrote:

Considering reopening and merging for 2.4, given the limited scope &
passing tests.

—
Reply to this email directly or view it on GitHub
#1785 (comment)
.

[mpaolini]

It bites me too: it showed up after an upgrade from 2.3.12 to 2.3.14. In the previous versions the editable field was completely ignored. Also, we could mention this (breaking, IMO) change in the changelogs.

FWIW +1

[mpaolini]

Moreover: this assertion triggers even when that very model field is specifically excluded from the serializer using Meta.fields or Meta.exclude

[mpaolini]

I think 04315c1 and ab5082d should be reverted: there are no tests for this new feature and it is not advertized in the changelogs and in the docs

[tomchristie]

Nope. We don't explicitly document every single aspect of how a ModelSerializer class determines it's default automatic set of fields. That doesn't mean it's incorrect for use to have behaviour there. We fix this bug, we don't just arbitrarily roll back functionality.

[mpaolini]

OK, bug fix and tests are needed then. All this came out because this new feature is completely ignored by the automatic tests

[tomchristie]

@mpaolini Volunteering? :)

[carltongibson]

Hey @mpaolini — Sorry you got stung by this one.

Do you think the patch here would suffice, especially given the rewrite coming in? Does @gdoermann's work-around work for you? Think you can pull this branch and add the failing test case?

Thanks!

[mpaolini]

hey @tomchristie @carltongibson yes I am volunteering, will provide some code later in the weekend. By the way, I just realized I personally know the guy who wrote the two "offending" commits. We were at uni together!