Skip to content

Commit

Permalink
fix: stickiness (#3471)
Browse files Browse the repository at this point in the history
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
- Fixes project model/store
- Fixes UI sync issues when switching stickiness 


## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->

<!-- Does it close an issue? Multiple? -->
Closes #

<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->

### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->


## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->

---------

Signed-off-by: andreas-unleash <andreas@getunleash.ai>
  • Loading branch information
andreas-unleash committed Apr 6, 2023
1 parent 4567ae6 commit 92328f6
Show file tree
Hide file tree
Showing 16 changed files with 79 additions and 61 deletions.
1 change: 0 additions & 1 deletion frontend/cypress/integration/projects/settings.spec.ts
Expand Up @@ -66,7 +66,6 @@ describe('project settings', () => {
);

cy.get("[data-testid='ADD_VARIANT_BUTTON']").first().click();
cy.wait(300);
//then
cy.get("[id='stickiness-select']")
.first()
Expand Down
Expand Up @@ -74,7 +74,7 @@ export const FeatureStrategyCreate = () => {
forceRefreshCache(feature);
ref.current = feature;
}
}, [feature]);
}, [feature.name]);

useEffect(() => {
if (strategyDefinition) {
Expand Down
Expand Up @@ -273,7 +273,13 @@ export const EnvironmentVariantsModal = ({
isChangeRequestConfigured(environment?.name || '') &&
uiConfig.flags.crOnVariants;

const stickiness = variants[0]?.stickiness || defaultStickiness;
const stickiness = useMemo(() => {
if (!loading) {
return variants[0]?.stickiness || defaultStickiness;
}
return '';
}, [loading, defaultStickiness, JSON.stringify(variants[0] ?? {})]);

const stickinessOptions = useMemo(
() => [
'default',
Expand All @@ -296,7 +302,7 @@ export const EnvironmentVariantsModal = ({
};

const onStickinessChange = (value: string) => {
updateStickiness(value).catch(console.warn);
updateStickiness(value);
};

const [error, setError] = useState<string | undefined>();
Expand All @@ -308,14 +314,13 @@ export const EnvironmentVariantsModal = ({
}, [apiPayload.error]);

const handleClose = () => {
updateStickiness(defaultStickiness).catch(console.warn);
updateStickiness(defaultStickiness);
setOpen(false);
};

if (loading || stickiness === '') {
return <Loader />;
}

return (
<SidebarModal open={open} onClose={handleClose} label="">
<FormTemplate
Expand Down
Expand Up @@ -12,10 +12,10 @@ import {
parseParameterString,
} from 'utils/parseParameter';
import { StickinessSelect } from './StickinessSelect/StickinessSelect';
import { useOptionalPathParam } from 'hooks/useOptionalPathParam';
import { useDefaultProjectSettings } from 'hooks/useDefaultProjectSettings';
import Loader from '../../../common/Loader/Loader';
import { useMemo } from 'react';
import { useRequiredPathParam } from 'hooks/useRequiredPathParam';

interface IFlexibleStrategyProps {
parameters: IFeatureStrategyParameters;
Expand All @@ -29,7 +29,7 @@ const FlexibleStrategy = ({
parameters,
editable = true,
}: IFlexibleStrategyProps) => {
const projectId = useOptionalPathParam('projectId');
const projectId = useRequiredPathParam('projectId');
const { defaultStickiness, loading } = useDefaultProjectSettings(projectId);

const onUpdate = (field: string) => (newValue: string) => {
Expand Down
Expand Up @@ -41,7 +41,6 @@ export const StickinessSelect = ({
);

const stickinessOptions = resolveStickinessOptions();

return (
<Select
id="stickiness-select"
Expand Down
1 change: 1 addition & 0 deletions frontend/src/hooks/api/getters/useProject/useProject.ts
Expand Up @@ -13,6 +13,7 @@ const fallbackProject: IProject = {
description: 'Default',
favorite: false,
mode: 'open',
defaultStickiness: 'default',
stats: {
archivedCurrentWindow: 0,
archivedPastWindow: 0,
Expand Down
54 changes: 8 additions & 46 deletions frontend/src/hooks/useDefaultProjectSettings.ts
@@ -1,57 +1,19 @@
import useUiConfig from './api/getters/useUiConfig/useUiConfig';
import { SWRConfiguration } from 'swr';
import { useCallback } from 'react';
import handleErrorResponses from './api/getters/httpErrorResponseHandler';
import { useConditionalSWR } from './api/getters/useConditionalSWR/useConditionalSWR';
import { ProjectMode } from 'component/project/Project/hooks/useProjectForm';
import { formatApiPath } from 'utils/formatPath';
import useProject from './api/getters/useProject/useProject';

export interface ISettingsResponse {
defaultStickiness?: string;
mode?: ProjectMode;
}
const DEFAULT_STICKINESS = 'default';
export const useDefaultProjectSettings = (
projectId?: string,
options?: SWRConfiguration
) => {
export const useDefaultProjectSettings = (projectId: string) => {
const { uiConfig } = useUiConfig();

const PATH = `api/admin/projects/${projectId}/settings`;
const { projectScopedStickiness } = uiConfig.flags;

const { data, isLoading, error, mutate } =
useConditionalSWR<ISettingsResponse>(
Boolean(projectId) && Boolean(projectScopedStickiness),
{},
['useDefaultProjectSettings', PATH],
() => fetcher(formatApiPath(PATH)),
options
);

const defaultStickiness = (): string => {
if (!isLoading) {
if (data?.defaultStickiness) {
return data?.defaultStickiness;
}
return DEFAULT_STICKINESS;
}
return '';
};

const refetch = useCallback(() => {
mutate().catch(console.warn);
}, [mutate]);
const { project, loading, error } = useProject(projectId);
return {
defaultStickiness: defaultStickiness(),
refetch,
loading: isLoading,
defaultStickiness: Boolean(projectScopedStickiness)
? project.defaultStickiness
: DEFAULT_STICKINESS,
mode: project.mode,
loading: loading,
error,
};
};

const fetcher = (path: string) => {
return fetch(path)
.then(handleErrorResponses('Project stickiness data'))
.then(res => res.json());
};
1 change: 1 addition & 0 deletions frontend/src/interfaces/project.ts
Expand Up @@ -24,6 +24,7 @@ export interface IProject {
favorite: boolean;
features: IFeatureToggleListItem[];
mode: 'open' | 'protected';
defaultStickiness: string;
}

export interface IProjectHealthReport extends IProject {
Expand Down
3 changes: 2 additions & 1 deletion src/lib/db/project-store.ts
Expand Up @@ -33,7 +33,7 @@ const COLUMNS = [
'updated_at',
];
const TABLE = 'projects';
const SETTINGS_COLUMNS = ['project_mode'];
const SETTINGS_COLUMNS = ['project_mode', 'default_stickiness'];
const SETTINGS_TABLE = 'project_settings';

export interface IEnvironmentProjectLink {
Expand Down Expand Up @@ -533,6 +533,7 @@ class ProjectStore implements IProjectStore {
health: row.health ?? 100,
updatedAt: row.updated_at || new Date(),
mode: row.project_mode || 'open',
defaultStickiness: row.default_stickiness || 'default',
};
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/lib/services/project-service.ts
Expand Up @@ -836,13 +836,12 @@ export default class ProjectService {
: Promise.resolve(false),
this.projectStatsStore.getProjectStats(projectId),
]);

return {
stats: projectStats,
name: project.name,
description: project.description,
mode: project.mode,
defaultStickiness: project.defaultStickiness || 'default',
defaultStickiness: project.defaultStickiness,
health: project.health || 0,
favorite: favorite,
updatedAt: project.updatedAt,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/types/model.ts
Expand Up @@ -372,7 +372,7 @@ export interface IProject {
updatedAt?: Date;
changeRequestsEnabled?: boolean;
mode: ProjectMode;
defaultStickiness?: string;
defaultStickiness: string;
}

export interface ICustomRole {
Expand Down
7 changes: 6 additions & 1 deletion src/test/e2e/api/admin/project/features.e2e.test.ts
Expand Up @@ -525,7 +525,12 @@ describe('Interacting with features using project IDs that belong to other proje
rootRole: RoleName.ADMIN,
});
await app.services.projectService.createProject(
{ name: otherProject, id: otherProject, mode: 'open' },
{
name: otherProject,
id: otherProject,
mode: 'open',
defaultStickiness: 'clientId',
},
dummyAdmin,
);

Expand Down
2 changes: 1 addition & 1 deletion src/test/e2e/api/proxy/proxy.e2e.test.ts
Expand Up @@ -101,7 +101,7 @@ const createProject = async (id: string, name: string): Promise<void> => {
email: `${randomId()}@example.com`,
});
await app.services.projectService.createProject(
{ id, name, mode: 'open' },
{ id, name, mode: 'open', defaultStickiness: 'default' },
user,
);
};
Expand Down
1 change: 1 addition & 0 deletions src/test/e2e/services/api-token-service.e2e.test.ts
Expand Up @@ -43,6 +43,7 @@ beforeAll(async () => {
name: 'Test Project',
description: 'Fancy',
mode: 'open' as const,
defaultStickiness: 'clientId',
};
const user = await stores.userStore.insert({
name: 'Some Name',
Expand Down

0 comments on commit 92328f6

Please sign in to comment.