-
Notifications
You must be signed in to change notification settings - Fork 633
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 my tickets view for logged in non-staff users #1143
Conversation
Note: This is a breaking change as it forces pagination on the API endoints. This should have been done from the start as the API without pagination is useless when there are large numbers of tickets.
…frontend now requires the API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me - giv en there are no unit tests to verify the functionality does what it should I will have to find time toi manually test this ... hopefully on the weekend.
I added unit tests for the API endpoint ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to work as described - just the one consideration noted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 - works as expected. Nice job.
This includes a breaking change as I forced pagination on the API.
Implements: #323
I also ended up switching to class based views for the public views.