-
Notifications
You must be signed in to change notification settings - Fork 1
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
Stock center/plasmids #692
Conversation
…-ts functions for improved readability and maintainability The strainConfig function has been refactored to use fp-ts functions for improved readability and maintainability. The pipe function is used to compose the array of search config members and the map function from the ReadonlyNonEmptyArray module is used to apply the baseConfig to each member. This refactoring improves the code by making it more functional and easier to understand.
…dability and maintainability The import statements have been formatted to be more readable by grouping them and adding line breaks. The head function from the fp-ts/ReadonlyArray module has been added to the import statements. The getOrElse function from the fp-ts/Option module has been added to the import statements. The function name has been changed from useConfigureStrainCatalogSearchGraphql to getStrainListConfiguration to better reflect its purpose. The code has been refactored to use lenses for accessing and modifying properties of objects. This improves code readability and maintainability. The basePipe has been refactored to use the head function from the fp-ts/ReadonlyNonEmptyArray module as a fallback when the filter does not match any elements. The graphqlPipe has been renamed to strainTypeFilterPipe to better reflect its purpose. The filterPipe has been renamed to additionalfiltersPipe to better reflect its purpose. The code has been formatted to improve readability and maintainability.
The types.ts file is added to the stock-center app. It includes types for PurchaseProperties, StrainItem, PlasmidItem, CatalogItem, StrainCartItem, CartItemLimit, and Cart. These types are used to define the structure of various items and the cart in the stock center app.
- Move the `PlasmidItem` type import to the local file to improve encapsulation and reduce dependencies. - Add support for plasmid items in the cart by introducing a new `plasmidItems` array in the `Cart` type. - Create a new `plasmidItemsAtom` to manage the state of the plasmid items in the cart. - Add a new `plasmidItemsAtom` to the `splitAtom` function to enable separate management of strain and plasmid items. - Create a new `remainingCartSpaceAtom` to calculate the remaining space in the cart based on the maximum items allowed. - Add a new `addPlasmidItemsAtom` to handle the addition of plasmid items to the cart, taking into account the remaining space. - Update the export statements to include the new types and atoms related to plasmid items. The changes were made to enhance the functionality of the stock center application by allowing users to add plasmid items to their cart. This required introducing new types, atoms, and state management for plasmid items.
… definition for PlasmidItem The import for StrainItem is removed as it is no longer used in the component. Additionally, a type definition for PlasmidItem is added to the RemoveFromCartButtonProperties type to allow for both StrainItem and PlasmidItem to be used as item props.
- Rename `addItemsAtom` to `addStrainItemsAtom` and `addPlasmidItemsAtom` to improve clarity and specificity. - Update type imports for `StrainItem`, `PlasmidItem`, and `CatalogItem` to reflect changes in the state. - Refactor `appendFee` function to `appendStrainFee` and `appendPlasmidFee` to improve semantics and specificity. - Refactor `handleClick` function to use `ts-pattern` library for pattern matching on `items` array. - Update function calls to `addStrainItems` and `addPlasmidItems` to reflect changes in the state. The changes were made to improve code readability and maintainability. The renaming of variables and functions provides clearer and more specific names, making it easier to understand their purpose. The use of `ts-pattern` library simplifies the logic for handling different types of items in the `items` array.
…Atom for clarity and consistency The addItemsAtom variable has been renamed to addStrainItemsAtom to improve clarity and consistency with the naming conventions used in the codebase.
… to accept a generic item type instead of specific properties The useCartItemProperties hook has been updated to accept a generic item type instead of specific properties from the Strain type. This change improves reusability and flexibility of the hook, allowing it to be used with different item types.
…Item to use as type for 'item' prop The 'data' prop in the AddToCartButton component has been renamed to 'items' to improve clarity and consistency. Additionally, the types StrainItem and PlasmidItem have been imported and used as the type for the 'item' prop to ensure type safety.
The PlasmidCatalog page component is added to the stock-center application. This component is responsible for displaying the plasmid catalog, including a search bar, a table of plasmids, and loading and error displays. It also includes functionality for fetching more plasmids when the user scrolls to the bottom of the page.
…f getStrainListConfiguration function call The CatalogTableDisplay component has been renamed to StrainCatalogTableDisplay to provide better clarity and consistency in naming. The useMemo hook is now used to memoize the result of the getStrainListConfiguration function call, which improves performance by avoiding unnecessary re-computation of the configuration.
…em type to represent both StrainItem and PlasmidItem The types.ts file has been modified to import the Plasmid type from "dicty-graphql-schema" and include it in the existing types. The StrainItem type has been updated to include the "__typename" field from the Strain type. Additionally, a new PlasmidItem type has been created to represent the fields of the Plasmid type. To handle both StrainItem and PlasmidItem, a new CatalogItem type has been introduced. This change allows for more flexibility when working with different types of items in the catalog.
…ogTableDisplay to display plasmid data in table format The import statements for the types CatalogItem, StrainItem, and PlasmidItem have been corrected to ensure the correct types are used. The cellFunction in CatalogTableDisplay now uses pattern matching to determine the itemPathSegment and itemIdentifier based on the type of the item. Additionally, a new component PlasmidCatalogTableDisplay has been created to display plasmid data in a table format. This improves code organization and allows for better separation of concerns.
The code in `graphql_cache.ts` has been modified to add support for pagination of the plasmid list. This includes defining types for the plasmid list, implementing pagination logic for merging and reading the plasmid list, and exporting the necessary functions for pagination. This change allows for more efficient handling of large plasmid lists in the application.
…s and unused code The commented out console.log statements and unused code have been removed to improve code cleanliness and readability.
The `listPlasmidsPagination` function is added to the `Query` fields in order to support listing plasmids in the application. This allows users to view and interact with plasmid data alongside strain data.
…ias for better clarity and consistency The type imports for Strain and the type alias for StrainItem and PlasmidItem have been removed as they are not used in the code. Instead, the type imports have been renamed to CatalogItem for better clarity and consistency.
…em and PlasmidItem to CatalogItem for better clarity and consistency The types StrainItem and PlasmidItem have been renamed to CatalogItem to provide better clarity and consistency in the codebase. This change improves the understanding of the types used in the RemoveFromCartButton component.
…iptor for better clarity and consistency The variable name itemIdentifier has been changed to itemDescriptor to provide a more descriptive and consistent name. This improves the readability and maintainability of the code.
…for clarity The StrainItem type has been renamed to CatalogItem to better reflect the purpose of the type and improve code readability and maintainability.
…use getCatalogItemDescriptor function The changes were made to improve code readability and maintainability by using more descriptive names like CatalogItem instead of StrainItem. The update to the item rendering logic allows for better abstraction and separation of concerns by utilizing the getCatalogItemDescriptor function to handle item details.
…escriptor using a custom function getCatalogItemDescriptor The ts-pattern library is replaced with a custom function getCatalogItemDescriptor to simplify the logic and improve maintainability. This change allows for a more flexible and scalable way to retrieve the catalog item descriptor based on the item type.
…logRows for clarity and consistency The Rows component has been renamed to CatalogRows to improve clarity and maintain consistency with the naming conventions used in the project. This change enhances readability and makes the codebase more cohesive.
…ath and descriptor based on item type The getCatalogItemDescriptor function is added to the utils folder in the ui-dsc package. This function takes a CatalogItem object as input and uses pattern matching to determine the item type (plasmid or strain) and return the corresponding item path and descriptor. This function enhances the codebase by providing a clean and concise way to retrieve specific information based on the item type.
…tItem for better clarity and consistency The type name StrainCartItem has been changed to CatalogCartItem to provide better clarity and consistency with the naming conventions used in the application. This change ensures that the type accurately represents the data it is intended to describe.
The `getCatalogItemPathAndDescriptor` utility function is added to the `ui-dsc` package. This function takes a `CatalogItem` object as input and returns an object with `itemPath` and `itemDescriptor` properties based on the type of the `CatalogItem`. If the `CatalogItem` has a `name` property, it is considered a `plasmid` and the `itemPath` is set to "plasmids" and the `itemDescriptor` is set to the `name` property. If the `CatalogItem` has a `label` property, it is considered a `strain` and the `itemPath` is set to "strains" and the `itemDescriptor` is set to the `label` property. If the `CatalogItem` does not match any of these patterns, the `itemPath` and `itemDescriptor` are set to empty strings. This utility function is added to provide a convenient way to extract the `itemPath` and `itemDescriptor` from a `CatalogItem` object in the UI of the `ui-dsc` package.
…thAndDescriptor for clarity and consistency The function getCatalogItemDescriptor has been renamed to getCatalogItemPathAndDescriptor to provide a more accurate and descriptive name. This change improves clarity and consistency throughout the codebase.
…tinueShoppingCard component The ContinueShoppingCard component now includes two buttons: one for the Strains Catalog and one for the Plasmids Catalog. This allows users to easily navigate to the respective catalog pages from the cart.
…getCatalogItemPathAndDescriptor utility function The type StrainCartItem has been renamed to CatalogCartItem to provide a more accurate and descriptive name. The import and usage of the strainOrPlasmid utility function has been replaced with the getCatalogItemPathAndDescriptor utility function to retrieve the itemPath and itemDescriptor. This change improves clarity and consistency in the codebase.
The HeaderRow component now includes support for plasmid items in addition to strain items in the cart. The plasmidItemsAtom is imported and used to calculate the total number of items in the cart by combining the strainItems and plasmidItems arrays. The CartIcon component now receives the updated cartItems array as the items prop. This change allows for a more comprehensive representation of the items in the cart.
…function instead of just the id The change ensures that the removeItem function receives the entire item object instead of just the id, which may be needed for additional operations or checks within the function.
…r readability and maintainability The removeItemAtom function in state.ts has been refactored to use ts-pattern library for pattern matching on the removed item's type. This change improves the code readability and maintainability by clearly handling different types of items (Strain or Plasmid) and removing the correct item from the corresponding atom.
…s to improve readability and maintainability The import syntax for type declarations in SubmitButton.tsx has been changed to use the `import type` syntax. This improves the readability of the code and makes it clear that these imports are only for type declarations and not for actual runtime imports. This change also helps with maintainability as it separates the type imports from the regular imports, making it easier to identify and manage them.
…rray<StrainCartItem> to Array<CatalogCartItem> for consistency and clarity The type of cartItems in the OrderState has been changed from Array<StrainCartItem> to Array<CatalogCartItem> to improve consistency and clarity in the codebase.
…bmission. The changes to the SubmitButton component allow it to handle both strainItems and plasmidItems in the cart. The updates to the getIDs and getOrderVariables functions ensure that the correct item type is used. Finally, the updates to the setOrder function ensure that the cartItems and cartTotal accurately reflect allItems.
Warning Rate Limit Exceeded@ktun95 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 12 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe recent update enhances the stock center application by introducing a Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (30)
- apps/stock-center/src/App.tsx (2 hunks)
- apps/stock-center/src/components/AddToCartButton.tsx (2 hunks)
- apps/stock-center/src/components/AddToCartButtonHandler.tsx (2 hunks)
- apps/stock-center/src/components/AvailableDisplay.tsx (2 hunks)
- apps/stock-center/src/components/CartList.tsx (5 hunks)
- apps/stock-center/src/components/HeaderRow.tsx (3 hunks)
- apps/stock-center/src/components/PaymentPage.tsx (1 hunks)
- apps/stock-center/src/components/RemoveFromCartButton.tsx (2 hunks)
- apps/stock-center/src/components/SubmitButton.tsx (4 hunks)
- apps/stock-center/src/hooks/useCartItemProperties.ts (1 hunks)
- apps/stock-center/src/pages/cart.tsx (2 hunks)
- apps/stock-center/src/pages/plasmids/index.tsx (1 hunks)
- apps/stock-center/src/pages/strains/index.tsx (2 hunks)
- apps/stock-center/src/state.ts (4 hunks)
- apps/stock-center/src/types.ts (1 hunks)
- packages/hook-dsc/src/graphql_cache.ts (5 hunks)
- packages/hook-dsc/src/graphql_config.ts (2 hunks)
- packages/hook-dsc/src/useConfigureStrainCatalogSearchGraphql.ts (3 hunks)
- packages/ui-dsc/src/tests/CatalogTableDisplay.test.tsx (3 hunks)
- packages/ui-dsc/src/cart/CartIcon.tsx (2 hunks)
- packages/ui-dsc/src/cart/CartItem.tsx (4 hunks)
- packages/ui-dsc/src/cart/ContinueShoppingCard.tsx (1 hunks)
- packages/ui-dsc/src/catalog/AddToCartDialog.tsx (1 hunks)
- packages/ui-dsc/src/catalog/AddToCartDialogContent.tsx (2 hunks)
- packages/ui-dsc/src/catalog/CatalogTableDisplay.tsx (6 hunks)
- packages/ui-dsc/src/order/OrderSummary.tsx (3 hunks)
- packages/ui-dsc/src/order/OrderSummaryListItems.tsx (1 hunks)
- packages/ui-dsc/src/types.ts (2 hunks)
- packages/ui-dsc/src/utils/getCartTotal.ts (1 hunks)
- packages/ui-dsc/src/utils/getCatalogItemPathAndDescriptor.ts (1 hunks)
Additional comments: 53
packages/ui-dsc/src/utils/getCartTotal.ts (1)
- 1-7: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-9]
The update to use
CatalogCartItem
in thegetCartTotal
function is a good approach for handling both strain and plasmid items in a unified way. This change aligns well with the PR objectives of integrating plasmids into the stock center application.packages/ui-dsc/src/utils/getCatalogItemPathAndDescriptor.ts (1)
- 1-19: The use of pattern matching in
getCatalogItemPathAndDescriptor
function is an excellent choice for handling different types of catalog items. This approach enhances code readability and maintainability, aligning well with the PR objectives.apps/stock-center/src/pages/cart.tsx (1)
- 14-27: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-24]
The refactoring of the
CartHandler
component to use pattern matching for conditional rendering is a significant improvement. It simplifies the logic and enhances code readability, aligning well with the PR objectives.apps/stock-center/src/hooks/useCartItemProperties.ts (1)
- 1-28: The updates to the
useCartItemProperties
hook, including the use of pattern matching and the transition tocartAtom
, significantly improve the code's readability and maintainability. These changes align well with the PR objectives.apps/stock-center/src/components/RemoveFromCartButton.tsx (1)
- 12-29: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-33]
The update to use
CatalogItem
in theRemoveFromCartButton
component and pass the entire item object to theremoveItem
function is a positive change. It simplifies the component's interface and enhances flexibility, aligning with the PR objectives.packages/ui-dsc/src/types.ts (1)
- 1-24: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-45]
The introduction of
PlasmidItem
,PlasmidCartItem
, and the unifiedCatalogItem
type, along with the updates to theCart
type, are crucial for integrating plasmids into the application. These changes are well-designed and align with the PR objectives.apps/stock-center/src/components/HeaderRow.tsx (1)
- 18-28: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-39]
The update in the
HeaderRow
component to includeplasmidItems
alongsidestrainItems
for the cart items count is a thoughtful enhancement. It aligns with the PR objectives and improves the application's usability by accurately reflecting the total number of items in the cart.packages/ui-dsc/src/catalog/AddToCartDialogContent.tsx (1)
- 16-37: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-34]
The updates to the
AddToCartDialogContent
component, including the use ofCatalogItem
and thegetCatalogItemPathAndDescriptor
utility function, significantly enhance the component's flexibility. These changes align well with the PR objectives and improve the user experience by providing a seamless way to manage both strains and plasmids.packages/ui-dsc/src/catalog/AddToCartDialog.tsx (2)
- 5-5: Renaming
StrainItem
toCatalogItem
is a good approach for generalizing the type to accommodate both strains and plasmids. This change enhances the modularity and flexibility of the component.- 9-11: The update to the
data
andsetCheckedItems
properties to useCatalogItem
aligns with the goal of handling both strains and plasmids. This is a positive change for consistency and future maintainability.packages/ui-dsc/src/cart/CartIcon.tsx (2)
- 6-6: Updating the import to use
CatalogCartItem
supports the integration of plasmids alongside strains. This change ensures that the cart can handle a broader range of item types, improving the application's flexibility.- 22-22: Changing the
items
property to an array ofCatalogCartItem
is a necessary update for accommodating both strains and plasmids in the cart. This enhances the component's versatility.packages/hook-dsc/src/graphql_config.ts (1)
- 19-44: The refactoring of the
strainConfig
function to return aReadonlyNonEmptyArray
and the use offp-ts
functions for mapping enhances type safety and functional programming practices. This change improves the maintainability and readability of the code by leveraging thefp-ts
library's capabilities.apps/stock-center/src/components/AddToCartButtonHandler.tsx (2)
- 7-7: Switching to the
CatalogItem
type for theitem
property inAddToCartButtonHandlerProperties
is a strategic move to accommodate both strains and plasmids. This change promotes code reuse and simplifies handling different item types.- 30-30: Passing an array of items to the
AddToCartButton
component, even if it's a single item, is a good practice for maintaining a consistent interface. This change allows for potential future enhancements where multiple items might be added to the cart simultaneously.packages/ui-dsc/src/order/OrderSummaryListItems.tsx (1)
- 28-47: Combining strain and plasmid items in the order summary and using
fp-ts
functions for mapping is a clean and functional approach. This ensures that both item types are treated uniformly, enhancing the user experience by providing a consolidated view of their cart contents.packages/ui-dsc/src/cart/ContinueShoppingCard.tsx (1)
- 44-55: Adding buttons for navigating to the "Strains Catalog" and "Plasmids Catalog" in the
ContinueShoppingCard
component is a user-friendly feature. It provides a clear path for users to continue shopping for different types of items, enhancing the overall usability of the application.apps/stock-center/src/App.tsx (1)
- 42-42: Integrating the
listPlasmidsPagination
function into the Apollo client cache configuration is a crucial step for supporting plasmid listings alongside strains. This change ensures that the application can handle data fetching and caching for both item types efficiently.apps/stock-center/src/types.ts (1)
- 1-78: The introduction and refinement of types such as
StrainItem
,PlasmidItem
,CatalogItem
, and their respective cart item types are well-thought-out. These changes are essential for supporting the handling of both strains and plasmids throughout the application, ensuring type safety and clarity in the codebase.packages/hook-dsc/src/useConfigureStrainCatalogSearchGraphql.ts (3)
- 1-6: The addition of
head
andgetOrElse
imports fromfp-ts
libraries is appropriate for functional programming patterns. Ensure that these functions are used effectively within the file to manipulate arrays and handle optional values.- 24-24: Renaming
useConfigureStrainCatalogSearchGraphql
togetStrainListConfiguration
improves clarity and better reflects the function's purpose, which is to configure the strain list rather than being a React hook. Good naming practice.- 42-71: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [24-69]
The updated logic within
getStrainListConfiguration
function incorporates additional filters and strain type configurations effectively. Using lenses frommonocle-ts
for immutable updates andfp-ts
functions for functional programming patterns is commendable. Ensure that the new logic aligns with the application's requirements for handling strain and plasmid items.apps/stock-center/src/components/AddToCartButton.tsx (3)
- 4-14: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-11]
The imports for
match
andP
fromts-pattern
along with the state atomsaddStrainItemsAtom
andaddPlasmidItemsAtom
are correctly added to support the updated functionality of handling different catalog item types. Good use of external libraries and state management.
- 48-49: The use of
useSetAtom
hook fromjotai
for bothaddStrainItemsAtom
andaddPlasmidItemsAtom
is appropriate for managing the application's state related to cart items. This ensures that the component can update the cart state based on user actions.- 28-61: The updates to the
AddToCartButton
component, including the use ofmatch
andP.array
fromts-pattern
for pattern matching, are well-implemented. This approach allows for clear differentiation betweenStrainItem
andPlasmidItem
types and applies the appropriate fee before adding items to the cart. This is a significant improvement in handling different types of catalog items.apps/stock-center/src/pages/strains/index.tsx (2)
- 1-6: The imports, including
getStrainListConfiguration
from@dictybase/hook-dsc
and components from@dictybase/ui-dsc
, are correctly updated to support the new functionality for strain catalog search configuration and display.- 1-29: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [19-59]
The use of
getStrainListConfiguration
to configure the strain catalog search based on search parameters is a significant improvement. It simplifies the process of obtaining the necessary configuration for querying strain data. Additionally, replacingCatalogTableDisplay
withStrainCatalogTableDisplay
for displaying strain catalog tables is a good practice for clarity and specificity.apps/stock-center/src/pages/plasmids/index.tsx (1)
- 19-76: The updates to use
usePlasmidListFilterQuery
for fetching plasmid data and displaying it usingPlasmidCatalogTableDisplay
are well-implemented. This ensures that the plasmid catalog page correctly fetches and displays plasmid items, aligning with the PR's objectives to integrate plasmids alongside strains.packages/ui-dsc/src/cart/CartItem.tsx (3)
- 9-16: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-13]
The imports, including the updated type
CatalogCartItem
and utility functiongetCatalogItemPathAndDescriptor
, are correctly added to support the updated functionality of theCartItem
component. This aligns with the PR's objectives to handle both strain and plasmid items in the cart.
- 35-35: Updating the type from
CartItem
toCatalogCartItem
in theShoppingCartItemProperties
type declaration improves clarity and specificity, especially in the context of handling both strain and plasmid items in the cart. Good practice for type safety and readability.- 42-54: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [45-61]
The updates within the
CartItem
component, including the use ofgetCatalogItemPathAndDescriptor
to determine the item path and descriptor based on the item type, are well-implemented. This approach allows for a unified component that can handle both strain and plasmid items, aligning with the PR's objectives.packages/ui-dsc/src/order/OrderSummary.tsx (2)
- 5-11: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-8]
The import of
OrderSummaryListItems
is correctly added to support the refactored functionality of rendering order summary items. This aligns with the PR's objectives to improve modularity and readability of the order summary component.
- 59-65: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [62-73]
Refactoring the individual rendering of cart items to use the
OrderSummaryListItems
component is a significant improvement in terms of modularity and readability. This change allows for a cleanerOrderSummary
component structure and aligns with best practices for component design.apps/stock-center/src/components/AvailableDisplay.tsx (3)
- 11-18: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-15]
The reorganization of imports and updates to state atoms in
AvailableDisplay.tsx
are correctly implemented to support the updated functionality. Streamlining imports and adjusting state atoms are good practices for maintaining clean and efficient code.
- 47-47: The use of
useSetAtom
for bothaddStrainItemsAtom
andremoveItemAtom
is appropriate for managing the application's state related to cart items. This ensures that the component can update the cart state based on user actions, aligning with the PR's objectives.- 41-41: Updating the type from
CartItem
toStrainCartItem
in theProperties
type declaration improves clarity and specificity. This change is aligned with the PR's objectives to refine type declarations for better code maintainability and readability.packages/hook-dsc/src/graphql_cache.ts (2)
- 54-81: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [4-77]
The addition of new types and functions related to plasmids, such as
PlasmidList
,PlasmidListPair
,whichPlasmids
, andlistPlasmidsPagination
, is well-implemented. These changes expand the functionality to include plasmid list queries alongside existing strain list queries, aligning with the PR's objectives to integrate plasmids into the application. Ensure that these new functionalities are thoroughly tested to maintain the application's reliability.
- 106-110: The update to export declarations to include
listPlasmidsPagination
is appropriate and ensures that the new functionality related to plasmid list queries can be utilized elsewhere in the application. This aligns with the PR's objectives and best practices for modular code design.apps/stock-center/src/state.ts (4)
- 17-17: The addition of
plasmidItems
to theCart
type aligns with the PR objectives to integrate plasmids alongside strains. This change is crucial for managing both item types within the shopping cart.- 36-40: The implementation of
plasmidItemsAtom
follows the pattern established forstrainItemsAtom
, ensuring consistency in state management for different item types. This approach facilitates the seamless integration of plasmids into the application's existing infrastructure.- 66-76: The
addPlasmidItemsAtom
atom is correctly implemented to handle the addition of plasmid items to the cart. It respects the remaining cart space, ensuring that the cart does not exceed its maximum capacity. This logic is essential for maintaining a consistent user experience across item types.- 79-94: The use of pattern matching with
ts-pattern
to differentiate betweenStrain
andPlasmid
items inremoveItemAtom
is a clean and efficient way to handle item removal from the cart. This approach enhances code readability and maintainability.apps/stock-center/src/components/PaymentPage.tsx (1)
- 26-26: Updating the import location of
ShippingFormData
to../types
from../state
likely reflects a reorganization of type definitions for better modularity and maintainability. This change is a good practice for keeping type definitions in a centralized location.apps/stock-center/src/components/CartList.tsx (4)
- 3-3: Renaming
mapWithIndex
toAmap
in the import statement fromfp-ts/Array
improves readability by aligning with the conventional naming of higher-order functions likemap
. This change enhances the consistency of functional programming practices within the codebase.- 61-72: The
StrainCartItemFromAtom
component is well-implemented, providing a clear and concise way to render strain items within the cart. The use ofuseAtomValue
for accessing atom state and the dispatch pattern for item removal are both effective practices.- 79-90: Similarly, the
PlasmidCartItemFromAtom
component follows the established pattern for rendering cart items, extending it to plasmid items. This consistency in handling different item types is crucial for maintaining a unified codebase and user experience.- 95-101: The logic in
renderTotalRows
to handle different combinations of strain and plasmid items in the cart is a thoughtful addition. It ensures that the total rows are rendered accurately based on the cart's contents, enhancing the user's understanding of their cart composition.packages/ui-dsc/src/__tests__/CatalogTableDisplay.test.tsx (1)
- 97-103: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [100-112]
Renaming the
Rows
component toCatalogRows
in the test cases reflects the changes made in theCatalogTableDisplay
module. This update ensures that the test cases remain aligned with the actual component names, which is crucial for maintaining accurate and meaningful tests.apps/stock-center/src/components/SubmitButton.tsx (2)
- 22-22: The introduction of the
CatalogItem
type and its use in place ofStrainCartItem
for functions likegetIDs
,getOrderVariables
, and within theSubmitButton
component itself is a significant improvement. It allows for a more generic handling of items, accommodating both strains and plasmids, aligning with the PR's objectives.- 192-204: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [157-201]
Including
plasmidItems
in the state and operations of theSubmitButton
component is a crucial update for supporting transactions involving both strains and plasmids. This change ensures that the application can handle orders containing a mix of item types, reflecting the enhanced functionality introduced by the PR.packages/ui-dsc/src/catalog/CatalogTableDisplay.tsx (3)
- 18-18: The addition of
CatalogItem
,StrainItem
, andPlasmidItem
types is a foundational change that supports the flexible handling of different item types within the catalog display components. This change is essential for the integration of plasmids alongside strains in the application.- 123-133: The introduction of the
CatalogRows
component for handling the display of items, whether they are strains or plasmids, is a significant enhancement. This component abstracts the common functionality needed for rendering items in a table format, improving code reusability and maintainability.- 190-212: Adding the
PlasmidCatalogTableDisplay
component specifically for plasmid data is a clear indication of the application's expanded functionality to include plasmids. This component mirrors the structure of the existing catalog display components, ensuring consistency in the user interface.
import { | ||
getStrainListConfiguration, | ||
defaultFilter, | ||
graphqlQueryVariables, | ||
} from "@dictybase/hook-dsc" | ||
import { P, match } from "ts-pattern" | ||
import { | ||
LoadingDisplay, | ||
PlasmidCatalogTableDisplay, | ||
ErrorDisplay, | ||
SearchBar, | ||
CatalogListWrapper, | ||
CatalogHeader, | ||
} from "@dictybase/ui-dsc" | ||
import { useIntersectionObserver } from "@dictybase/hook" | ||
import { useRef } from "react" | ||
import { usePlasmidListFilterQuery } from "dicty-graphql-schema" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The imports for graphqlQueryVariables
from @dictybase/hook-dsc
and components from @dictybase/ui-dsc
are correctly updated to support the new functionality for plasmid catalog display. However, there are unused imports that should be removed to clean up the code.
- import { getStrainListConfiguration, defaultFilter } from "@dictybase/hook-dsc"
- import { SearchBar } from "@dictybase/ui-dsc"
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
import { | |
getStrainListConfiguration, | |
defaultFilter, | |
graphqlQueryVariables, | |
} from "@dictybase/hook-dsc" | |
import { P, match } from "ts-pattern" | |
import { | |
LoadingDisplay, | |
PlasmidCatalogTableDisplay, | |
ErrorDisplay, | |
SearchBar, | |
CatalogListWrapper, | |
CatalogHeader, | |
} from "@dictybase/ui-dsc" | |
import { useIntersectionObserver } from "@dictybase/hook" | |
import { useRef } from "react" | |
import { usePlasmidListFilterQuery } from "dicty-graphql-schema" | |
import { | |
graphqlQueryVariables, | |
} from "@dictybase/hook-dsc" | |
import { P, match } from "ts-pattern" | |
import { | |
LoadingDisplay, | |
PlasmidCatalogTableDisplay, | |
ErrorDisplay, | |
CatalogListWrapper, | |
CatalogHeader, | |
} from "@dictybase/ui-dsc" | |
import { useIntersectionObserver } from "@dictybase/hook" | |
import { useRef } from "react" | |
import { usePlasmidListFilterQuery } from "dicty-graphql-schema" |
Three test files have been updated to account for the addition of the Plasmid catalog. The prop `strainItems` in CatalogRows has been updated to `items`. The ContinueShoppingCard's has been changed to have two buttons: one for the Strain Catalog and one for the Plasmid Catalog. The tests for the ContinueShoppingCard have been updated to look for these elements.
getStrainListConfiguration, defaultFilter, and Searchbar were not being used and were therefore removed from the file.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #692 +/- ##
===========================================
+ Coverage 94.75% 94.81% +0.05%
===========================================
Files 192 194 +2
Lines 14241 14437 +196
Branches 494 498 +4
===========================================
+ Hits 13494 13688 +194
- Misses 747 749 +2
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (4)
- apps/stock-center/src/pages/plasmids/index.tsx (1 hunks)
- packages/ui-dsc/src/tests/CatalogTableDisplay.test.tsx (3 hunks)
- packages/ui-dsc/src/tests/ContinueShoppingCard.test.tsx (2 hunks)
- packages/ui-dsc/src/tests/OrderSummary.test.tsx (3 hunks)
Additional Context Used
GitHub Check Runs (1)
ESLint Report Analysis failure (1)
apps/stock-center/src/pages/plasmids/index.tsx: [failure] 1-3: apps/stock-center/src/pages/plasmids/index.tsx#L1-L3
[prettier/prettier] Replace⏎··graphqlQueryVariables,⏎
with·graphqlQueryVariables·
Additional comments not posted (9)
packages/ui-dsc/src/__tests__/ContinueShoppingCard.test.tsx (1)
10-11
: The addition of a new route for "/plasmids" and the update to the "/strains" route are correctly implemented, ensuring the application can navigate between these two sections effectively.packages/ui-dsc/src/__tests__/OrderSummary.test.tsx (4)
5-5
: The import ofPlasmidCartItem
alongsideStrainCartItem
is correctly added to support the new plasmid items in the order summary tests.
14-21
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [8-18]
The separation of strain and plasmid items into
strainItems
andplasmidItems
arrays is a good approach for handling different item types in the cart. This change enhances the clarity and maintainability of the test setup.
30-37
: The test correctly asserts the presence of strain and plasmid item IDs in the document. However, it's important to also verify the quantity and fee for both item types to ensure the order summary displays all necessary information accurately.Consider adding assertions to verify the quantity and fee for both strain and plasmid items.
44-44
: The test for correct address formatting is well-implemented, ensuring that shipping and payment addresses, as well as shipping account numbers and payment types, are displayed correctly.apps/stock-center/src/pages/plasmids/index.tsx (2)
16-42
: The implementation of infinite scrolling using theuseIntersectionObserver
hook and thefetchMore
function from the GraphQL query is correctly set up. This approach allows for efficient loading of additional plasmid data as the user scrolls, enhancing the user experience.
49-66
: The use of thets-pattern
library for conditional rendering based on thedata
,loading
, anderror
states is a clean and readable approach. It effectively handles the different states of the GraphQL query, ensuring that the appropriate UI elements are displayed.packages/ui-dsc/src/__tests__/CatalogTableDisplay.test.tsx (2)
9-9
: The renaming of theRows
component toCatalogRows
is correctly reflected in the import statement. This change likely aligns with broader naming conventions and clarifies the component's purpose.
97-104
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [100-113]
The tests for the
CatalogRows
component correctly verify the rendering of table rows for each item in the provided array and the additional row for fetching more data whennextCursor
is greater than 1. This ensures that the component behaves as expected in different scenarios.
test("should render a Button with the text 'Plasmids Catalog' that links user to the /strains route", async () => { | ||
render(<App />) | ||
const buttonElement = screen.getByText("Plasmids Catalog") | ||
|
||
await userEvent.click(buttonElement) | ||
|
||
expect(screen.getByText("Plasmid Catalog")).toBeInTheDocument() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a discrepancy in the test description for the 'Plasmids Catalog' button. The description mentions that it links to the "/strains" route, but based on the implementation, it should link to the "/plasmids" route. Please correct the test description to accurately reflect the intended functionality.
- test("should render a Button with the text 'Plasmids Catalog' that links user to the /strains route", async () => {
+ test("should render a Button with the text 'Plasmids Catalog' that links user to the /plasmids route", async () => {
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
test("should render a Button with the text 'Plasmids Catalog' that links user to the /strains route", async () => { | |
render(<App />) | |
const buttonElement = screen.getByText("Plasmids Catalog") | |
await userEvent.click(buttonElement) | |
expect(screen.getByText("Plasmid Catalog")).toBeInTheDocument() | |
test("should render a Button with the text 'Plasmids Catalog' that links user to the /plasmids route", async () => { | |
render(<App />) | |
const buttonElement = screen.getByText("Plasmids Catalog") | |
await userEvent.click(buttonElement) | |
expect(screen.getByText("Plasmid Catalog")).toBeInTheDocument() |
import { | ||
graphqlQueryVariables, | ||
} from "@dictybase/hook-dsc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement for graphqlQueryVariables
can be simplified according to the static analysis hint. This change will improve code readability.
- import {
- graphqlQueryVariables,
- } from "@dictybase/hook-dsc"
+ import { graphqlQueryVariables } from "@dictybase/hook-dsc"
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
import { | |
graphqlQueryVariables, | |
} from "@dictybase/hook-dsc" | |
import { graphqlQueryVariables } from "@dictybase/hook-dsc" |
The test description for the Plasmids Catalog button has been updated to say that it should link to the `/plasmids` route instead of the `/strains` route.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- apps/stock-center/src/pages/plasmids/index.tsx (1 hunks)
- packages/ui-dsc/src/tests/ContinueShoppingCard.test.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- apps/stock-center/src/pages/plasmids/index.tsx
- packages/ui-dsc/src/tests/ContinueShoppingCard.test.tsx
Additional Context Used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (3)
- packages/ui-dsc/src/tests/OrderSummary.test.tsx (3 hunks)
- packages/ui-dsc/src/tests/SearchPhenotypeContainer.test.tsx (1 hunks)
- packages/ui-dsc/src/functional.tsx (1 hunks)
Additional Context Used
GitHub Check Runs (1)
ESLint Report Analysis failure (4)
packages/ui-dsc/src/tests/OrderSummary.test.tsx: [failure] 5-5: packages/ui-dsc/src/tests/OrderSummary.test.tsx#L5
[import/named] PlasmidCartItem not found in '../types'
packages/ui-dsc/src/tests/SearchPhenotypeContainer.test.tsx: [failure] 25-27: packages/ui-dsc/src/tests/SearchPhenotypeContainer.test.tsx#L25-L27
[prettier/prettier] Replace·await·vi.importActual<⏎····typeof·import("react-router-dom")⏎··
with⏎····await·vi.importActual<typeof·import("react-router-dom")
packages/ui-dsc/src/functional.tsx: [failure] 76-76: packages/ui-dsc/src/functional.tsx#L76
[prettier/prettier] Insert·(
packages/ui-dsc/src/functional.tsx: [failure] 77-77: packages/ui-dsc/src/functional.tsx#L77
[prettier/prettier] Insert⏎)
Additional comments not posted (3)
packages/ui-dsc/src/__tests__/OrderSummary.test.tsx (3)
2-11
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [8-17]
The update to rename
items
tostrainItems
and its definition looks good.
14-21
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [18-27]
The introduction and definition of the
plasmidItems
array are well-implemented.
30-43
: The updated test assertions correctly reference items from bothstrainItems
andplasmidItems
, ensuring the tests accurately reflect the inclusion of plasmid items alongside strains.
packages/ui-dsc/src/functional.tsx
Outdated
const panelWrapper = (title: string) => (element: JSX.Element) => | ||
<PanelWrapper title={title}>{element}</PanelWrapper> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The modification to panelWrapper
improves readability by removing unnecessary parentheses. However, there are formatting issues flagged by static analysis. Please apply the following formatting corrections:
- const panelWrapper = (title: string) => (element: JSX.Element) =>
+ const panelWrapper = (title: string) => (element: JSX.Element) => (
<PanelWrapper title={title}>{element}</PanelWrapper>
-
+ )
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
const panelWrapper = (title: string) => (element: JSX.Element) => | |
<PanelWrapper title={title}>{element}</PanelWrapper> | |
const panelWrapper = (title: string) => (element: JSX.Element) => ( | |
<PanelWrapper title={title}>{element}</PanelWrapper> | |
) |
const originalModule = await vi.importActual< | ||
typeof import("react-router-dom") | ||
>("react-router-dom") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reformatting of the react-router-dom
import statement introduces a formatting issue. Please apply the following correction to align with prettier standards:
- const originalModule = await vi.importActual<
- typeof import("react-router-dom")
- >("react-router-dom")
+ const originalModule = await vi.importActual<typeof import("react-router-dom")>(
+ "react-router-dom"
+ )
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
const originalModule = await vi.importActual< | |
typeof import("react-router-dom") | |
>("react-router-dom") | |
const originalModule = await vi.importActual<typeof import("react-router-dom")>( | |
"react-router-dom" | |
) |
White spacining has been added in the render calls in the OrderSummary test files to conform with our formatting standards.
5471886
to
bb0e1d2
Compare
The PlasmidCartItem type was not being exported from types.ts. This commit adds PlasmidCartItem to the exports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/ui-dsc/src/tests/OrderSummary.test.tsx (3 hunks)
Additional Context Used
GitHub Check Runs (1)
ESLint Report Analysis failure (1)
packages/ui-dsc/src/tests/OrderSummary.test.tsx: [failure] 5-5: packages/ui-dsc/src/tests/OrderSummary.test.tsx#L5
[import/named] PlasmidCartItem not found in '../types'
Additional comments not posted (1)
packages/ui-dsc/src/__tests__/OrderSummary.test.tsx (1)
39-39
: There's a commented-out assertion related to the quantity of strain items. If this assertion is no longer relevant, consider removing the comment to keep the test code clean and focused. If it's temporarily disabled for a specific reason, adding a comment to explain the context would be helpful.
@@ -2,10 +2,10 @@ | |||
import { render, screen } from "@testing-library/react" | |||
import { OrderSummary } from "../order/OrderSummary" | |||
import { mockValues } from "../utils/mockValues" | |||
import { StrainCartItem } from "../types" | |||
import { StrainCartItem, PlasmidCartItem } from "../types" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import of PlasmidCartItem
is flagged as not found in '../types'. Please verify that PlasmidCartItem
is correctly exported from the types file and that the import path is correct. If PlasmidCartItem
is indeed exported and the path is correct, this might be a false positive, but it's crucial to ensure the integrity of imports for the test suite to function correctly.
A test was added that renders a CatalogRow with plasmid data. The test checks whether a row element is created for each item in the plasmid data array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (3)
- packages/ui-dsc/src/tests/CatalogTableDisplay.test.tsx (3 hunks)
- packages/ui-dsc/src/mocks/mockPlasmids.ts (1 hunks)
- packages/ui-dsc/src/types.ts (2 hunks)
Additional Context Used
GitHub Check Runs (1)
ESLint Report Analysis failure (30)
packages/ui-dsc/src/tests/CatalogTableDisplay.test.tsx: [failure] 136-136: packages/ui-dsc/src/tests/CatalogTableDisplay.test.tsx#L136
[prettier/prettier] Delete·
packages/ui-dsc/src/tests/CatalogTableDisplay.test.tsx: [failure] 158-159: packages/ui-dsc/src/tests/CatalogTableDisplay.test.tsx#L158-L159
[prettier/prettier] Delete⏎
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 52-52: packages/ui-dsc/src/mocks/mockPlasmids.ts#L52
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 53-53: packages/ui-dsc/src/mocks/mockPlasmids.ts#L53
[prettier/prettier] Replace····
with··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 54-54: packages/ui-dsc/src/mocks/mockPlasmids.ts#L54
[prettier/prettier] Replace······
with····
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 4-4: packages/ui-dsc/src/mocks/mockPlasmids.ts#L4
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 55-55: packages/ui-dsc/src/mocks/mockPlasmids.ts#L55
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 5-5: packages/ui-dsc/src/mocks/mockPlasmids.ts#L5
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 6-6: packages/ui-dsc/src/mocks/mockPlasmids.ts#L6
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 7-7: packages/ui-dsc/src/mocks/mockPlasmids.ts#L7
[prettier/prettier] Replace······summary:
with····summary:⏎·····
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 8-8: packages/ui-dsc/src/mocks/mockPlasmids.ts#L8
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 9-9: packages/ui-dsc/src/mocks/mockPlasmids.ts#L9
[prettier/prettier] Replace······__typename:·"Plasmid"
with····__typename:·"Plasmid",
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 56-56: packages/ui-dsc/src/mocks/mockPlasmids.ts#L56
[prettier/prettier] Replace··summary:
withsummary:⏎·····
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 10-10: packages/ui-dsc/src/mocks/mockPlasmids.ts#L10
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 57-57: packages/ui-dsc/src/mocks/mockPlasmids.ts#L57
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 11-11: packages/ui-dsc/src/mocks/mockPlasmids.ts#L11
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 58-58: packages/ui-dsc/src/mocks/mockPlasmids.ts#L58
[prettier/prettier] Replace······__typename:·"Plasmid"
with····__typename:·"Plasmid",
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 12-12: packages/ui-dsc/src/mocks/mockPlasmids.ts#L12
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 59-59: packages/ui-dsc/src/mocks/mockPlasmids.ts#L59
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 13-13: packages/ui-dsc/src/mocks/mockPlasmids.ts#L13
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 60-60: packages/ui-dsc/src/mocks/mockPlasmids.ts#L60
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 14-14: packages/ui-dsc/src/mocks/mockPlasmids.ts#L14
[prettier/prettier] Replace······summary:
with····summary:⏎·····
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 61-61: packages/ui-dsc/src/mocks/mockPlasmids.ts#L61
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 15-15: packages/ui-dsc/src/mocks/mockPlasmids.ts#L15
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 62-62: packages/ui-dsc/src/mocks/mockPlasmids.ts#L62
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 16-16: packages/ui-dsc/src/mocks/mockPlasmids.ts#L16
[prettier/prettier] Replace······__typename:·"Plasmid"
with····__typename:·"Plasmid",
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 63-63: packages/ui-dsc/src/mocks/mockPlasmids.ts#L63
[prettier/prettier] Replace······summary:
with····summary:⏎·····
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 17-17: packages/ui-dsc/src/mocks/mockPlasmids.ts#L17
[prettier/prettier] Replace····
with··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 64-64: packages/ui-dsc/src/mocks/mockPlasmids.ts#L64
[prettier/prettier] Delete··
packages/ui-dsc/src/mocks/mockPlasmids.ts: [failure] 18-18: packages/ui-dsc/src/mocks/mockPlasmids.ts#L18
[prettier/prettier] Delete··
Additional comments not posted (9)
packages/ui-dsc/src/types.ts (6)
6-8
: Consider adding comments to theStrainItem
type to briefly describe its purpose and usage within the application. This can improve code readability and maintainability.
10-12
: ThePlasmidItem
type is well-defined and aligns with the new functionality. However, adding comments to describe its purpose and how it's used could enhance code readability.
14-14
: TheCatalogItem
type, which is a union ofStrainItem
andPlasmidItem
, is a good use of TypeScript's type system for polymorphism. This approach facilitates the handling of catalog items in a generic manner.
16-16
: ThePlasmidCartItem
type correctly extendsPlasmidItem
withPurchaseProperties
. This is a clean way to add purchasing-related properties to plasmid items in the cart.
17-17
: TheCatalogCartItem
type is a good example of extending a union type (CatalogItem
) with additional properties (PurchaseProperties
). This design supports a flexible and scalable cart system.
21-21
: It's great to see theplasmidItems
array included in theCart
type, reflecting the new functionality related to plasmids. This change ensures that the cart can handle both strain and plasmid items.packages/ui-dsc/src/__tests__/CatalogTableDisplay.test.tsx (2)
9-9
: Renaming theRows
component toCatalogRows
and updating its usage is a good practice for clarity and specificity. This change makes it easier to understand the component's purpose.
98-109
: The new test case for rendering table rows for plasmids is well-implemented. It correctly uses themockPlasmids
data and checks that the number of rendered rows matches the length of themockPlasmids
array. This ensures that theCatalogRows
component can handle plasmid data correctly.packages/ui-dsc/src/mocks/mockPlasmids.ts (1)
3-87
: The mock plasmid data is well-structured and provides a comprehensive set of information for each plasmid. This will be valuable for testing the new plasmid-related functionality. However, consider adding a brief comment at the beginning of the array to describe its purpose and how it's used within the application.
nextCursor={4} | ||
targetReference={{} as RefObject<HTMLTableRowElement>} | ||
/> | ||
</MemoryRouter>, | ||
) | ||
expect(screen.getAllByRole("row")).toHaveLength(testListStrains.length + 1) | ||
}) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an unnecessary space at the end of the file. Removing trailing spaces can help maintain code cleanliness.
-})
+})
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
}) | |
}) |
{ | ||
id: "DBP0001090", | ||
name: "pCpnA-GFP", | ||
summary: "parent plasmid: pTX-GFP vector (11.2 kb), cpnA cDNA (1.8kb); cpnA cDNA is cloned into the KpnI site of pTX-GFP; the KpnI site of the pTX-GFP plasmid for expression of copines with a GFP tag at the C-terminus.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some formatting inconsistencies in the mock data, such as inconsistent indentation and line breaks. Applying a consistent code formatting tool like Prettier can help maintain code quality and readability.
- summary: "parent plasmid: pTX-GFP vector (11.2 kb), cpnA cDNA (1.8kb); cpnA cDNA is cloned into the KpnI site of pTX-GFP; the KpnI site of the pTX-GFP plasmid for expression of copines with a GFP tag at the C-terminus.",
+ summary: `
+ parent plasmid: pTX-GFP vector (11.2 kb), cpnA cDNA (1.8kb); cpnA cDNA is cloned into the KpnI site of pTX-GFP;
+ the KpnI site of the pTX-GFP plasmid for expression of copines with a GFP tag at the C-terminus.
+ `,
Consider applying similar changes to other entries for consistency.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
summary: "parent plasmid: pTX-GFP vector (11.2 kb), cpnA cDNA (1.8kb); cpnA cDNA is cloned into the KpnI site of pTX-GFP; the KpnI site of the pTX-GFP plasmid for expression of copines with a GFP tag at the C-terminus.", | |
summary: ` | |
parent plasmid: pTX-GFP vector (11.2 kb), cpnA cDNA (1.8kb); cpnA cDNA is cloned into the KpnI site of pTX-GFP; | |
the KpnI site of the pTX-GFP plasmid for expression of copines with a GFP tag at the C-terminus. | |
`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- packages/ui-dsc/src/tests/CatalogTableDisplay.test.tsx (3 hunks)
- packages/ui-dsc/src/mocks/mockPlasmids.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/ui-dsc/src/mocks/mockPlasmids.ts
Additional Context Used
GitHub Check Runs (1)
ESLint Report Analysis failure (1)
packages/ui-dsc/src/tests/CatalogTableDisplay.test.tsx: [failure] 158-159: packages/ui-dsc/src/tests/CatalogTableDisplay.test.tsx#L158-L159
[prettier/prettier] Delete⏎
Additional comments not posted (2)
packages/ui-dsc/src/__tests__/CatalogTableDisplay.test.tsx (2)
9-9
: The renaming of theRows
component toCatalogRows
aligns with the PR objectives to enhance the plasmid display functionality. This change improves clarity and specificity in the component naming.
98-109
: The addition of a new test case to verify the correct rendering of table rows for a given plasmid array is a valuable enhancement. It ensures that theCatalogRows
component behaves as expected with the new plasmid data structure. Consider adding assertions for the content of the rendered rows to ensure that the plasmid data is displayed correctly, not just the row count.
@@ -140,3 +155,4 @@ | |||
expect(screen.getByRole("cell", { name: "Strain ID" })).toBeInTheDocument() | |||
}) | |||
}) | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The static analysis tool flagged an unnecessary newline at the end of the file. Removing this newline will adhere to code formatting standards and maintain cleanliness.
-
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
CatalogTableDisplay.test.tsx and mockPlasmids had unnecessary whitespace characters that did not conform to the coding standards of the project. This commit removes those whitespaces.
250dbd2
to
aaf7e8c
Compare
Tests for PlasmidCatalogTableDisplay have been added to the test file. The tests render the PlasmidCatalogTableDisplay component with mock plasmid data and checks that the corresponding rows have been rendered with the plasmid name.
347ff8b
to
8babf45
Compare
Summary by CodeRabbit
PlasmidCatalog
component for displaying plasmid data with lazy loading support.Rows
component toCatalogRows
and updated its properties for better clarity in table displays.Plasmid
related types for enhanced catalog and cart item management.