(BEDS-481) API: dashboards notifications struct adjustments #873
Conversation
Deploying beaconchain with
|
| Latest commit: |
46ff8a2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4963b06c.beaconchain.pages.dev |
| Branch Preview URL: | https://beds-481-api-dashboards-noti.beaconchain.pages.dev |
See: BEDS-481
See: BEDS-481
LuccaBitfly
force-pushed
the
BEDS-481/api-dashboards-notifications-struct-changes
branch
from
31ab2ee to
b38dbbb
Compare
- Modified `wrapWithIdentifier` function to accept multiple keys instead of a single `getIdFromRow` function. - The unique identifier `wrapped_identifier` is now generated by joining the values of the specified keys with a dash (`-`). - Adapted all usages of `wrapWithIdentifier` See: BEDS-481
frontend/utils/wrapWithIdentifier.ts
Outdated
| /** | ||
| * This function wraps each element in an `ApiPagingResponse` interface with a unique id field `wrapped_identifier`. | ||
| * @param response The api response to wrap | ||
| * @param getIdFromRow A function that takes a row and builds a unique identifier | ||
| * @returns The wrapped response | ||
| */ | ||
| export function wrapWithIdentifier<T>(response: ApiPagingResponse<T> | undefined, ...keys: (keyof T)[]) { | ||
| if (!response) { | ||
| return | ||
| } | ||
| return { | ||
| data: response.data.map(row => ({ | ||
| ...row, | ||
| wrapped_identifier: keys.map(key => row[key]).join('-'), | ||
| })), | ||
| paging: response.paging, | ||
| } | ||
| } |
There was a problem hiding this comment.
np: something like that would be kewl
| /** | |
| * This function wraps each element in an `ApiPagingResponse` interface with a unique id field `wrapped_identifier`. | |
| * @param response The api response to wrap | |
| * @param getIdFromRow A function that takes a row and builds a unique identifier | |
| * @returns The wrapped response | |
| */ | |
| export function wrapWithIdentifier<T>(response: ApiPagingResponse<T> | undefined, ...keys: (keyof T)[]) { | |
| if (!response) { | |
| return | |
| } | |
| return { | |
| data: response.data.map(row => ({ | |
| ...row, | |
| wrapped_identifier: keys.map(key => row[key]).join('-'), | |
| })), | |
| paging: response.paging, | |
| } | |
| } | |
| /** | |
| * Sometimes the api does not provide a `unique id` for objects in an array. | |
| * But in `frontend` there are `components` that need to differentiate between the items. | |
| * So an identifier for each item is constructed out of a combination of the `object`. | |
| */ | |
| export function addIdentifier<T>(response: ApiPagingResponse<T> | undefined, ...keys: (keyof T)[]) { | |
| if (!response) { | |
| return | |
| } | |
| return { | |
| data: response.data.map(row => ({ | |
| ...row, | |
| identifier: keys.map(key => row[key]).join('-'), | |
| })), | |
| paging: response.paging, | |
| } | |
| } |
There was a problem hiding this comment.
maybe think about generalize this function (is it important for the function that there has to be a row?)
There was a problem hiding this comment.
Pushed the changes, ptal.
As for the generalization, since this func takes ApiPagingResponses, which only concern tables, the function can expect there to be rows. If we want to add identifiers so something other than ApiPagingResponses, we can think about generalizing more.
| <BcTable | ||
| :data="notificationsDashboards" | ||
| data-key="notification_id" | ||
| :data="wrapWithIdentifier(notificationsDashboards, 'is_account_dashboard', 'dashboard_id', 'group_id', 'epoch')" | ||
| data-key="wrapped_identifier" |
There was a problem hiding this comment.
when using <BcTable> and using wrapedWithIdentifier function, which has the consequence that data-key would be always the wrapped_identifier key...
It would be nice if BcTable would be refactored that data-key is not needed anymore
There was a problem hiding this comment.
Do we want to refactor BcTable to do that though? As far as the prime vue component goes, data-key is optional and only optimizes performance, which may be lost if we wrap every time when using BcTable. I would suggest investigating whether BcTable truly needs a unique ID first (at a later time) and keeping this for now.
See: BEDS-481
LuccaBitfly
force-pushed
the
BEDS-481/api-dashboards-notifications-struct-changes
branch
from
becd3d3 to
46ff8a2
Compare
| export function addIdentifier<T>(response: ApiPagingResponse<T> | undefined, ...keys: (keyof T)[]) { | ||
| if (!response) { | ||
| return | ||
| } | ||
| return { | ||
| data: response.data.map(row => ({ | ||
| ...row, | ||
| identifier: keys.map(key => row[key]).join('-'), | ||
| })), | ||
| paging: response.paging, | ||
| } | ||
| } |
There was a problem hiding this comment.
np: this should work too and is a bit cleaner imo
| export function addIdentifier<T>(response: ApiPagingResponse<T> | undefined, ...keys: (keyof T)[]) { | |
| if (!response) { | |
| return | |
| } | |
| return { | |
| data: response.data.map(row => ({ | |
| ...row, | |
| identifier: keys.map(key => row[key]).join('-'), | |
| })), | |
| paging: response.paging, | |
| } | |
| } | |
| export function addIdentifier<T>( | |
| pagingResponse: ApiPagingResponse<T> | undefined, | |
| keys: (keyof T)[], | |
| ) { | |
| if (!pagingResponse) { | |
| return | |
| } | |
| return { | |
| ...pagingResponse, | |
| data: pagingResponse.data | |
| .map(item => ({ | |
| ...item, | |
| identifier: keys.map(key => item[key]).join('-'), | |
| })), | |
| } | |
| } |
There was a problem hiding this comment.
But again, imo this is a code smell and could potentially introduce performance issues in large data sets.
As this will be done for every row in a table.
I still think data manipulation should happen in backend.
LuccaBitfly
deleted the
BEDS-481/api-dashboards-notifications-struct-changes
branch
No description provided.