Add editor functionality

[vertliba]

[codecov-io]

Codecov Report

Merging #52 into master will not change coverage. The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #52   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      4    +1     
  Lines         218    271   +53     
=====================================
+ Hits          218    271   +53

Impacted Files	Coverage Δ
rest_framework_datatables/viewsets.py	100% <100%> (ø)

---

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 613a636...c47fc10. Read the comment docs.

[izimobil]

Hi, thanks for the pull request.

As is, it can't be merged though, here's the requirements for merging it:

• don't change anything in the README, docs, examples, etc... IMHO, you should just add a section for the Editor (optional) functionality, but don't change existing stuff that is still valid.
• There are too many unused static files in the example app, we should only ship necessary files.
• the getCookie / CSRF js code should be in a js file, so the end user just have to include it and don't have to write the code each time (add a note in the relevant documentation section)
• why did you change get_genres, get_DT_RowID and get_DT_RowAttr to static methods ?
• why did you drop support for DRF < 3.9 ?
• please rename get_post_date to get_post_data, it's confusing.

Regards.

[vertliba]

Hi! Thanks for your review.

Here are my comments on your questions:

• don't change anything in the README, docs, examples, etc... IMHO, you should just add a section for the Editor (optional) functionality, but don't change existing stuff that is still valid.

I can just create the new section for the new readme, i will have to add another example there. But in this case, the readme will increase greatly. Or I can make the readme as similar as possible to yours, but to change example. I guess this is a good way. What do you think?

• There are too many unused static files in the example app, we should only ship necessary files.

I'll remove unused static files.

• the getCookie / CSRF js code should be in a js file, so the end user just have to include it and don't have to write the code each time (add a note in the relevant documentation section)

Ok.

• why did you change get_genres, get_DT_RowID and get_DT_RowAttr to static methods ?

These methods don't use object instances, so converting them reduces memory usage and it just why the static methods were created in python. Or maybe I haven't considered something?

• why did you drop support for DRF < 3.9 ?

In earlier versions of DRF, deserialization of internal serializers was different. Because of this the implementation of the support of earlier versions can't be done graceful and required more efforts.

• please rename get_post_date to get_post_data, it's confusing.

Ok.

Regard.

[izimobil]

Hi! Thanks for your review. Here are my comments on your questions:

• don't change anything in the README, docs, examples, etc... IMHO, you should just add a section for the Editor (optional) functionality, but don't change existing stuff that is still valid.

I can just create the new section for the new readme, i will have to add another example there. But in this case, the readme will increase greatly. Or I can make the readme as similar as possible to yours, but to change example. I guess this is a good way. What do you think?

Maybe you can just add the missing code parts for the example ? just the different javascript code ? I'm not too concerned about the README getting bigger, the more docs, the better.

• There are too many unused static files in the example app, we should only ship necessary files.

I'll remove unused static files.

• the getCookie / CSRF js code should be in a js file, so the end user just have to include it and don't have to write the code each time (add a note in the relevant documentation section)

Ok.

• why did you change get_genres, get_DT_RowID and get_DT_RowAttr to static methods ?

These methods don't use object instances, so converting them reduces memory usage and it just why the static methods were created in python. Or maybe I haven't considered something?

You are right in the sense that they don't use object instances by default in this particular example, but one may use the self (serializer instance) argument to do more fancy stuff like accessing the serializer attributes or other methods. With static methods such things would be impossible to do, so I would keep it as is.

• why did you drop support for DRF < 3.9?

In earlier versions of DRF, deserialization of internal serializers was different. Because of this the implementation of the support of earlier versions can't be done graceful and required more efforts.

Hmm, that's a bit unfortunate. Can you please give me a link to the relevant changes in DRF? I can't find any note of these changes in the changelog...

• please rename get_post_date to get_post_data, it's confusing.

Ok.

Best regards.

[vertliba]

You are right in the sense that they don't use object instances by default in this particular example, but one may use the self (serializer instance) argument to do more fancy stuff like accessing the serializer attributes or other methods. With static methods such things would be impossible to do, so I would keep it as is.

Ok.

• why did you drop support for DRF < 3.9?

In earlier versions of DRF, deserialization of internal serializers was different. Because of this the implementation of the support of earlier versions can't be done graceful and required more efforts.

Hmm, that's a bit unfortunate. Can you please give me a link to the relevant changes in DRF? I can't find any note of these changes in the changelog...

Here is this commit: https://github.com/encode/django-rest-framework/commit/587058e3c25aac4d871828a3ef19637eb9e8ddbd

[MarkoBox]

Hi, I'm currently testing this commit and so far its working fine. It would be great if it would get merged. @VVyacheslav if you haven't got time for this i'm volunteering to do the cleanup on new pr.

[JacoBezuidenhout]

any progress on this? At the moment I am using the django-rest-framework-datatables-editor library but it is unmaintained and does not support django 3 due to one import line in pagination.py

We would love to use your library rather.

[izimobil]

Hi all,

I would be more than happy to merge this PR once all the points I noted in my previous comment are addressed.

For the record, what's need to be done:

• Don't change anything in the README, docs, examples, etc... IMHO, you should just add a section for the Editor (optional) functionality, but don't change existing stuff that is still valid.
• There are too many unused static files in the example app, we should only ship necessary files.
• The getCookie / CSRF js code should be in a js file, so the end user just have to include it and don't have to write the code each time (add a note in the relevant documentation section)
• Why did you change get_genres, get_DT_RowID and get_DT_RowAttr to static methods ?
• Please rename get_post_date to get_post_data, it's confusing.
• Why did you drop support for DRF < 3.9 ? (Update: Now that DRF is 3.12, I guess we can live with that.)

@MarkoBox , @JacoBezuidenhout , @VVyacheslav : Help wanted, as I don't own a Editor license.