Skip to content

feat(site): add user activity on template insights #10013

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

Merged
merged 2 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
feat(site): add user activity on template insights
  • Loading branch information
BrunoQuaresma committed Oct 3, 2023
commit 786f43753c9307ba7605b5b46b129763fdaacd81
8 changes: 8 additions & 0 deletions site/src/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1488,6 +1488,14 @@ export const getInsightsUserLatency = async (
return response.data;
};

export const getInsightsUserActivity = async (
filters: InsightsParams,
): Promise<TypesGen.UserActivityInsightsResponse> => {
const params = new URLSearchParams(filters);
const response = await axios.get(`/api/v2/insights/user-activity?${params}`);
return response.data;
};

export type InsightsTemplateParams = InsightsParams & {
interval: "day" | "week";
};
Expand Down
7 changes: 7 additions & 0 deletions site/src/api/queries/insights.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,10 @@ export const insightsUserLatency = (params: API.InsightsParams) => {
queryFn: () => API.getInsightsUserLatency(params),
};
};

export const insightsUserActivity = (params: API.InsightsParams) => {
return {
queryKey: ["insights", "userActivity", params.template_ids, params],
queryFn: () => API.getInsightsUserActivity(params),
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
TemplateInsightsResponse,
TemplateParameterUsage,
TemplateParameterValue,
UserActivityInsightsResponse,
UserLatencyInsightsResponse,
} from "api/typesGenerated";
import { ComponentProps, ReactNode } from "react";
Expand All @@ -41,7 +42,11 @@ import Tooltip from "@mui/material/Tooltip";
import LinkOutlined from "@mui/icons-material/LinkOutlined";
import { InsightsInterval, IntervalMenu } from "./IntervalMenu";
import { WeekPicker, numberOfWeeksOptions } from "./WeekPicker";
import { insightsTemplate, insightsUserLatency } from "api/queries/insights";
import {
insightsTemplate,
insightsUserActivity,
insightsUserLatency,
} from "api/queries/insights";
import { useSearchParams } from "react-router-dom";

const DEFAULT_NUMBER_OF_WEEKS = numberOfWeeksOptions[0];
Expand All @@ -65,9 +70,11 @@ export default function TemplateInsightsPage() {
template_ids: template.id,
...getDateRangeFilter(dateRange),
};

const insightsFilter = { ...commonFilters, interval };
const { data: templateInsights } = useQuery(insightsTemplate(insightsFilter));
const { data: userLatency } = useQuery(insightsUserLatency(commonFilters));
const { data: userActivity } = useQuery(insightsUserActivity(commonFilters));

return (
<>
Expand Down Expand Up @@ -97,6 +104,7 @@ export default function TemplateInsightsPage() {
}
templateInsights={templateInsights}
userLatency={userLatency}
userActivity={userActivity}
interval={interval}
/>
</>
Expand Down Expand Up @@ -137,11 +145,13 @@ const getDateRange = (
export const TemplateInsightsPageView = ({
templateInsights,
userLatency,
userActivity,
controls,
interval,
}: {
templateInsights: TemplateInsightsResponse | undefined;
userLatency: UserLatencyInsightsResponse | undefined;
userActivity: UserActivityInsightsResponse | undefined;
controls: ReactNode;
interval: InsightsInterval;
}) => {
Expand All @@ -161,7 +171,7 @@ export const TemplateInsightsPageView = ({
sx={{
display: "grid",
gridTemplateColumns: "repeat(3, minmax(0, 1fr))",
gridTemplateRows: "440px auto",
gridTemplateRows: "440px 440px auto",
gap: (theme) => theme.spacing(3),
}}
>
Expand All @@ -170,11 +180,12 @@ export const TemplateInsightsPageView = ({
interval={interval}
data={templateInsights?.interval_reports}
/>
<UserLatencyPanel data={userLatency} />
<UsersLatencyPanel data={userLatency} />
<TemplateUsagePanel
sx={{ gridColumn: "span 3" }}
sx={{ gridColumn: "span 2" }}
data={templateInsights?.report?.apps_usage}
/>
<UsersActivityPanel data={userActivity} />
<TemplateParametersUsagePanel
sx={{ gridColumn: "span 3" }}
data={templateInsights?.report?.parameters_usage}
Expand Down Expand Up @@ -216,7 +227,7 @@ const ActiveUsersPanel = ({
);
};

const UserLatencyPanel = ({
const UsersLatencyPanel = ({
data,
...panelProps
}: PanelProps & { data: UserLatencyInsightsResponse | undefined }) => {
Expand Down Expand Up @@ -276,6 +287,65 @@ const UserLatencyPanel = ({
);
};

const UsersActivityPanel = ({
data,
...panelProps
}: PanelProps & { data: UserActivityInsightsResponse | undefined }) => {
const users = data?.report.users;

return (
<Panel {...panelProps} sx={{ overflowY: "auto", ...panelProps.sx }}>
<PanelHeader>
<PanelTitle sx={{ display: "flex", alignItems: "center", gap: 1 }}>
Activity by user
<HelpTooltip size="small">
<HelpTooltipTitle>How is activity calculated?</HelpTooltipTitle>
<HelpTooltipText>
When a connection is initiated to a user&apos;s workspace they are
considered an active user. e.g. apps, web terminal, SSH
</HelpTooltipText>
</HelpTooltip>
</PanelTitle>
</PanelHeader>
<PanelContent>
{!data && <Loader sx={{ height: "100%" }} />}
Copy link
Member

Choose a reason for hiding this comment

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

I don't like checking just data for loading. we do this in a lot of places, and it ends up with lots of places that show spinners instead of error messages.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I usually handle errors when they are expected like form errors, or possible user actions that can cause an "invalid" state. IMO handling all the errors caused by async can be too much and idk if that adds too much value, to be honest, but I'm open to that.

Maybe could be interesting to start a thread on dev about handling errors in the UI and see if we can come up with a more solid strategy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm also going to put some thoughts on that 🤔. Can we get this work merged and I open the PR to handle the error for all the Insights pages?

Copy link
Member

Choose a reason for hiding this comment

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

fair enough

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there anything else I should do to get your quality approval? 👀

Copy link
Member

Choose a reason for hiding this comment

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

Not going to treat this as a blocking comment, but does the whole data need to be made a prop, or would it be safe just to have the component receive the users, since that's the only part the component seems like it would ever care about?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it could but since its goal is to display the UserActivityInsightsResponse data I think it is ok to use it.

I think it is just easier to maintain and extend using the context object than passing prop by prop.

{users && users.length === 0 && <NoDataAvailable />}
{users &&
users
.sort((a, b) => b.seconds - a.seconds)
.map((row) => (
<Box
key={row.user_id}
sx={{
display: "flex",
justifyContent: "space-between",
fontSize: 14,
py: 1,
}}
>
<Box sx={{ display: "flex", alignItems: "center", gap: 1.5 }}>
<UserAvatar
username={row.username}
avatarURL={row.avatar_url}
/>
<Box sx={{ fontWeight: 500 }}>{row.username}</Box>
</Box>
<Box
css={(theme) => ({
color: theme.palette.text.secondary,
fontSize: 13,
textAlign: "right",
})}
>
{formatTime(row.seconds)}
</Box>
</Box>
))}
</PanelContent>
</Panel>
);
};

const TemplateUsagePanel = ({
data,
...panelProps
Expand All @@ -292,15 +362,21 @@ const TemplateUsagePanel = ({
// The API returns a row for each app, even if the user didn't use it.
const hasDataAvailable = validUsage && validUsage.length > 0;
return (
<Panel {...panelProps}>
<Panel {...panelProps} css={{ overflowY: "auto" }}>
<PanelHeader>
<PanelTitle>App & IDE Usage</PanelTitle>
</PanelHeader>
<PanelContent>
{!data && <Loader sx={{ height: 200 }} />}
{!data && <Loader sx={{ height: "100%" }} />}
{data && !hasDataAvailable && <NoDataAvailable sx={{ height: 200 }} />}
{data && hasDataAvailable && (
<Box sx={{ display: "flex", flexDirection: "column", gap: 3 }}>
<Box
sx={{
display: "flex",
flexDirection: "column",
gap: 3,
}}
>
{validUsage
.sort((a, b) => b.seconds - a.seconds)
.map((usage, i) => {
Expand Down