-
Notifications
You must be signed in to change notification settings - Fork 34
DFI-1266 API scopes for disa-returns #119
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
Conversation
|
| "key":"read:isa-returns", | ||
| "name":"Access ISA returns reports", | ||
| "description":"Scope to access ISA returns reports" | ||
| }, |
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.
Is 'returns reports' a tautology like 'PIN number'?
I would go with:
- Access ISA returns
- Scope to access ISA returns
Given that new API name is going to be ISA Returns, we might need to default to using 'returns' in API schema and use 'report' or 'reports' only in descriptive text when talking specifically about monthly reports/reporting. At DISA Phase 2, if annual ISA statistical returns are added to API, they will always be referred to as 'returns'.
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.
I'm not too sure on that one tbh, I thought the report was the result of submitting ISA returns and the only data the user can access currently in API is the reconciliation reports but maybe thats my misunderstanding and probably limiting for future iterations on the API.
Happy to change to the suggestions 👍🏼
| "key":"write:isa-returns", | ||
| "name":"Submit ISA returns and ISA manager’s declaration", | ||
| "description":"Scope to submit ISA returns and the ISA manager’s declaration" | ||
| } |
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.
Can probably remove 'the'.
fmcgrath
left a comment
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.
2 minor comments but 1 of them is a discussion point.
No description provided.