Enhance TODOs

Signed-off-by: lucferbux <[email protected]>

clients/ui/frontend/src/__tests__/cypress/cypress/pages/appChrome.ts

@@ -9,7 +9,7 @@ class AppChrome {
     cy.testA11y();
   }
 
-  // TODO: implement when authorization is enabled
+  // TODO: [Auth-enablement] Uncomment once auth is enabled
   // shouldBeUnauthorized() {
   //   cy.findByTestId('unauthorized-error');
   //   return this;

clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/application.ts

@@ -7,7 +7,7 @@ import type { Matcher, MatcherOptions as DTLMatcherOptions } from '@testing-libr
 declare global {
   namespace Cypress {
     interface Chainable {
-      // TODO: Uncomment when authorization is enabled
+      // TODO: [Auth-enablement] Uncomment once auth is enabled
       // /**
       //  * Visits the URL and performs a login if necessary.
       //  * Uses credentials supplied by environment variables if not provided.
@@ -121,7 +121,7 @@ declare global {
   }
 }
 
-// TODO: Uncomment when authorization is enabled
+// TODO: [Auth-enablement] Uncomment once auth is enabled
 // Cypress.Commands.add('visitWithLogin', (url, user = TEST_USER) => {
 //   if (Cypress.env('MOCK')) {
 //     cy.visit(url);

clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts

@@ -205,35 +205,22 @@ describe('Model Registry core', () => {
   });
 });
 
-// TODO: Enable when model registration is there
-// describe('Register Model button', () => {
-//   it('Navigates to register page from empty state', () => {
-//     initIntercepts({ disableModelRegistryFeature: false, registeredModels: [] });
-//     modelRegistry.visit();
-//     modelRegistry.findRegisterModelButton().click();
-//     cy.findByTestId('app-page-title').should('exist');
-//     cy.findByTestId('app-page-title').contains('Register model');
-//     cy.findByText('Model registry - modelregistry-sample').should('exist');
-//   });
-
-//   it('Navigates to register page from table toolbar', () => {
-//     initIntercepts({ disableModelRegistryFeature: false });
-//     modelRegistry.visit();
-//     modelRegistry.findRegisterModelButton().click();
-//     cy.findByTestId('app-page-title').should('exist');
-//     cy.findByTestId('app-page-title').contains('Register model');
-//     cy.findByText('Model registry - modelregistry-sample').should('exist');
-//   });
-
-//   it('should be accessible for non-admin users', () => {
-//     asProjectEditUser();
-//     initIntercepts({
-//       disableModelRegistryFeature: false,
-//       allowed: false,
-//     });
-
-//     modelRegistry.visit();
-//     modelRegistry.navigate();
-//     modelRegistry.shouldModelRegistrySelectorExist();
-//   });
-// });
+describe('Register Model button', () => {
+  it('Navigates to register page from empty state', () => {
+    initIntercepts({ registeredModels: [] });
+    modelRegistry.visit();
+    modelRegistry.findRegisterModelButton().click();
+    cy.findByTestId('app-page-title').should('exist');
+    cy.findByTestId('app-page-title').contains('Register model');
+    cy.findByText('Model registry - modelregistry-sample').should('exist');
+  });
+
+  it('Navigates to register page from table toolbar', () => {
+    initIntercepts({ registeredModels: [] });
+    modelRegistry.visit();
+    modelRegistry.findRegisterModelButton().click();
+    cy.findByTestId('app-page-title').should('exist');
+    cy.findByTestId('app-page-title').contains('Register model');
+    cy.findByText('Model registry - modelregistry-sample').should('exist');
+  });
+});

clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelVersions.cy.ts

