Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit193c7ce

Browse files
authored
fix(site): tighten interface design for various frontend utility functions (#18894)
Precursor to#18895Splitting this off so that the changes are easier to review.## Changes made- Improve type-safety for the `withQuery` Storybook decorator- Centralized almost all queries that deal with template versions to usea shared, exported query key prefix- Updated `useFilter` and `usePagination` to have much more strictreturn types, and decoupled them from React Router at the interfacelevel- Also added some extra input validation and performance optimizationsto `useFilter` and `usePagination`- Removed a stale data when working with checked workspaces for the`/workspaces` page- Removed hacky `useEffect` call for syncing URL state to checkedworkspaces, in favor of explicit state syncs- Did some extra cleanup and removed unused values## Notes- Many of the changes here were in service of the main PR. I'll try tohighlight notable areas, but if there's anything that's not clear, feelfree to post a comment in this PR. Ideally you shouldn't really have tolook at the other PR to understand this one, so if something'sconfusing, that's a sign that something's wrong<!-- This is an auto-generated comment: release notes by coderabbit.ai-->## Summary by CodeRabbit* **Refactor*** Improved handling of URL search parameters and state synchronizationin filter and pagination features across multiple pages.* Centralized and clarified state management for workspace selection andbatch actions on the Workspaces page.* Enhanced type safety and naming consistency in batch actions andfilter components.* Updated filter and pagination hooks to accept explicit parameters andcallbacks for better maintainability.* Streamlined prop naming and menu handling in workspace filtercomponents for clarity.* **Bug Fixes*** Prevented unnecessary state updates when filter values remainunchanged.* **Tests** * Updated tests for improved type safety and more precise assertions.<!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent2180d17 commit193c7ce

File tree

16 files changed

+309
-198
lines changed

16 files changed

+309
-198
lines changed

‎site/.storybook/preview.jsx‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,13 @@ function withHelmet(Story) {
101101
);
102102
}
103103

