Skip to content

Conversation

@wh0th3h3llam1
Copy link

  • Add AccessToken model
  • Create API for AccessToken
  • Add tests

Copy link
Owner

@mrmn2 mrmn2 left a comment

Choose a reason for hiding this comment

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

Thank you for the PR and sorry for the late review. I have added some minor corrections. Additionally the code quality job has failed, so you need to fix the findings of flake8. I also suggest to run black locally so the code is correctly formatted. Otherwise the black step will probably also fail. Take a look at the development docs.

@wh0th3h3llam1
Copy link
Author

@mrmn2 , do you want me to add the UI part of creating/updating/deleting tokens in this PR or a different one?

@mrmn2
Copy link
Owner

mrmn2 commented Oct 11, 2025

@mrmn2 , do you want me to add the UI part of creating/updating/deleting tokens in this PR or a different one?

I have created an Issue with some basic information: #171. I might have forgotten something, feel free to reach out.

@wh0th3h3llam1 wh0th3h3llam1 requested a review from mrmn2 October 13, 2025 18:09
# Create your views here.


class AccessTokenViewSet(ModelViewSet):
Copy link
Owner

Choose a reason for hiding this comment

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

I thought about this some more. If I am not mistaken the ModelViewSet will, amongst others add the create and destroy action. I don't think this is really needed. Creating and destroying tokens should be done via the UI. As the PdfDing is using Django's template engine token creation and deletion can be handled directly by Django instead of the API. Or did I miss something? Sorry that I did only think about this now.

Copy link
Author

Choose a reason for hiding this comment

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

Right, I got your point. I'll remove Token Creation/Deletion from API.

  • What about token rotation? That should also not be allowed
  • For updation, only thing that can be updated is name.

So for this viewset, only update api is required, with only name that can be updated.

I'll still keep all the serializers, so that when UI part is done, it can be reused/referred

Copy link
Owner

Choose a reason for hiding this comment

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

I am very sorry for the late reply. Things will be better from on as I can dedicate more time on PdfDing again. Yes rotation should also only be done via the UI. As you mentioned you can keep the serializers.

Copy link
Owner

Choose a reason for hiding this comment

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

Also you'll need to rebase on the repo"s master branch as I updated pyproject.yaml and poetry.lock which will cause merge conflicts.

Copy link
Owner

Choose a reason for hiding this comment

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

You need to regenerate the lock file via poetry lock as the tests will not run otherwise.

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