FFT-187 Adding notes api for payroll #624
Conversation
DenisDDBT
commented
Feb 18, 2025
DenisDDBT
commented
- JIRA ticket referenced in title
- Title is clear and concise
- Description gives any relevant detail
- Tests are up to date
- Documentation is up to date
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
DenisDDBT
force-pushed
the
FFT-187/notes-api
branch
from
78470e9 to
6016fe8
Compare
adding service function fixing sort order adding error handleing fixing error handler adding basic tests
DenisDDBT
force-pushed
the
FFT-187/notes-api
branch
from
6016fe8 to
3105eeb
Compare
DenisDDBT
force-pushed
the
FFT-187/notes-api
branch
2 times, most recently
from
c9cf14b to
d441dab
Compare
| @@ -83,32 +83,64 @@ export async function getData(url) { | |||
| */ | |||
| export async function postData(url = "", data = {}, headers = {}) { | |||
| const csrftoken = window.CSRF_TOKEN; | |||
| try { | |||
There was a problem hiding this comment.
i feel uneasy about some of the changes made here. i need to spend more time here.
| max-width: 100%; | ||
| width: 200px; | ||
| display: inline-block; | ||
| transform: translateY(6px); |
There was a problem hiding this comment.
it is, Paul has asked for ellipsis which is not active if your element is inline, and if I convert it to inline-block, line heights etc are getting different to the next link element. I could use flex or grid but they also come with different caveats.
There was a problem hiding this comment.
idea: could we handle it in javascript and truncate the string there?
There was a problem hiding this comment.
As a rule of thumb, I try to avoid css task to be outsourced to js. Having said that, if you are keen to do it in js, sure, I can add some function to truncate the text and add ... if content is more thatn 20 chars
payroll/api.py
Outdated
| return JsonResponse({}, status=204) | ||
| except json.JSONDecodeError: | ||
| return JsonResponse({"error": "Invalid JSON format"}, status=400) | ||
| except ValidationError: |
There was a problem hiding this comment.
i can't see anything that would return a validation error?
the service function raises value error, did you mean that?
| @@ -19,6 +19,16 @@ | |||
| EditPayrollApiView.as_view(), | |||
| name="api", | |||
| ), | |||
| path( | |||
| "api/<str:cost_centre_code>/<int:financial_year>/vacancies/notes", | |||
There was a problem hiding this comment.
we need to implement this for vacancies as well
do you want to do this as part of this ticket or a separate one?
There was a problem hiding this comment.
This was my point I raised yesterday, is there any need to add it if logic and params are the same?
There was a problem hiding this comment.
your API endpoint will need to handle both vacancies and employees which it currently doesn't
