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

Fix all lint warnings in UI #1388

Merged
merged 3 commits into from
May 31, 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
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export const AnnouncementBanner: React.FC<AnnouncementBannerProps> = ({
if (user.apiKey) {
fetchVersionNumber();
}
}, [user.apiKey]);
}, [user.apiKey, onShow]);

// Make sure user is on appropriate pages and that the banner should be shown.
if (
Expand Down
2 changes: 1 addition & 1 deletion src/ui/common/src/components/pages/account/AccountPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ client = aqueduct.Client(
}

fetchServerConfig();
}, []);
}, [dispatch, user]);

useEffect(() => {
if (!updatingNotifications) {
Expand Down
4 changes: 2 additions & 2 deletions src/ui/common/src/components/pages/resource/id/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ const ResourceDetailsPage: React.FC<ResourceDetailsPageProps> = ({
})
);
}
}, [selectedResource]);
}, [dispatch, selectedResource, resourceId, user]);

// Load the server config to check if the selected resource is currently being used as storage.
// If that is the case, we hide the option to delete the resource from the user.
Expand All @@ -156,7 +156,7 @@ const ResourceDetailsPage: React.FC<ResourceDetailsPageProps> = ({
}

fetchServerConfig();
}, [user.apiKey]);
}, [dispatch, user]);

const {
data: workflowAndDagIDs,
Expand Down
4 changes: 2 additions & 2 deletions src/ui/common/src/components/pages/resources/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const ResourcesPage: React.FC<Props> = ({ user, Layout = DefaultLayout }) => {
}

fetchServerConfig();
}, [user]);
}, [dispatch, user]);

useEffect(() => {
document.title = 'Resources | Aqueduct';
Expand All @@ -73,7 +73,7 @@ const ResourcesPage: React.FC<Props> = ({ user, Layout = DefaultLayout }) => {
}

// If the last storage migration failed, display the error message.
const { data, error, isLoading } = useStorageMigrationListQuery({
const { data, isLoading } = useStorageMigrationListQuery({
apiKey: user.apiKey,
limit: '1', // only fetch the latest result.
});
Expand Down
2 changes: 1 addition & 1 deletion src/ui/common/src/components/pages/workflow/id/hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export function useWorkflowIds(apiKey: string): useWorkflowIdsOutputs {
});
return;
}
}, [wfIdParam, dagResultIdParam, dagResult, dagIdParam, dag]);
}, [navigate, wfIdParam, dagResultIdParam, dagResult, dagIdParam, dag]);

return {
workflowId: wfIdParam,
Expand Down
2 changes: 1 addition & 1 deletion src/ui/common/src/components/pages/workflow/id/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ const WorkflowPage: React.FC<WorkflowPageProps> = ({
refetchWorkflow();
setCurrentTab('Details');
}
}, [editWorkflowSuccess]);
}, [refetchWorkflow, editWorkflowSuccess]);

// This workflow doesn't exist.
if (wfError) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ const DeleteResourceDialog: React.FC<Props> = ({
}

fetchServerConfig();
}, []);
}, [dispatch, user]);

