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.

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

UI: remove initial date from client counts #27816

Merged
merged 16 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 3 additions & 0 deletions changelog/27816.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: remove initial start/end parameters on the activity call for client counts dashboard.
```
41 changes: 26 additions & 15 deletions ui/app/adapters/clients/activity.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,31 +8,42 @@ import { formatDateObject } from 'core/utils/client-count-utils';
import { debug } from '@ember/debug';

export default class ActivityAdapter extends ApplicationAdapter {
formatTimeParam(dateObj, isEnd = false) {
let formatted;
if (dateObj) {
try {
const iso = dateObj.timestamp || formatDateObject(dateObj, isEnd);
formatted = iso;
} catch (e) {
// carry on
}
}
return formatted;
}
// javascript localizes new Date() objects but all activity log data is stored in UTC
// create date object from user's input using Date.UTC() then send to backend as unix
// time params from the backend are formatted as a zulu timestamp
formatQueryParams(queryParams) {
if (queryParams?.current_billing_period) {
// { current_billing_period: true } automatically queries the activity log
// from the builtin license start timestamp to the current month
return queryParams;
const query = {};
const start = this.formatTimeParam(queryParams?.start_time);
const end = this.formatTimeParam(queryParams?.end_time, true);
if (start) {
query.start_time = start;
}
let { start_time, end_time } = queryParams;
start_time = start_time.timestamp || formatDateObject(start_time);
end_time = end_time.timestamp || formatDateObject(end_time, true);
return { start_time, end_time };
if (end) {
query.end_time = end;
}
return query;
}

queryRecord(store, type, query) {
const url = `${this.buildURL()}/internal/counters/activity`;
const queryParams = this.formatQueryParams(query);
if (queryParams) {
return this.ajax(url, 'GET', { data: queryParams }).then((resp) => {
const response = resp || {};
response.id = response.request_id || 'no-data';
return response;
});
}
return this.ajax(url, 'GET', { data: queryParams }).then((resp) => {
const response = resp || {};
response.id = response.request_id || 'no-data';
return response;
});
}

urlForFindRecord(id) {
Expand Down
14 changes: 3 additions & 11 deletions ui/app/components/clients/activity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
// contains getters that filter and extract data from activity model for use in charts

import Component from '@glimmer/component';
import { isSameMonth, fromUnixTime } from 'date-fns';
import { isSameMonth } from 'date-fns';
import { parseAPITimestamp } from 'core/utils/date-formatters';
import { calculateAverage } from 'vault/utils/chart-helpers';
import { filterVersionHistory, hasMountsKey, hasNamespacesKey } from 'core/utils/client-count-utils';
Expand All @@ -24,8 +24,8 @@ import type {
interface Args {
activity: ClientsActivityModel;
versionHistory: ClientsVersionHistoryModel[];
startTimestamp: number;
endTimestamp: number;
startTimestamp: string;
endTimestamp: string;
namespace: string;
mountPath: string;
}
Expand All @@ -40,14 +40,6 @@ export default class ClientsActivityComponent extends Component<Args> {
return calculateAverage(data, key);
};

get startTimeISO() {
return fromUnixTime(this.args.startTimestamp).toISOString();
}

get endTimeISO() {
return fromUnixTime(this.args.endTimestamp).toISOString();
}

get byMonthActivityData() {
const { activity, namespace } = this.args;
return namespace ? this.filteredActivityByMonth : activity.byMonth;
Expand Down
4 changes: 2 additions & 2 deletions ui/app/components/clients/date-range.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
@text="Set date range"
@icon="edit"
{{on "click" (fn (mut this.showEditModal) true)}}
data-test-set-date-range
data-test-date-range-edit
/>
{{/if}}
</div>
Expand Down Expand Up @@ -87,7 +87,7 @@
data-test-date-range-validation
>{{this.validationError}}</Hds::Form::Error>
{{/if}}
{{#if this.useDefaultDates}}
{{#if (and this.version.isEnterprise this.useDefaultDates)}}
<Hds::Alert @type="compact" @color="highlight" class="has-top-margin-xs" data-test-range-default-alert as |A|>
<A.Description>Dashboard will use the default date range from the API.</A.Description>
</Hds::Alert>
Expand Down
4 changes: 2 additions & 2 deletions ui/app/components/clients/date-range.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ export default class ClientsDateRangeComponent extends Component<Args> {
}

get validationError() {
if (this.useDefaultDates) {
// this means we want to reset, which is fine
if (this.useDefaultDates && this.version.isEnterprise) {
// this means we want to reset, which is fine for ent only
return null;
}
if (!this.startDate || !this.endDate) {
Expand Down
4 changes: 2 additions & 2 deletions ui/app/components/clients/page/counts.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
</p>

<Clients::DateRange
@startTime={{this.startTimestampISO}}
@endTime={{this.endTimestampISO}}
@startTime={{@startTimestamp}}
@endTime={{@endTimestamp}}
@onChange={{this.onDateChange}}
class="has-bottom-margin-l"
/>
Expand Down
30 changes: 11 additions & 19 deletions ui/app/components/clients/page/counts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import Component from '@glimmer/component';
import { service } from '@ember/service';
import { action } from '@ember/object';
import { fromUnixTime, isSameMonth, isAfter } from 'date-fns';
import { isSameMonth, isAfter } from 'date-fns';
import { parseAPITimestamp } from 'core/utils/date-formatters';
import { filterVersionHistory } from 'core/utils/client-count-utils';

Expand All @@ -21,11 +21,11 @@ interface Args {
activity: ClientsActivityModel;
activityError?: AdapterError;
config: ClientsConfigModel;
endTimestamp: number;
endTimestamp: string; // ISO format
mountPath: string;
namespace: string;
onFilterChange: CallableFunction;
startTimestamp: number;
startTimestamp: string; // ISO format
versionHistory: ClientsVersionHistoryModel[];
}

Expand All @@ -34,29 +34,21 @@ export default class ClientsCountsPageComponent extends Component<Args> {
@service declare readonly version: VersionService;
@service declare readonly store: StoreService;

get startTimestampISO() {
return this.args.startTimestamp ? fromUnixTime(this.args.startTimestamp).toISOString() : null;
}

get endTimestampISO() {
return this.args.endTimestamp ? fromUnixTime(this.args.endTimestamp).toISOString() : null;
}

get formattedStartDate() {
return this.startTimestampISO ? parseAPITimestamp(this.startTimestampISO, 'MMMM yyyy') : null;
return this.args.startTimestamp ? parseAPITimestamp(this.args.startTimestamp, 'MMMM yyyy') : null;
}

// returns text for empty state message if noActivityData
get dateRangeMessage() {
if (this.startTimestampISO && this.endTimestampISO) {
if (this.args.startTimestamp && this.args.endTimestamp) {
const endMonth = isSameMonth(
parseAPITimestamp(this.startTimestampISO) as Date,
parseAPITimestamp(this.endTimestampISO) as Date
parseAPITimestamp(this.args.startTimestamp) as Date,
parseAPITimestamp(this.args.endTimestamp) as Date
)
? ''
: `to ${parseAPITimestamp(this.endTimestampISO, 'MMMM yyyy')}`;
: `to ${parseAPITimestamp(this.args.endTimestamp, 'MMMM yyyy')}`;
// completes the message 'No data received from { dateRangeMessage }'
return `from ${parseAPITimestamp(this.startTimestampISO, 'MMMM yyyy')} ${endMonth}`;
return `from ${parseAPITimestamp(this.args.startTimestamp, 'MMMM yyyy')} ${endMonth}`;
}
return null;
}
Expand Down Expand Up @@ -127,9 +119,9 @@ export default class ClientsCountsPageComponent extends Component<Args> {
// show banner if startTime returned from activity log (response) is after the queried startTime
const { activity, config } = this.args;
const activityStartDateObject = parseAPITimestamp(activity.startTime) as Date;
const queryStartDateObject = parseAPITimestamp(this.startTimestampISO) as Date;
const queryStartDateObject = parseAPITimestamp(this.args.startTimestamp) as Date;
const isEnterprise =
this.startTimestampISO === config.billingStartTimestamp?.toISOString() && this.version.isEnterprise;
this.args.startTimestamp === config.billingStartTimestamp?.toISOString() && this.version.isEnterprise;
const message = isEnterprise ? 'Your license start date is' : 'You requested data from';

if (
Expand Down
4 changes: 2 additions & 2 deletions ui/app/components/clients/page/overview.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
@totalClientAttribution={{this.totalClientAttribution}}
@newClientAttribution={{this.newClientAttribution}}
@selectedNamespace={{@namespace}}
@startTimestamp={{this.startTimeISO}}
@endTimestamp={{this.endTimeISO}}
@startTimestamp={{@startTimestamp}}
@endTimestamp={{@endTimestamp}}
@responseTimestamp={{@activity.responseTimestamp}}
@isHistoricalMonth={{and (not this.isCurrentMonth) (not this.isDateRange)}}
@upgradesDuringActivity={{this.upgradesDuringActivity}}
Expand Down
70 changes: 49 additions & 21 deletions ui/app/routes/vault/cluster/clients/counts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@

import Route from '@ember/routing/route';
import { service } from '@ember/service';
import timestamp from 'core/utils/timestamp';
import { getUnixTime } from 'date-fns';
import { fromUnixTime } from 'date-fns';

import type FlagsService from 'vault/services/flags';
import type StoreService from 'vault/services/store';
import type VersionService from 'vault/services/version';
import type { ModelFrom } from 'vault/vault/route';
import type ClientsRoute from '../clients';
import type ClientsCountsController from 'vault/controllers/vault/cluster/clients/counts';
import { setStartTimeQuery } from 'core/utils/client-count-utils';

export interface ClientsCountsRouteParams {
start_time?: string | number | undefined;
Expand All @@ -39,38 +37,68 @@ export default class ClientsCountsRoute extends Route {
return this.flags.fetchActivatedFlags();
}

async getActivity(start_time: number | null, end_time: number) {
/**
* This method returns the query param timestamp if it exists. If not, it returns the activity timestamp value instead.
*/
paramOrResponseTimestamp(
qpMillisString: string | number | undefined,
activityTimeStamp: string | undefined
) {
let timestamp: string | undefined;
const millis = Number(qpMillisString);
if (!isNaN(millis)) {
timestamp = fromUnixTime(millis).toISOString();
}
// fallback to activity timestamp only if there was no query param
Copy link
Contributor

Choose a reason for hiding this comment

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

thank you for these comments!

if (!timestamp && activityTimeStamp) {
timestamp = activityTimeStamp;
}
return timestamp;
}

async getActivity(params: ClientsCountsRouteParams) {
let activity, activityError;
// if there is no start_time we want the user to manually choose a date
// in that case bypass the query so that the user isn't stuck viewing the activity error
if (start_time) {
// if CE without start time we want to skip the activity call
// so that the user is forced to choose a date range
if (this.version.isEnterprise || params.start_time) {
const query = {
// start and end params are optional -- if not provided, will fallback to API default
start_time: this.formatTimeQuery(params?.start_time),
end_time: this.formatTimeQuery(params?.end_time),
};
try {
activity = await this.store.queryRecord('clients/activity', {
start_time: { timestamp: start_time },
end_time: { timestamp: end_time },
});
activity = await this.store.queryRecord('clients/activity', query);
} catch (error) {
activityError = error;
}
}
return { activity, activityError };
return {
activity,
activityError,
};
}

// Takes the string URL param and formats it as the adapter expects it,
// if it exists and is valid
formatTimeQuery(param: string | number | undefined) {
let timeParam: { timestamp: number } | undefined;
const millis = Number(param);
if (!isNaN(millis)) {
timeParam = { timestamp: millis };
}
return timeParam;
}

async model(params: ClientsCountsRouteParams) {
const { config, versionHistory } = this.modelFor('vault.cluster.clients') as ModelFrom<ClientsRoute>;
// only enterprise versions will have a relevant billing start date, if null users must select initial start time
const startTime = setStartTimeQuery(this.version.isEnterprise, config);

const startTimestamp = Number(params.start_time) || startTime;
const endTimestamp = Number(params.end_time) || getUnixTime(timestamp.now());
const { activity, activityError } = await this.getActivity(startTimestamp, endTimestamp);

const { activity, activityError } = await this.getActivity(params);
return {
activity,
activityError,
config,
endTimestamp,
startTimestamp,
// activity.startTime corresponds to first month with data, but we want first month returned or requested
startTimestamp: this.paramOrResponseTimestamp(params?.start_time, activity?.byMonth[0]?.timestamp),
endTimestamp: this.paramOrResponseTimestamp(params?.end_time, activity?.endTime),
Comment on lines +100 to +101
Copy link
Contributor

Choose a reason for hiding this comment

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

So the new functionality is these params correspond to the range of data requested and the activity above (line 93) has start and end time parameters that correspond to the start_time and end_time from the /activity response?

I'm just thinking through the logic we have that lets users know "You have requested [start date] we only have data from [start of data]"

paramOrResponseTimestamp is helpfully named, I wonder if a comment above this function would help clarify the difference between selecting the first month timestamp vs the start/end params on the actual activity response 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I can definitely add a comment with a description about what this does!

If query params were used, we want to set the start and end timestamps to match those, as you said, because the API start time matches the first month that data was returned. I found that if I just relied on the API timestamp responses, the message about only having data from [start of data] never showed up, because the timestamps matched. So, instead I had to default to the first month's timestamp if no query param was used, to ensure that the discrepancy message displayed when the data wasn't available for all months.

versionHistory,
};
}
Expand Down
11 changes: 0 additions & 11 deletions ui/lib/core/addon/utils/client-count-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,6 @@ export const filterVersionHistory = (
return [];
};

export const setStartTimeQuery = (
isEnterprise: boolean,
config: ClientsConfigModel | Record<string, never>
) => {
// CE versions have no license and so the start time defaults to "0001-01-01T00:00:00Z"
if (isEnterprise && _hasConfig(config)) {
return getUnixTime(config.billingStartTimestamp);
}
return null;
};

// METHODS FOR SERIALIZING ACTIVITY RESPONSE
export const formatDateObject = (dateObj: { monthIdx: number; year: number }, isEnd: boolean) => {
const { year, monthIdx } = dateObj;
Expand Down
Loading
Loading