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

Add more configurability to the library #6

Merged
merged 11 commits into from
Apr 18, 2023
Merged

Conversation

akikoskinen
Copy link
Contributor

@akikoskinen akikoskinen commented Apr 17, 2023

The first version of this library was very opinionated and assumed quite a lot from the project that would use it. It has since come obvious that those assumptions don't hold for many projects. Projects still using this library has had to circumvent the limitations in various ways. This PR offers configurability to the library so that projects wouldn't need to hack it so much.

There's another PR City-of-Helsinki/example-backend-profile#15 using these new features.

The versions of the tests that used the correct scope were redundant
because the correct scope is used in all the other tests. The point of
these tests is to check what happens when an incorrect scope is used and
that's what the tests now do.
There was a lot of repetition in the tests for the preparation for doing
the query request.
CHANGELOG.md Show resolved Hide resolved
tests/test_gdpr_api_delete.py Outdated Show resolved Hide resolved
There was a lot of repetition in the tests for the preparation for doing
the delete request.
The GDPR model instance was searched by its primary key field. This is
still the default behaviour, but now the field lookup can also be
changed with a `GDPR_API_MODEL_LOOKUP` setting.
In addition to being a Django field lookup value, the
`GDPR_API_MODEL_LOOKUP` setting can now also point to a function that
provides the GDPR model instance. That way the user of this library has
full control over how that instance gets found.
It's required that a User model can be obtained from the model instance
specified by the `GDPR_API_MODEL` setting. By default the model
instance's `user` attribute is tried. If that won't work, it's now
possible to configure a function with the `GDPR_API_USER_PROVIDER`
setting. That function can do anything that is needed to obtain the
user.
This was existing functionality, but missing any tests.
Raising a Django REST Framework `APIException` gets handled by the DRF's
exception handler. The default handler produces some JSON content into
the response. The exception handler can also be overridden by the user
in which case this library would have no control over what the response
contains. Better not use exceptions at all and just return a Response
instance.
The default data deletion behaviour, deleting the GDPR API model
instance, might not be the correct procedure in every case. Some
application might for example want to anonymise the data instead of
deletion. It's now possible to provide a function that gets called
instead of the default behaviour.
The function may return an `ErrorResponse` instance. If it does, the
database transaction gets rolled back and the returned instance is
serialized into the HTTP response, providing a correctly structured
response contents.
The URL pattern string can be given with the `GDPR_API_URL_PATTERN`
setting.

The setting affects the Djano URLConf, which is cached after the first
load. There's a trick in conftest.py that reloads the URLConfs every
time this setting changes. That trick isn't needed during runtime of the
app because settings shouldn't change during runtime.
@codecov-commenter
Copy link

Codecov Report

Merging #6 (a05c6dd) into main (1ece52b) will increase coverage by 2.36%.
The diff coverage is 95.16%.

❗ Current head a05c6dd differs from pull request most recent head 858deb1. Consider uploading reports for the commit 858deb1 to get more accurate results

@@            Coverage Diff             @@
##             main       #6      +/-   ##
==========================================
+ Coverage   92.23%   94.59%   +2.36%     
==========================================
  Files           4        5       +1     
  Lines         103      148      +45     
  Branches       14       24      +10     
==========================================
+ Hits           95      140      +45     
+ Misses          5        4       -1     
- Partials        3        4       +1     
Impacted Files Coverage Δ
helsinki_gdpr/views.py 96.11% <94.11%> (+1.91%) ⬆️
helsinki_gdpr/types.py 100.00% <100.00%> (ø)
helsinki_gdpr/urls.py 100.00% <100.00%> (ø)

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

@akikoskinen akikoskinen merged commit 858deb1 into main Apr 18, 2023
@akikoskinen akikoskinen deleted the configurability branch April 18, 2023 11:43
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.

None yet

3 participants