(BEDS-469) DA: notifications dashboard table #893
Conversation
…/notifications-dashboard-table
Deploying beaconchain with
|
| Latest commit: |
b2b8eef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ebc528ee.beaconchain.pages.dev |
| Branch Preview URL: | https://beds-469-notifications-dashb.beaconchain.pages.dev |
backend/pkg/api/types/data_access.go
Outdated
| Epoch uint64 | ||
| ChainId uint64 | ||
| DashboardName string | ||
| GroupName string |
There was a problem hiding this comment.
issue: This does not uniquely identify a row since dashboards and their groups can have the same name multiple times.
You would need the DashboardId & GroupId instead.
There was a problem hiding this comment.
Correct I missed that, thanks
Edit:
Thinking about it.. this might be a bit tricky. If we want the user to sort by dashboard/group name, we need to have both I think. If name is replaced by id it's possible to return duplicated rows.
I'll push that for now, can think of some examples and/or we discuss this in detail.
There was a problem hiding this comment.
Yes, in the cursor I would have all 4 of them for an easier query.
remoterami
force-pushed
the
BEDS-469/notifications-dashboard-table
branch
from
1b234eb to
0b1331d
Compare
| @@ -687,6 +687,9 @@ func (v *validationError) checkNetworkParameter(param string) uint64 { | |||
| } | |||
|
|
|||
| func (v *validationError) checkNetworksParameter(param string) []uint64 { | |||
| if param == "" { | |||
There was a problem hiding this comment.
@LuccaBitfly is this change ok with you?
| GroupName string `json:"group_name"` | ||
| 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' | 'validator_got_slashed' | 'validator_has_slashed' | 'incoming_tx' | 'outgoing_tx' | 'transfer_erc20' | 'transfer_erc721' | 'transfer_erc1155')[]" faker:"slice_len=2, oneof: validator_online, validator_offline, group_online, group_offline, attestation_missed, proposal_success, proposal_missed, proposal_upcoming, max_collateral, min_collateral, sync, withdrawal, validator_got_slashed, validator_has_slashed, incoming_tx, outgoing_tx, transfer_erc20, transfer_erc721, transfer_erc1155"` | ||
| IsAccountDashboard bool `db:"is_account_dashboard" json:"is_account_dashboard"` // if false it's a validator dashboard |
There was a problem hiding this comment.
@LuccaBitfly is this change ok with you?
…ions-dashboard-table
| @@ -146,23 +155,12 @@ func (d *DataAccessService) GetDashboardNotifications(ctx context.Context, userI | |||
| defaultColumns := []t.SortColumn{ | |||
There was a problem hiding this comment.
thought: Since the default column entries only require those that uniquely identify a row you don't need to add the dashboard and group names to it.
However if you change it this way you would need the offset if again.
Both ways work for me but consider that the query could be faster with less WHERE/ORDER BY.
There was a problem hiding this comment.
True, while the ids would be enough it's kind of a workaround that allows unique sorting by name at the cost of increased overhead.
I'd change it if the query becomes an issue
…ions-dashboard-table
No description provided.