-
Notifications
You must be signed in to change notification settings - Fork 21
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
Improve node activities and add first draft for global activities #5747
Conversation
pa-lem
commented
Feb 13, 2025
•
edited
Loading
edited
- Add limit and link on the node activities view
- Update details cards UI to not grow
- Add starting point for the global activities list view + menu link to this view
- Handle group member events in both details and list views
- Add global activities pagination and search


…-global-activities
CodSpeed Performance ReportMerging #5747 will not alter performanceComparing Summary
|
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.
the backend change looks good
offset, | ||
limit, | ||
search, | ||
}: { ids?: Array<string | undefined>; offset?: number; limit?: number; search?: string }) { | ||
const currentBranchName = getCurrentBranchName(); | ||
const timeMachineDate = store.get(datetimeAtom); | ||
|
||
return queryOptions({ |
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.
ideally, it should be an infiniteQueryOptions
} | ||
} | ||
} | ||
`; | ||
|
||
export function getEventsFromApi({ |
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.
this file name should be get-events-from-api.ts
return graphqlClient.query({ | ||
query: gql(getEventsQuery({ ids })), | ||
query: gql(EVENTS_QUERY), |
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.
gql should be part of EVENTS_QUERY
{"attributes" in props && <NodeEvent {...props} />} | ||
|
||
{"attributes" in props && <EventAttributes attributes={props.attributes} />} |
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.
same condition
const { event, branch } = props; | ||
|
||
return ( | ||
<> |
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.
fragment is useless no?
@@ -22,11 +25,39 @@ export const GlobalEvents = () => { | |||
return edge.node; | |||
}); | |||
|
|||
const count = data?.data?.[INFRAHUB_EVENT]?.count; |
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.
this kind of destructuring are better to be done in useEvents
directly. The goal is to have data read to use instantly when calling useEvents
const { event, account_id, primary_node } = props; | ||
|
||
return ( | ||
<> |
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.
fragment useless no?
import { EventNodeInterface } from "@/shared/api/graphql/generated/graphql"; | ||
import { ReactElement } from "react"; | ||
|
||
export const GROUP_EVENTS_MAPPING: Record<string, (node: string, group: string) => ReactElement> = { |
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.
ReactNode instead of ReactElement is "more correct"
@@ -32,6 +40,14 @@ export const NodeEvents = () => { | |||
{activities?.map((activity) => ( | |||
<Event key={activity.id} {...activity} /> | |||
))} | |||
|
|||
{data?.data?.[INFRAHUB_EVENT]?.count > MAX_EVENTS && ( |
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.
same things, it's better to put destructuration inside of useEvents
return [searchFilter ?? "", setSearch]; | ||
}; | ||
|
||
export default useSearch; |
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 prefer to avoid default export to enforce function name
|
||
const MAX_EVENTS = 5; | ||
|
||
export const NodeEvents = () => { | ||
const { objectid } = useParams(); | ||
|
||
const { isLoading, data, error } = useEvents({ ids: [objectid], limit: MAX_EVENTS }); | ||
const { isLoading, data, count, error } = useEvents({ ids: [objectid], limit: MAX_EVENTS }); |
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.
if you check isPending + error, data is guarantee to be available after
…nto ple-global-activities