@@ -130,7 +130,7 @@ describe('Model Versions', () => {
   });
 
   it('Model versions table', () => {
-    // TODO: Uncomment when we fix finding listbox items
+    // TODO: [Testing] Uncomment when we fix finding listbox items
 
     initIntercepts({
       modelRegistries: [

clients/ui/frontend/src/app/App.tsx

@@ -62,7 +62,7 @@ const App: React.FC = () => {
               <Button
                 variant="secondary"
                 onClick={() => {
-                  // TODO: logout
+                  // TODO: [Auth-enablement] Logout when auth is enabled
                 }}
               >
                 Logout
@@ -94,7 +94,9 @@ const App: React.FC = () => {
         </MastheadBrand>
       </MastheadMain>
 
-      <MastheadContent>{/* TODO: Change this into a component for Header Tools */}</MastheadContent>
+      <MastheadContent>
+        {/* TODO: [Auth-enablement] Add logout and user status once we enable itNavigates to register page from table toolbar */}
+      </MastheadContent>
     </Masthead>
   );
 

clients/ui/frontend/src/app/AppRoutes.tsx

@@ -25,7 +25,7 @@ export const useAdminSettings = (): NavDataItem[] => {
   // get auth access for example set admin as true
   const isAdmin = true; //this should be a call to getting auth / role access
 
-  // TODO: Remove the linter skip when we implement authentication
+  // TODO: [Auth-enablement] Remove the linter skip when we implement authentication
   // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
   if (!isAdmin) {
     return [];
@@ -56,7 +56,7 @@ const AppRoutes: React.FC = () => {
       <Route path="/modelRegistry/*" element={<ModelRegistryRoutes />} />
       <Route path="*" element={<NotFound />} />
       {
-        // TODO: Remove the linter skip when we implement authentication
+        // TODO: [Auth-enablement] Remove the linter skip when we implement authentication
         // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
         isAdmin && (
           <Route path="/modelRegistrySettings/*" element={<ModelRegistrySettingsRoutes />} />

clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx

@@ -35,7 +35,8 @@ const RegistrationCommonFormSections: React.FC<RegistrationCommonFormSectionsPro
   isFirstVersion,
   latestVersion,
 }) => {
-  // const [isAutofillModalOpen, setAutofillModalOpen] = React.useState(false); TODO: Check wether we should use data connections
+  // TODO: [Data connections] Check wether we should use data connections
+  // const [isAutofillModalOpen, setAutofillModalOpen] = React.useState(false);
 
   // const connectionDataMap: Record<string, keyof RegistrationCommonFormData> = {
   //   AWS_S3_ENDPOINT: 'modelLocationEndpoint',

clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/useRegistrationCommonState.ts

@@ -17,7 +17,7 @@ export const useRegistrationCommonState = (): RegistrationCommonState => {
   const [submitError, setSubmitError] = React.useState<Error | undefined>(undefined);
 
   const { apiState } = React.useContext(ModelRegistryContext);
-  const author = 'kubeflow-user'; // TODO: enable this once we have users ---> useAppSelector((state) => state.user || '');
+  const author = 'kubeflow-user'; // TODO: [Auth-enablement] Enable this once we have users ---> useAppSelector((state) => state.user || '');
 
   const handleSubmit = (doSubmit: () => Promise<unknown>) => {
     setIsSubmitting(true);

clients/ui/frontend/src/app/pages/settings/ModelRegistriesTable.tsx

@@ -9,7 +9,7 @@ type ModelRegistriesTableProps = {
 };
 
 const ModelRegistriesTable: React.FC<ModelRegistriesTableProps> = ({ modelRegistries }) => (
-  // TODO: Add toolbar once we manage permissions
+  // TODO: [Model Registry RBAC] Add toolbar once we manage permissions
   <Table
     data-testid="model-registries-table"
     data={modelRegistries}

clients/ui/frontend/src/app/pages/settings/ModelRegistriesTableRow.tsx

@@ -17,6 +17,6 @@ const ModelRegistriesTableRow: React.FC<ModelRegistriesTableRowProps> = ({ model
   </>
 );
 
-// TODO: Get rest of columns once we manage permissions
+// TODO: [Model Registry RBAC] Get rest of columns once we manage permissions
 
 export default ModelRegistriesTableRow;

clients/ui/frontend/src/app/pages/settings/columns.ts

@@ -8,7 +8,7 @@ export const modelRegistryColumns: SortableData<ModelRegistry>[] = [
     sortable: (a, b) => a.name.localeCompare(b.name),
     width: 30,
   },
-  // TODO: Add once we manage permissions
+  // TODO: [Model Registry RBAC] Add once we manage permissions
   // {
   //   field: 'status',
   //   label: 'Status',

clients/ui/frontend/src/shared/api/apiUtils.ts

@@ -167,7 +167,6 @@ export const isModelRegistryResponse = <T>(response: unknown): response is Model
   if (typeof response === 'object' && response !== null) {
     // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
     const modelRegistryBody = response as { data?: T };
-    // TODO: Check if data is conforming any type so we have a proper check
     return modelRegistryBody.data !== undefined;
   }
   return false;

clients/ui/frontend/src/shared/api/errorUtils.ts

@@ -17,7 +17,6 @@ export const handleRestFailures = <T>(promise: Promise<T>): Promise<T> =>
       }
       if (isCommonStateError(e)) {
         // Common state errors are handled by useFetchState at storage level, let them deal with it
-        // TODO: check whether we need this or not
         throw e;
       }
       // eslint-disable-next-line no-console

clients/ui/frontend/src/shared/hooks/useSettings.tsx

@@ -62,7 +62,7 @@ export const useSettings = (): {
 };
 
 // Mock a settings config call
-// TODO: replace with thea actual call once we have the endpoint
+// TODO: [Data Flow] replace with thea actual call once we have the endpoint
 export const fetchConfig = async (): Promise<ConfigSettings> => ({
   common: {
     featureFlags: {
@@ -72,7 +72,7 @@ export const fetchConfig = async (): Promise<ConfigSettings> => ({
 });
 
 // Mock a settings user call
-// TODO: replace with thea actual call once we have the endpoint
+// TODO: [Auth-enablement] replace with thea actual call once we have the endpoint
 export const fetchUser = async (): Promise<UserSettings> => ({
   username: 'admin',
   isAdmin: true,

clients/ui/frontend/src/shared/hooks/useTimeBasedRefresh.ts

@@ -33,7 +33,7 @@ const useTimeBasedRefresh = (): SetTime => {
     if (lastDate < refreshDateMarker) {
       setNewDateString(refreshDateMarker.toString());
       console.log('Logging out and refreshing');
-      // TODO: Replace with actual logout function
+      // TODO: [Auth-enablement] Replace with actual logout function
       //logout().then(() => window.location.reload());
     } else {
       console.error(

clients/ui/frontend/src/shared/types.ts

@@ -1,23 +1,23 @@
 import { ValueOf } from '~/shared/typeHelpers';
 
-//  TODO: Get the status config params
+//  TODO: [Data Flow] Get the status config params
 export type UserSettings = {
   username: string;
   isAdmin: boolean;
   isAllowed: boolean;
 };
 
-// TODO: Add more config parameters
+// TODO: [Data Flow] Add more config parameters
 export type ConfigSettings = {
   common: CommonConfig;
 };
 
-// TODO: Add more config parameters
+// TODO: [Data Flow] Add more config parameters
 export type CommonConfig = {
   featureFlags: FeatureFlag;
 };
 
-// TODO: Add more config parameters
+// TODO: [Data Flow] Add more config parameters
 export type FeatureFlag = {
   modelRegistry: boolean;
 };

clients/ui/frontend/src/shared/utilities/const.ts

@@ -1,4 +1,4 @@
-// TODO: Fetch the .env variable here.
+// TODO: [Env Handling] Fetch the .env variable here.
 const POLL_INTERVAL = 30000;
 
 export { POLL_INTERVAL };

clients/ui/frontend/src/shared/utilities/time.ts

@@ -1,4 +1,4 @@
-// TODO: Trimed down version of the original file. Needs to be updated with the original file.
+// TODO: [Reusable Codebase] Trimed down version of the original file. Needs to be updated with the original file.
 
 const printAgo = (time: number, unit: string) => `${time} ${unit}${time > 1 ? 's' : ''} ago`;
 const printIn = (time: number, unit: string) => `in ${time} ${unit}${time > 1 ? 's' : ''}`;