Porting to REST Framework 2 #142

Merged
merged 30 commits into from Dec 21, 2012

Conversation

Projects
None yet
5 participants
@markotibold
Contributor

markotibold commented Nov 13, 2012

Don't pull this one in yet.

Parking this one here for now until RF2 supports File and Image ModelSerializers (soon). Which means you cannot upload files or images just yet.

Until then feel free to leave comments, test it, or just take a look at the code.

@markotibold

This comment has been minimized.

Show comment Hide comment
@markotibold

markotibold Nov 13, 2012

Contributor

Work on ImageField and FileField in progress, see: tomchristie/django-rest-framework#408

Contributor

markotibold commented Nov 13, 2012

Work on ImageField and FileField in progress, see: tomchristie/django-rest-framework#408

@markotibold

This comment has been minimized.

Show comment Hide comment
@markotibold

markotibold Nov 17, 2012

Contributor

Ready for being pulled in. Port complete.

Contributor

markotibold commented Nov 17, 2012

Ready for being pulled in. Port complete.

@mvdwaeter

This comment has been minimized.

Show comment Hide comment
@mvdwaeter

mvdwaeter Dec 3, 2012

Contributor

Tested, in the frontend everything works fine.

However, in the backend (with API_RENDER_HTML=True) you can add/edit fiber content without staff permissions (!)
Hence, permissions should be added to the views of the rest_api.
Also, the forms to create/update a new Page aren't complete (e.g. the field title is missing, and redirect_page is required). Also the url field seems to be different than the Page.url field. It requires an absolute url instead of a Fiber url (relative, named, absolute)

When entering an absolute url, I get the error:
Cannot set values on a ManyToManyField which specifies an intermediary model. Use fiber.PageContentItem's Manager instead.

In general, projects which also use django-restframework as a dependecy for other apps than fiber are going to break.
Hence, this should be clear in the version tag in Fiber. Maybe this is also a good time to start adding a Changelog in the README.rst

Contributor

mvdwaeter commented Dec 3, 2012

Tested, in the frontend everything works fine.

However, in the backend (with API_RENDER_HTML=True) you can add/edit fiber content without staff permissions (!)
Hence, permissions should be added to the views of the rest_api.
Also, the forms to create/update a new Page aren't complete (e.g. the field title is missing, and redirect_page is required). Also the url field seems to be different than the Page.url field. It requires an absolute url instead of a Fiber url (relative, named, absolute)

When entering an absolute url, I get the error:
Cannot set values on a ManyToManyField which specifies an intermediary model. Use fiber.PageContentItem's Manager instead.

In general, projects which also use django-restframework as a dependecy for other apps than fiber are going to break.
Hence, this should be clear in the version tag in Fiber. Maybe this is also a good time to start adding a Changelog in the README.rst

@mvdwaeter

This comment has been minimized.

Show comment Hide comment
@mvdwaeter

mvdwaeter Dec 3, 2012

Contributor

A 'nice to have' would be that on "/api/v2/pages/1/move_page/" the form has two fields: position & target_node_id instead of one big json field.

However, I think this is an issue from django-restframework.
Im using djangorestframework==2.1.6.

Contributor

mvdwaeter commented Dec 3, 2012

A 'nice to have' would be that on "/api/v2/pages/1/move_page/" the form has two fields: position & target_node_id instead of one big json field.

However, I think this is an issue from django-restframework.
Im using djangorestframework==2.1.6.

@nvandijk

This comment has been minimized.

Show comment Hide comment
@nvandijk

nvandijk Dec 20, 2012

Contributor

@markotibold Can you rebase your rf2 branch on the latest master branch, then we can test it with the latest version of django tiber

Contributor

nvandijk commented Dec 20, 2012

@markotibold Can you rebase your rf2 branch on the latest master branch, then we can test it with the latest version of django tiber

@markotibold

This comment has been minimized.

Show comment Hide comment
@markotibold

markotibold Dec 21, 2012

Contributor

Done.

On Dec 21, 2012, at 12:16 AM, Niels van Dijk notifications@github.com wrote:

@markotibold Can you rebase your rf2 branch on the latest master branch, then we can test it with the latest version of django tiber


Reply to this email directly or view it on GitHub.

Contributor

markotibold commented Dec 21, 2012

Done.

On Dec 21, 2012, at 12:16 AM, Niels van Dijk notifications@github.com wrote:

@markotibold Can you rebase your rf2 branch on the latest master branch, then we can test it with the latest version of django tiber


Reply to this email directly or view it on GitHub.

@markotibold

This comment has been minimized.

Show comment Hide comment
@markotibold

markotibold Dec 21, 2012

Contributor

@nvandijk don't pull this in yet.

I found out that posting a new content item is broken. Further investigation is needed.

Contributor

markotibold commented Dec 21, 2012

@nvandijk don't pull this in yet.

I found out that posting a new content item is broken. Further investigation is needed.

@markotibold

This comment has been minimized.

Show comment Hide comment
@markotibold

markotibold Dec 21, 2012

Contributor

Nevermind, I had the API_RENDER_HTML setting set, which obviously admin.js does not like.

Perhaps we should set the accept header top application/json in our api requests.

Contributor

markotibold commented Dec 21, 2012

Nevermind, I had the API_RENDER_HTML setting set, which obviously admin.js does not like.

Perhaps we should set the accept header top application/json in our api requests.

markotibold added some commits Nov 10, 2012

First stab at porting to rf2.
Ported PageList and PageDetail.
Rename view names.
Add IsAuthenticated permission class.
Use HyperlinkedModelSerializer for images and files so that their url…
… is serialized as well. Otherwise DELETE will not work.
Converted README from rst to markdown.
Added a compatiblity warning for projects using REST Framework 0.3.X or
0.4.X.
Added a changelog.
Dropped redundant INSTALL.rst.

dbunskoek added a commit that referenced this pull request Dec 21, 2012

Merge pull request #142 from markotibold/rf2
Porting to REST Framework 2, thanks @markotibold

@dbunskoek dbunskoek merged commit 026cea8 into django-fiber:master Dec 21, 2012

1 check passed

default The Travis build passed
Details
@coveralls

This comment has been minimized.

Show comment Hide comment
@coveralls

coveralls Feb 28, 2014

Coverage Status

Changes Unknown when pulling f028db8 on markotibold:rf2 into * on ridethepony:master*.

Coverage Status

Changes Unknown when pulling f028db8 on markotibold:rf2 into * on ridethepony:master*.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment