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

Implement API fencing #56

Merged
merged 17 commits into from
Jan 19, 2021
Merged

Implement API fencing #56

merged 17 commits into from
Jan 19, 2021

Conversation

antonagestam
Copy link
Contributor

@antonagestam antonagestam commented Jan 15, 2021

Todo

  • Write docs
  • Document USE_TZ requirement
  • Guard against using USE_TZ = False
  • Coverage
  • OpenAPI schema

Using the allow_if_unmodified_since from this patch requires USE_TZ = True. In my opinion that's best practice anyway and reasonable requirement, let me know if you think we should support USE_TZ = False. We should document the requirement and look into ways of making friendly and early error messages.

@github-actions
Copy link

github-actions bot commented Jan 16, 2021

File Coverage Lines Branches Missing
All files 92% 97% 87%
__init__.py 83% 100% 67%
models.py 97% 100% 93%
url.py 97% 100% 94%
admin/extension.py 93% 100% 86%
admin/api/i18n.py 90% 80% 100% 9
admin/api/mixins.py 69% 75% 63% 70, 78-84, 94, 110, 117-128
admin/api/serializers.py 75% 100% 50%
admin/api/versioning.py 92% 100% 83%
admin/api/views.py 99% 99% 100% 134
admin/api/schemas/base.py 88% 75% 100% 3
admin/api/schemas/yasg.py 89% 96% 82% 77, 86, 125-126
drf/fencing.py 87% 95% 80% 80-86, 109
management/commands/syncpermissions.py 88% 95% 80% 31

Minimum allowed coverage is 85%

Generated by 🐒 cobertura-action against fc8e0bf

Copy link
Contributor

@hannseman hannseman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic! 🥇

Left a couple of small comments but looks really solid.

@antonagestam
Copy link
Contributor Author

The OpenAPI schema is currently looked botched when running this on SAPI (not sure why). I'll do some digging before merging.

@antonagestam
Copy link
Contributor Author

I had missed inheriting from BananasSwaggerSchema which botched the naming, and missed decorating partial_update, both are fixed now.

Copy link
Contributor

@hannseman hannseman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! 🚀

@antonagestam antonagestam merged commit 05088bf into master Jan 19, 2021
@antonagestam antonagestam deleted the feature/drf-update-fencing branch January 19, 2021 12:58
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.

2 participants