104+
/**
105+
* This JSX file isn't part of the main project, so it doesn't get to use the
106+
* ambient types defined in `storybook.d.ts` to provide extra type-safety.
107+
* Extracting main key to avoid typos.
108+
*/
109+
constqueryParametersKey="queries";
110+
104111
/**@type {Decorator} */
105112
functionwithQuery(Story,{ parameters}){
106113
constqueryClient=newQueryClient({
@@ -112,8 +119,8 @@ function withQuery(Story, { parameters }) {
112119
},
113120
});
114121

115-
if(parameters.queries){
116-
for(constqueryofparameters.queries){
122+
if(parameters[queryParametersKey]){
123+
for(constqueryofparameters[queryParametersKey]){
117124
queryClient.setQueryData(query.key,query.data);
118125
}
119126
}

‎site/src/api/queries/templates.ts‎

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -48,21 +48,6 @@ export const templates = (
4848
};
4949
};
5050

51-
constgetTemplatesByOrganizationQueryKey=(
52-
organization:string,
53-
options?:GetTemplatesOptions,
54-
)=>[organization,"templates",options?.deprecated];
55-
56-
consttemplatesByOrganization=(
57-
organization:string,
58-
options:GetTemplatesOptions={},
59-
)=>{
60-
return{
61-
queryKey:getTemplatesByOrganizationQueryKey(organization,options),
62-
queryFn:()=>API.getTemplatesByOrganization(organization,options),
63-
};
64-
};
65-
6651
exportconsttemplateACL=(templateId:string)=>{
6752
return{
6853
queryKey:["templateAcl",templateId],
@@ -121,9 +106,11 @@ export const templateExamples = () => {
121106
};
122107
};
123108

109+
exportconsttemplateVersionRoot:string="templateVersion";
110+
124111
exportconsttemplateVersion=(versionId:string)=>{
125112
return{
126-
queryKey:["templateVersion",versionId],
113+
queryKey:[templateVersionRoot,versionId],
127114
queryFn:()=>API.getTemplateVersion(versionId),
128115
};
129116
};
@@ -134,7 +121,7 @@ export const templateVersionByName = (
134121
versionName:string,
135122
)=>{
136123
return{
137-
queryKey:["templateVersion",organizationId,templateName,versionName],
124+
queryKey:[templateVersionRoot,organizationId,templateName,versionName],
138125
queryFn:()=>
139126
API.getTemplateVersionByName(organizationId,templateName,versionName),
140127
};
@@ -153,7 +140,7 @@ export const templateVersions = (templateId: string) => {
153140
};
154141

155142
exportconsttemplateVersionVariablesKey=(versionId:string)=>[
156-
"templateVersion",
143+
templateVersionRoot,
157144
versionId,
158145
"variables",
159146
];
@@ -216,7 +203,7 @@ export const templaceACLAvailable = (
216203
};
217204

218205
consttemplateVersionExternalAuthKey=(versionId:string)=>[
219-
"templateVersion",
206+
templateVersionRoot,
220207
versionId,
221208
"externalAuth",
222209
];
@@ -257,21 +244,21 @@ const createTemplateFn = async (options: CreateTemplateOptions) => {
257244

258245
exportconsttemplateVersionLogs=(versionId:string)=>{
259246
return{
260-
queryKey:["templateVersion",versionId,"logs"],
247+
queryKey:[templateVersionRoot,versionId,"logs"],
261248
queryFn:()=>API.getTemplateVersionLogs(versionId),
262249
};
263250
};
264251

265252
exportconstrichParameters=(versionId:string)=>{
266253
return{
267-
queryKey:["templateVersion",versionId,"richParameters"],
254+
queryKey:[templateVersionRoot,versionId,"richParameters"],
268255
queryFn:()=>API.getTemplateVersionRichParameters(versionId),
269256
};
270257
};
271258

272259
exportconstresources=(versionId:string)=>{
273260
return{
274-
queryKey:["templateVersion",versionId,"resources"],
261+
queryKey:[templateVersionRoot,versionId,"resources"],
275262
queryFn:()=>API.getTemplateVersionResources(versionId),
276263
};
277264
};
@@ -293,7 +280,7 @@ export const previousTemplateVersion = (
293280
)=>{
294281
return{
295282
queryKey:[
296-
"templateVersion",
283+
templateVersionRoot,
297284
organizationId,
298285
templateName,
299286
versionName,
@@ -313,7 +300,7 @@ export const previousTemplateVersion = (
313300

314301
exportconsttemplateVersionPresets=(versionId:string)=>{
315302
return{
316-
queryKey:["templateVersion",versionId,"presets"],
303+
queryKey:[templateVersionRoot,versionId,"presets"],
317304
queryFn:()=>API.getTemplateVersionPresets(versionId),
318305
};
319306
};

‎site/src/components/Filter/Filter.tsx‎

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import { useDebouncedFunction } from "hooks/debounce";
1616
import{ExternalLinkIcon}from"lucide-react";
1717
import{ChevronDownIcon}from"lucide-react";
1818
import{typeFC,typeReactNode,useEffect,useRef,useState}from"react";
19-
importtype{useSearchParams}from"react-router";
2019

2120
typePresetFilter={
2221
name:string;
@@ -27,35 +26,55 @@ type FilterValues = Record<string, string | undefined>;
2726

2827
typeUseFilterConfig={
2928
/**
30-
* The fallback value to use in the event that no filter params can be parsed
31-
* from the search params object. This value is allowed to change on
32-
* re-renders.
29+
* The fallback value to use in the event that no filter params can be
30+
* parsed from the search params object.
3331
*/
3432
fallbackFilter?:string;
35-
searchParamsResult:ReturnType<typeofuseSearchParams>;
33+
searchParams:URLSearchParams;
34+
onSearchParamsChange:(newParams:URLSearchParams)=>void;
3635
onUpdate?:(newValue:string)=>void;
3736
};
3837

38+
exporttypeUseFilterResult=Readonly<{
39+
query:string;
40+
values:FilterValues;
41+
used:boolean;
42+
update:(newValues:string|FilterValues)=>void;
43+
debounceUpdate:(newValues:string|FilterValues)=>void;
44+
cancelDebounce:()=>void;
45+
}>;
46+
3947
exportconstuseFilterParamsKey="filter";
4048

4149
exportconstuseFilter=({
4250
fallbackFilter="",
43-
searchParamsResult,
51+
searchParams,
52+
onSearchParamsChange,
4453
onUpdate,
45-
}:UseFilterConfig)=>{
46-
const[searchParams,setSearchParams]=searchParamsResult;
54+
}:UseFilterConfig):UseFilterResult=>{
4755
constquery=searchParams.get(useFilterParamsKey)??fallbackFilter;
4856

4957
constupdate=(newValues:string|FilterValues)=>{
5058
constserialized=
5159
typeofnewValues==="string" ?newValues :stringifyFilter(newValues);
60+
constnoUpdateNeeded=query===serialized;
61+
if(noUpdateNeeded){
62+
return;
63+
}
5264

65+
/**
66+
*@todo 2025-07-15 - We have a slightly nasty bug here, where trying to
67+
* update state via immutable state updates causes our code to break.
68+
*
69+
* In theory, it would be better to make a copy of the search params. We
70+
* can then mutate and dispatch the copy instead of the original. Doing
71+
* that causes other parts of our existing logic to break, though.
72+
* That's a sign that our other code is slightly broken, and only just
73+
* happens to work by chance right now.
74+
*/
5375
searchParams.set(useFilterParamsKey,serialized);
54-
setSearchParams(searchParams);
55-
56-
if(onUpdate!==undefined){
57-
onUpdate(serialized);
58-
}
76+
onSearchParamsChange(searchParams);
77+
onUpdate?.(serialized);
5978
};
6079

6180
const{debounced:debounceUpdate, cancelDebounce}=useDebouncedFunction(
@@ -73,8 +92,6 @@ export const useFilter = ({
7392
};
7493
};
7594

76-
exporttypeUseFilterResult=ReturnType<typeofuseFilter>;
77-
7895
constparseFilterQuery=(filterQuery:string):FilterValues=>{
7996
if(filterQuery===""){
8097
return{};

‎site/src/hooks/usePagination.ts‎

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,43 @@
11
import{DEFAULT_RECORDS_PER_PAGE}from"components/PaginationWidget/utils";
2-
importtype{useSearchParams}from"react-router";
32

4-
exportconstusePagination=({
5-
searchParamsResult,
6-
}:{
7-
searchParamsResult:ReturnType<typeofuseSearchParams>;
8-
})=>{
9-
const[searchParams,setSearchParams]=searchParamsResult;
10-
constpage=searchParams.get("page") ?Number(searchParams.get("page")) :1;
11-
constlimit=DEFAULT_RECORDS_PER_PAGE;
12-
constoffset=page<=0 ?0 :(page-1)*limit;
3+
constpaginationKey="page";
134

14-
constgoToPage=(page:number)=>{
15-
searchParams.set("page",page.toString());
16-
setSearchParams(searchParams);
17-
};
5+
typeUsePaginationOptions=Readonly<{
6+
searchParams:URLSearchParams;
7+
onSearchParamsChange:(newParams:URLSearchParams)=>void;
8+
}>;
9+
10+
typeUsePaginationResult=Readonly<{
11+
page:number;
12+
limit:number;
13+
offset:number;
14+
goToPage:(page:number)=>void;
15+
}>;
16+
17+
exportfunctionusePagination(
18+
options:UsePaginationOptions,
19+
):UsePaginationResult{
20+
const{ searchParams, onSearchParamsChange}=options;
21+
constlimit=DEFAULT_RECORDS_PER_PAGE;
22+
constrawPage=Number.parseInt(searchParams.get(paginationKey)||"1",10);
23+
constpage=Number.isNaN(rawPage)||rawPage<=0 ?1 :rawPage;
1824

1925
return{
2026
page,
2127
limit,
22-
goToPage,
23-
offset,
28+
offset:Math.max(0,(page-1)*limit),
29+
goToPage:(newPage)=>{
30+
constabortNavigation=
31+
page===newPage||
32+
!Number.isFinite(newPage)||
33+
!Number.isInteger(newPage);
34+
if(abortNavigation){
35+
return;
36+
}
37+
38+
constcopy=newURLSearchParams(searchParams);
39+
copy.set("page",newPage.toString());
40+
onSearchParamsChange(copy);
41+
},
2442
};
25-
};
43+
}

‎site/src/pages/AuditPage/AuditPage.test.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import{screen,waitFor}from"@testing-library/react";
22
importuserEventfrom"@testing-library/user-event";
33
import{API}from"api/api";
4+
importtype{AuditLogsRequest}from"api/typesGenerated";
45
import{DEFAULT_RECORDS_PER_PAGE}from"components/PaginationWidget/utils";
56
import{http,HttpResponse}from"msw";
67
import{
@@ -106,7 +107,7 @@ describe("AuditPage", () => {
106107
awaituserEvent.type(filterField,query);
107108

108109
awaitwaitFor(()=>
109-
expect(getAuditLogsSpy).toBeCalledWith({
110+
expect(getAuditLogsSpy).toHaveBeenCalledWith<[AuditLogsRequest]>({
110111
limit:DEFAULT_RECORDS_PER_PAGE,
111112
offset:0,
112113
q:query,

‎site/src/pages/AuditPage/AuditPage.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ const AuditPage: FC = () => {
3333
const[searchParams,setSearchParams]=useSearchParams();
3434
constauditsQuery=usePaginatedQuery(paginatedAudits(searchParams));
3535
constfilter=useFilter({
36-
searchParamsResult:[searchParams,setSearchParams],
36+
searchParams,
37+
onSearchParamsChange:setSearchParams,
3738
onUpdate:auditsQuery.goToFirstPage,
3839
});
3940

‎site/src/pages/ConnectionLogPage/ConnectionLogPage.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ const ConnectionLogPage: FC = () => {
2929
paginatedConnectionLogs(searchParams),
3030
);
3131
constfilter=useFilter({
32-
searchParamsResult:[searchParams,setSearchParams],
32+
searchParams,
33+
onSearchParamsChange:setSearchParams,
3334
onUpdate:connectionlogsQuery.goToFirstPage,
3435
});
3536

‎site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,7 @@ export const CreateWorkspacePageViewExperimental: FC<
104104
owner,
105105
setOwner,
106106
})=>{
107-
const[suggestedName,setSuggestedName]=useState(()=>
108-
generateWorkspaceName(),
109-
);
107+
const[suggestedName,setSuggestedName]=useState(generateWorkspaceName);
110108
const[showPresetParameters,setShowPresetParameters]=useState(false);
111109
constid=useId();
112110
constworkspaceNameInputRef=useRef<HTMLInputElement>(null);
@@ -120,14 +118,8 @@ export const CreateWorkspacePageViewExperimental: FC<
120118

121119
// Only touched fields are sent to the websocket
122120
// Autofilled parameters are marked as touched since they have been modified
123-
constinitialTouched=parameters.reduce(
124-
(touched,parameter)=>{
125-
if(autofillByName[parameter.name]!==undefined){
126-
touched[parameter.name]=true;
127-
}
128-
returntouched;
129-
},
130-
{}asRecord<string,boolean>,
121+
constinitialTouched=Object.fromEntries(
122+
parameters.filter((p)=>autofillByName[p.name]).map((p)=>[p,true]),
131123
);
132124

133125
// The form parameters values hold the working state of the parameters that will be submitted when creating a workspace

‎site/src/pages/TemplatesPage/TemplatesPage.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,11 @@ const TemplatesPage: FC = () => {
1414
const{ permissions,user:me}=useAuthenticated();
1515
const{ showOrganizations}=useDashboard();
1616

17-
constsearchParamsResult=useSearchParams();
17+
const[searchParams,setSearchParams]=useSearchParams();
1818
constfilter=useFilter({
1919
fallbackFilter:"deprecated:false",
20-
searchParamsResult,
20+
searchParams,
21+
onSearchParamsChange:setSearchParams,
2122
onUpdate:()=>{},// reset pagination
2223
});
2324

‎site/src/pages/UsersPage/UsersPage.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,8 @@ type UserPageProps = {
3939
constUsersPage:FC<UserPageProps>=({ defaultNewPassword})=>{
4040
constqueryClient=useQueryClient();
4141
constnavigate=useNavigate();
42-
constsearchParamsResult=useSearchParams();
42+
const[searchParams,setSearchParams]=useSearchParams();
4343
const{ entitlements}=useDashboard();
44-
const[searchParams]=searchParamsResult;
4544

4645
constgroupsByUserIdQuery=useQuery(groupsByUserId());
4746
constauthMethodsQuery=useQuery(authMethods());
@@ -58,9 +57,10 @@ const UsersPage: FC<UserPageProps> = ({ defaultNewPassword }) => {
5857
enabled:viewDeploymentConfig,
5958
});
6059

61-
constusersQuery=usePaginatedQuery(paginatedUsers(searchParamsResult[0]));
60+
constusersQuery=usePaginatedQuery(paginatedUsers(searchParams));
6261
constuseFilterResult=useFilter({
63-
searchParamsResult,
62+
searchParams,
63+
onSearchParamsChange:setSearchParams,
6464
onUpdate:usersQuery.goToFirstPage,
6565
});
6666

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp