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

[#293] update token configuration file namespaces #305

Merged
merged 2 commits into from
Dec 13, 2024

Conversation

SonnyBA
Copy link
Contributor

@SonnyBA SonnyBA commented Dec 12, 2024

Fixes #293

Changes

Renames the configuration file namespaces for token configuration

openklant_tokens_config_enable: true
openklant_tokens:
zgw_tokens_tokenauth_config_enable: true
zgw_tokens_tokenauth:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought we mentioned to call this tokenauth_config_enable? This prefix doesn't apply for Objects/Objecttypes, because those are not part of Zaakgericht werken

Copy link
Contributor Author

@SonnyBA SonnyBA Dec 12, 2024

Choose a reason for hiding this comment

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

@danielmursa-dev and I discussed that adding a prefix for the library that will be responsible for defining these tokens would be nice, because it takes away some work in advance as the configuration files will have the correct namespaces already, hence the zgw_tokens prefix. I guess the question here is, how are we going to call the library and use that prefix?

Edit: maybe something like commonground_tokens (so the namespaces becomes commonground_tokens_tokenauth)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm okay, I'll approve for now so that we can get the other PR in Objecttypes merged as well. We'll probably have to take a look at all the namespaces next week anyway

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see that in https://github.com/maykinmedia/objecttypes-api/pull/135/files this was changed to just tokenauth, could you change it to that as well? I think no prefix is better than a prefix that might be confusing

@stevenbal stevenbal self-requested a review December 13, 2024 08:48
@stevenbal stevenbal self-requested a review December 13, 2024 08:57
@SonnyBA SonnyBA merged commit 4715811 into master Dec 13, 2024
17 checks passed
@SonnyBA SonnyBA deleted the feature/#293-update-token-namespace branch December 13, 2024 10:33
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.

Add support for Tokens via django-setup-configuration
3 participants