Skip to content
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.

Sign up for GitHub

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

(BEDS-481) API: dashboards notifications struct adjustments #873

Merged
merged 5 commits into from
Oct 7, 2024
Merged

(BEDS-481) API: dashboards notifications struct adjustments #873

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f0174b1
refactor: move response wrapping func to utils
LuccaBitfly Oct 2, 2024
b29696d
refactor: change dashboard notification types
LuccaBitfly Oct 2, 2024
b38dbbb
refactor: adapt dashboard notification table to type changes
LuccaBitfly Oct 2, 2024
facc404
refactor: update wrapWithIdentifier to support dynamic key generation
LuccaBitfly Oct 2, 2024
46ff8a2
refactor: rename wrapWithIdentifier to addIdentifier
LuccaBitfly Oct 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions backend/pkg/api/types/notifications.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ type InternalGetUserNotificationsResponse ApiDataResponse[NotificationOverviewDa
type NotificationDashboardsTableRow struct {
IsAccountDashboard bool `json:"is_account_dashboard"` // if false it's a validator dashboard
ChainId uint64 `json:"chain_id"`
Timestamp int64 `json:"timestamp"`
Epoch uint64 `json:"epoch"`
DashboardId uint64 `json:"dashboard_id"`
GroupId uint64 `json:"group_id"`
GroupName string `json:"group_name"`
NotificationId uint64 `json:"notification_id"` // may be string? db schema is not defined afaik
EntityCount uint64 `json:"entity_count"`
EventTypes []string `json:"event_types" tstype:"('validator_online' | 'validator_offline' | 'group_online' | 'group_offline' | 'attestation_missed' | 'proposal_success' | 'proposal_missed' | 'proposal_upcoming' | 'max_collateral' | 'min_collateral' | 'sync' | 'withdrawal' | 'got_slashed' | 'has_slashed' | 'incoming_tx' | 'outgoing_tx' | 'transfer_erc20' | 'transfer_erc721' | 'transfer_erc1155')[]" faker:"oneof: validator_offline, group_offline, attestation_missed, proposal_success, proposal_missed, proposal_upcoming, max_collateral, min_collateral, sync, withdrawal, slashed_own, incoming_tx, outgoing_tx, transfer_erc20, transfer_erc721, transfer_erc1155"`
EventTypes []string `json:"event_types" tstype:"('validator_online' | 'validator_offline' | 'group_online' | 'group_offline' | 'attestation_missed' | 'proposal_success' | 'proposal_missed' | 'proposal_upcoming' | 'max_collateral' | 'min_collateral' | 'sync' | 'withdrawal' | 'got_slashed' | 'has_slashed' | 'incoming_tx' | 'outgoing_tx' | 'transfer_erc20' | 'transfer_erc721' | 'transfer_erc1155')[]" faker:"slice_len=2, oneof: validator_offline, group_offline, attestation_missed, proposal_success, proposal_missed, proposal_upcoming, max_collateral, min_collateral, sync, withdrawal, slashed_own, incoming_tx, outgoing_tx, transfer_erc20, transfer_erc721, transfer_erc1155"`
}

type InternalGetUserNotificationDashboardsResponse ApiPagingResponse[NotificationDashboardsTableRow]
Expand Down
17 changes: 2 additions & 15 deletions frontend/components/dashboard/table/DashboardTableRewards.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,19 +137,6 @@ const findNextEpochDuties = (epoch: number) => {

return list.join(', ')
}

const wrappedRewards = computed(() => {
if (!rewards.value) {
return
}
return {
data: rewards.value.data.map(d => ({
...d,
identifier: `${d.epoch}-${d.group_id}`,
})),
paging: rewards.value.paging,
}
})
</script>

<template>
Expand All @@ -168,8 +155,8 @@ const wrappedRewards = computed(() => {
<template #table>
<ClientOnly fallback-tag="span">
<BcTable
:data="wrappedRewards"
data-key="identifier"
:data="wrapWithIdentifier(rewards, 'epoch', 'group_id')"
data-key="wrapped_identifier"
:expandable="true"
class="rewards-table"
:cursor
Expand Down
10 changes: 5 additions & 5 deletions frontend/components/notifications/DashboardsTable.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ const { overview } = useNotificationsDashboardOverviewStore()
<template #table>
<ClientOnly fallback-tag="span">
<BcTable
:data="notificationsDashboards"
data-key="notification_id"
:data="wrapWithIdentifier(notificationsDashboards, 'is_account_dashboard', 'dashboard_id', 'group_id', 'epoch')"
data-key="wrapped_identifier"
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@LuccaBitfly LuccaBitfly Oct 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

:expandable="!colsVisible.notifications"
:cursor
:page-size
Expand All @@ -89,7 +89,7 @@ const { overview } = useNotificationsDashboardOverviewStore()
</template>
</Column>
<Column
field="timestamp"
field="epoch"
sortable
header-class="col-age"
body-class="col-age"
Expand All @@ -99,8 +99,8 @@ const { overview } = useNotificationsDashboardOverviewStore()
</template>
<template #body="slotProps">
<BcFormatTimePassed
:value="slotProps.data.timestamp"
type="go-timestamp"
:value="slotProps.data.epoch"
type="epoch"
/>
</template>
</Column>
Expand Down
4 changes: 2 additions & 2 deletions frontend/types/api/notifications.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ export type InternalGetUserNotificationsResponse = ApiDataResponse<NotificationO
export interface NotificationDashboardsTableRow {
is_account_dashboard: boolean; // if false it's a validator dashboard
chain_id: number /* uint64 */;
timestamp: number /* int64 */;
epoch: number /* uint64 */;
dashboard_id: number /* uint64 */;
group_id: number /* uint64 */;
group_name: string;
notification_id: number /* uint64 */; // may be string? db schema is not defined afaik
entity_count: number /* uint64 */;
event_types: ('validator_online' | 'validator_offline' | 'group_online' | 'group_offline' | 'attestation_missed' | 'proposal_success' | 'proposal_missed' | 'proposal_upcoming' | 'max_collateral' | 'min_collateral' | 'sync' | 'withdrawal' | 'got_slashed' | 'has_slashed' | 'incoming_tx' | 'outgoing_tx' | 'transfer_erc20' | 'transfer_erc721' | 'transfer_erc1155')[];
}
Expand Down
19 changes: 19 additions & 0 deletions frontend/utils/wrapWithIdentifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import type { ApiPagingResponse } from '~/types/api/common'
/**
* 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,
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

np: something like that would be kewl

Suggested change
/**
* 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,
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe think about generalize this function (is it important for the function that there has to be a row?)

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Loading