florimondmanca / djangorestframework-api-key

🔐 API key permissions for Django REST Framework
https://florimondmanca.github.io/djangorestframework-api-key/
MIT License
669 stars 103 forks source link

Drop redundant .has_object_permission impl on DRF 3.14.0 or above #240

Closed florimondmanca closed 1 year ago

florimondmanca commented 1 year ago

Closes #150

This essentially reverts #25 when DRF 3.14.0 or above is detected.

Not reverting in all cases prevents breakage (i.e. API keys not being checked anymore if using them with a bitwise OR) if people upgrade DRF-api-key while still using an older DRF.

florimondmanca commented 1 year ago

@sparkyb Hi. I know it's been a while since you filed #150, but here's something that I think would resolve it. Would you like to review this PR so I get a fresh pair of eyes on it? Thanks!

florimondmanca commented 1 year ago

/azp run

codecov-commenter commented 1 year ago

Codecov Report

Merging #240 (a0ba0a5) into master (731c66a) will not change coverage. The diff coverage is 100.00%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff            @@
##            master      #240   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines          602       593    -9     
=========================================
- Hits           602       593    -9     
Files Changed Coverage Δ
src/rest_framework_api_key/permissions.py 100.00% <100.00%> (ø)
tests/test_permissions.py 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

florimondmanca commented 1 year ago

/AzurePipelines run

florimondmanca commented 1 year ago

There's something wrong with the Azure Pipelines integration -- builds aren't being triggered when I push to this branch. Need to trigger them manually… They do trigger on master though. :shrug:

florimondmanca commented 1 year ago

I'm not as familiar with the method you're using to do the version comparison

I've just learnt myself about this. Previously I would have used pkg_resources.parse_version() but it looks like it was deprecated in favor of packaging. https://stackoverflow.com/a/21065570

Thanks a lot for the quick review @sparkyb. I think we can move forward with this change.

azure-pipelines[bot] commented 1 year ago
Pipelines were unable to run due to time out waiting for the pull request to finish merging.
azure-pipelines[bot] commented 1 year ago
Pipelines were unable to run due to time out waiting for the pull request to finish merging.