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

fix: last seen cherrypick #5717

Merged
merged 2 commits into from
Dec 21, 2023
Merged

fix: last seen cherrypick #5717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d27a7c1
fix: last seen cherrypick
kwasniew Dec 21, 2023
3c9499d
fix: cherry pick undefined date comparison
kwasniew Dec 21, 2023
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
27 changes: 16 additions & 11 deletions src/lib/features/feature-toggle/feature-toggle-strategies-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { ensureStringValue, mapValues } from '../../util';
import { IFeatureProjectUserParams } from './feature-toggle-controller';
import { Db } from '../../db/db';
import Raw = Knex.Raw;
import { isAfter } from 'date-fns';
import {
IFeatureSearchParams,
IQueryParam,
Expand Down Expand Up @@ -387,7 +388,6 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
acc.description = r.description;
acc.project = r.project;
acc.stale = r.stale;
acc.lastSeenAt = r.last_seen_at;

acc.createdAt = r.created_at;
acc.type = r.type;
Expand All @@ -399,8 +399,11 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
}

if (
acc.lastSeenAt === undefined ||
new Date(r.env_last_seen_at) > new Date(acc.lastSeenAt)
acc.lastSeenAt == null ||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is required as new Date(undefined) doesn't work correctly as new Date(null)

Copy link
Member

Choose a reason for hiding this comment

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

But would acc.lastSeenAt now be undefined and not null because of the removal on this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, but the line I removed was always breaking the current acc state

isAfter(
new Date(r.env_last_seen_at),
new Date(acc.lastSeenAt),
)
) {
acc.lastSeenAt = r.env_last_seen_at;
}
Expand Down Expand Up @@ -861,7 +864,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {

getAggregatedSearchData(rows): IFeatureOverview {
return rows.reduce((acc, row) => {
if (acc[row.feature_name] !== undefined) {
if (acc[row.feature_name]) {
const environmentExists = acc[
row.feature_name
].environments.some(
Expand Down Expand Up @@ -906,9 +909,10 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
}
const featureRow = acc[row.feature_name];
if (
featureRow.lastSeenAt === undefined ||
new Date(row.env_last_seen_at) >
new Date(featureRow.last_seen_at)
isAfter(
new Date(row.env_last_seen_at),
new Date(featureRow.lastSeenAt),
)
) {
featureRow.lastSeenAt = row.env_last_seen_at;
}
Expand All @@ -918,7 +922,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {

getFeatureOverviewData(rows): IFeatureOverview {
return rows.reduce((acc, row) => {
if (acc[row.feature_name] !== undefined) {
if (acc[row.feature_name]) {
const environmentExists = acc[
row.feature_name
].environments.some(
Expand Down Expand Up @@ -953,9 +957,10 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
}
const featureRow = acc[row.feature_name];
if (
featureRow.lastSeenAt === undefined ||
new Date(row.env_last_seen_at) >
new Date(featureRow.last_seen_at)
isAfter(
new Date(row.env_last_seen_at),
new Date(featureRow.lastSeenAt),
)
) {
featureRow.lastSeenAt = row.env_last_seen_at;
}
Expand Down
4 changes: 3 additions & 1 deletion src/lib/features/feature-toggle/tests/feature-toggle-service.e2e.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -680,13 +680,14 @@ test('Should return last seen at per environment', async () => {
db.rawDatabase,
);

const { environments } = await service.getFeature({
const { environments, lastSeenAt } = await service.getFeature({
featureName,
projectId: 'default',
environmentVariants: false,
});

expect(environments[0].lastSeenAt).toEqual(new Date(date));
expect(lastSeenAt).toEqual(new Date(date));

// Test with feature flag on
const config = createTestConfig();
Expand All @@ -707,4 +708,5 @@ test('Should return last seen at per environment', async () => {
expect(featureToggle.environments[0].lastSeenAt).toEqual(
new Date(lastSeenAtStoreDate),
);
expect(featureToggle.lastSeenAt).toEqual(new Date(lastSeenAtStoreDate));
});
13 changes: 11 additions & 2 deletions src/test/e2e/api/admin/project/projects.e2e.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ test('response should include last seen at per environment', async () => {
.expect(200);

expect(body.features[0].environments[0].lastSeenAt).toEqual(testDate);
expect(body.features[0].lastSeenAt).toEqual(testDate);

const appWithLastSeenRefactor = await setupAppWithCustomConfig(
db.stores,
Expand All @@ -180,6 +181,9 @@ test('response should include last seen at per environment', async () => {
expect(response.body.features[0].environments[0].lastSeenAt).toEqual(
'2023-10-01T12:34:56.000Z',
);
expect(response.body.features[0].lastSeenAt).toEqual(
'2023-10-01T12:34:56.000Z',
);
});

test('response should include last seen at per environment for multiple environments', async () => {
Expand Down Expand Up @@ -221,16 +225,19 @@ test('response should include last seen at per environment for multiple environm
'multiple-environment-last-seen-at',
db.rawDatabase,
'default',
'2023-10-01 12:32:56',
);
await insertLastSeenAt(
'multiple-environment-last-seen-at',
db.rawDatabase,
'development',
'2023-10-01 12:34:56',
);
await insertLastSeenAt(
'multiple-environment-last-seen-at',
db.rawDatabase,
'production',
'2023-10-01 12:33:56',
);

const { body } = await appWithLastSeenRefactor.request
Expand All @@ -243,11 +250,13 @@ test('response should include last seen at per environment for multiple environm
const [def, development, production] = featureEnvironments;

expect(def.name).toBe('default');
expect(def.lastSeenAt).toEqual('2023-10-01T12:34:56.000Z');
expect(def.lastSeenAt).toEqual('2023-10-01T12:32:56.000Z');

expect(development.name).toBe('development');
expect(development.lastSeenAt).toEqual('2023-10-01T12:34:56.000Z');

expect(production.name).toBe('production');
expect(production.lastSeenAt).toEqual('2023-10-01T12:34:56.000Z');
expect(production.lastSeenAt).toEqual('2023-10-01T12:33:56.000Z');

expect(body.features[1].lastSeenAt).toBe('2023-10-01T12:34:56.000Z');
});