const deleteResourceStatus = useSelector(
(state: RootState) => state.resourceReducer.deletionStatus
Expand Down
5 changes: 3 additions & 2 deletions src/ui/common/src/components/resources/dialogs/dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import React, { MouseEventHandler, useEffect, useState } from 'react';
import { FormProvider, useForm } from 'react-hook-form';
import { useDispatch, useSelector } from 'react-redux';
import * as Yup from 'yup';
import { ObjectShape } from 'yup/lib/object';

import { useResourceWorkflowsGetQuery } from '../../../handlers/AqueductApi';
import {
Expand Down Expand Up @@ -46,7 +47,7 @@ type Props = {
showMigrationDialog?: () => void;
resourceToEdit?: Resource;
dialogContent: React.FC<ResourceDialogProps<ResourceConfig>>;
validationSchema: Yup.ObjectSchema<any>;
validationSchema: Yup.ObjectSchema<ObjectShape>;
};

const ResourceDialog: React.FC<Props> = ({
Expand Down Expand Up @@ -156,7 +157,7 @@ const ResourceDialog: React.FC<Props> = ({

// Unsubscribe and handle lifecycle changes.
return () => subscription.unsubscribe();
}, [methods.watch]);
}, [methods, submitDisabled]);

useEffect(() => {
if (isSucceeded(connectStatus)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const GCSDialog: React.FC<GCSDialogProps> = ({
if (setMigrateStorage) {
setMigrateStorage(initialUseAsStorage === 'true');
}
}, [setMigrateStorage]);
}, [setMigrateStorage, initialUseAsStorage]);

return (
<Box sx={{ mt: 2 }}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export const S3Dialog: React.FC<S3DialogProps> = ({
if (setMigrateStorage) {
setMigrateStorage(initialUseAsStorage === 'true');
}
}, [setMigrateStorage]);
}, [setMigrateStorage, initialUseAsStorage]);

const configProfileInput = (
<ResourceTextInputField
Expand Down
7 changes: 5 additions & 2 deletions src/ui/common/src/components/resources/dialogs/schema.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import * as Yup from 'yup';
import { ObjectShape } from 'yup/lib/object';

export function requiredAtCreate(
schema,
schema: Yup.ObjectSchema<ObjectShape>,
editMode: boolean,
msg?: string | undefined
): any {
): Yup.ObjectSchema<ObjectShape> {
return editMode ? schema : schema.required(msg);
}
98 changes: 54 additions & 44 deletions src/ui/common/src/components/tables/PaginatedSearchTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,48 +116,6 @@ export const PaginatedSearchTable: React.FC<PaginatedSearchTableProps> = ({
const [rows, setRows] = useState(rowData);
const [orderedRows, setOrderedRows] = useState(rowData);

/**
* Function used to test whether a row should be included in search results.
* This function allows for searching over arbitrary columns since it takes in a searchQuery and a column to search through.
* To allow for more control at the caller's level, this function calls onShouldInclude if there is a function passed in.
* This allows us to do things like search through a row item that is an array (assuming the callback implements the search for the column) for example.
* @param rowItem - Row item to test whether or not to include in search results.
* @param searchQuery - Query to search check e.g. rowItem[searchColumn] === searchQuery
* @param searchColumn - Column inside row item to use for search.
* @returns - true or false whether the rowItem[searchColumn] is a match for searchQuery
*/
const shouldInclude = (rowItem, searchQuery, searchColumn): boolean => {
// Since table cells can contain complex objects, this implementation is up to the caller.
// Otherwise, we default to using 'name' (two fields currently in use by the Workflows list table and Data list tables) as the field to conduct the search on.
if (onShouldInclude) {
return onShouldInclude(rowItem, searchQuery, searchColumn);
}

// filter rows by name, which is our default filter column.
let shouldInclude = false;
switch (searchColumn) {
case 'Name': {
sortColumns.forEach((column) => {
if (column.name === searchColumn) {
let v = rowItem;
for (const path of column.sortAccessPath) {
v = v[path];
}
shouldInclude = v.toLowerCase().includes(searchQuery.toLowerCase());
}
});
break;
}
default: {
// no name column, return true for everything.
shouldInclude = true;
break;
}
}

return shouldInclude;
};

const handleChangePage = (event: unknown, newPage: number) => {
setPage(newPage);
};
Expand All @@ -174,6 +132,49 @@ export const PaginatedSearchTable: React.FC<PaginatedSearchTableProps> = ({
};

useEffect(() => {
/**
* Function used to test whether a row should be included in search results.
* This function allows for searching over arbitrary columns since it takes in a searchQuery and a column to search through.
* To allow for more control at the caller's level, this function calls onShouldInclude if there is a function passed in.
* This allows us to do things like search through a row item that is an array (assuming the callback implements the search for the column) for example.
* @param rowItem - Row item to test whether or not to include in search results.
* @param searchQuery - Query to search check e.g. rowItem[searchColumn] === searchQuery
* @param searchColumn - Column inside row item to use for search.
* @returns - true or false whether the rowItem[searchColumn] is a match for searchQuery
*/
const shouldInclude = (rowItem, searchQuery, searchColumn): boolean => {
// Since table cells can contain complex objects, this implementation is up to the caller.
// Otherwise, we default to using 'name' (two fields currently in use by the Workflows list table and Data list tables) as the field to conduct the search on.
if (onShouldInclude) {
return onShouldInclude(rowItem, searchQuery, searchColumn);
}

// filter rows by name, which is our default filter column.
let shouldInclude = false;
switch (searchColumn) {
case 'Name': {
sortColumns.forEach((column) => {
if (column.name === searchColumn) {
let v = rowItem;
for (const path of column.sortAccessPath) {
v = v[path];
}
shouldInclude = v
.toLowerCase()
.includes(searchQuery.toLowerCase());
}
});
break;
}
default: {
// no name column, return true for everything.
shouldInclude = true;
break;
}
}

return shouldInclude;
};
if (searchQuery.length > 0) {
const filteredRows = rows.filter((rowItem) => {
return shouldInclude(rowItem, searchQuery, searchColumn);
Expand All @@ -186,7 +187,16 @@ export const PaginatedSearchTable: React.FC<PaginatedSearchTableProps> = ({
setRows(rowData);
}
}
}, [searchQuery, rowData]);
}, [
searchQuery,
rowData,
orderedRows,
rows,
searchColumn,
sortConfig,
onShouldInclude,
sortColumns,
]);

useEffect(() => {
if (
Expand Down Expand Up @@ -228,7 +238,7 @@ export const PaginatedSearchTable: React.FC<PaginatedSearchTableProps> = ({
});
setRows(sortedRows);
setOrderedRows(sortedRows);
}, [sortConfig]);
}, [sortConfig, rowData]);

// Need to slice (.slice(page * rowsPerPage, page * rowsPerPage + rowsPerPage)) after map because we do the API calls in the GetColumnValue function.
// As a result, if there are less than rowsPerPage number of rows, less hooks are rendered than expected. Thus, we render all hooks.
Expand Down
7 changes: 6 additions & 1 deletion src/ui/common/src/components/workflows/WorkflowSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,12 @@ const WorkflowSettings: React.FC<Props> = ({ user, dag, workflow }) => {
}
}
}
}, [deleteWorkflowSuccess, deleteWorkflowError, navigate]);
}, [
deleteWorkflowSuccess,
deleteWorkflowError,
deleteWorkflowResponse,
navigate,
]);

const resources = useSelector(
(state: RootState) => state.resourcesReducer.resources
Expand Down
3 changes: 2 additions & 1 deletion src/ui/common/src/utils/resources.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ObjectSchema } from 'yup';
import { ObjectShape } from 'yup/lib/object';

import { apiAddress } from '../components/hooks/useAqueductConsts';
import UserProfile from './auth';
Expand Down Expand Up @@ -354,7 +355,7 @@ export type Info = {
dialog?: React.FC<ResourceDialogProps<ResourceConfig>>;
// TODO: figure out typescript type for yup schema
// This may be useful: https://stackoverflow.com/questions/66171196/how-to-use-yups-object-shape-with-typescript
validationSchema: (editMode: boolean) => ObjectSchema<any>;
validationSchema: (editMode: boolean) => ObjectSchema<ObjectShape>;
};

export type ServiceInfoMap = {
Expand Down