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

feat(vehicle-mileage): bulk mileage list search'n'filter #16906

Merged
merged 11 commits into from
Nov 22, 2024

Conversation

thorkellmani
Copy link
Member

@thorkellmani thorkellmani commented Nov 15, 2024

What

  • Add filter and search to vehicles
  • Some model changes

Why

  • Long lists need search

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Formatting passes locally with my changes
  • I have rebased against main before asking for a review

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced a new optional field for filtering vehicles based on required mileage registration.
    • Added a new endpoint /VehicleHistoryV2 for enhanced vehicle history retrieval.
    • Updated schemas to include additional properties related to mileage registration.
    • Enhanced GraphQL queries to provide more detailed vehicle mileage data.
    • Improved user interaction and data management in the VehicleBulkMileage component with advanced filtering capabilities.
  • Bug Fixes

    • Streamlined rendering logic for vehicle mileage registration details.
  • Documentation

    • Updated client configuration to reflect new features and endpoint changes.
  • Chores

    • Removed unnecessary checks in component logic for improved performance.

@thorkellmani thorkellmani added the deploy-feature Deploys features to dev label Nov 15, 2024
Copy link
Contributor

coderabbitai bot commented Nov 15, 2024

Caution

Review failed

The pull request is closed.

Walkthrough

The pull request introduces several modifications across various files related to vehicle mileage registration and handling. Key changes include the addition of new optional fields to data models, updates to service methods for processing vehicle lists and mileage history, and enhancements to GraphQL queries. The client configuration has been updated to include a new endpoint, and the user interface components have been refined to improve functionality and user experience. Additionally, some fields have been removed or modified to streamline data structures.

Changes

File Path Change Summary
libs/api/domains/vehicles/src/lib/dto/vehiclesListInputV3.ts Added optional field filterOnlyRequiredMileageRegistration?: boolean in VehiclesListInputV3.
libs/api/domains/vehicles/src/lib/models/v3/mileageDetails.model.ts Added optional field lastMileageRegistration?: MileageRegistration in MileageDetails.
libs/api/domains/vehicles/src/lib/models/v3/mileageRegistration.model.ts Added optional field internalId?: number in MileageRegistration.
libs/api/domains/vehicles/src/lib/models/v3/mileageRegistrationHistory.model.ts Removed optional field lastMileageRegistration?: MileageRegistration in MileageRegistrationHistory.
libs/api/domains/vehicles/src/lib/services/vehicles.service.ts Updated getVehiclesListV3 and getVehicleMileageHistory methods to accommodate new fields and logic.
libs/clients/vehicles/src/clientConfig.json Updated description field, added new endpoint /VehicleHistoryV2, and modified schemas.
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.graphql Added lastMileageRegistration in vehiclesList query, added internalId in mileageRegistration fragment, removed lastMileageRegistration from vehicleMileageRegistrationHistory.
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.tsx Enhanced component with new state variables and changed query hook to useVehiclesListLazyQuery.
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageRow.tsx Streamlined rendering logic by removing checks for lastMileageRegistration.
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageTable.tsx Uncommented line to display lastRegistration in the table header.
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/types.ts Removed lastMileageRegistration?: Date from VehicleProps, added new structured lastMileageRegistration in VehicleType.
libs/portals/my-pages/assets/src/screens/VehicleBulkMileageJobDetail/VehicleBulkMileageJobDetail.tsx Simplified rendering logic for action buttons.
libs/portals/my-pages/assets/src/screens/VehicleBulkMileageUpload/VehicleBulkMileageUpload.tsx Modified buttonGroup structure without changing functionality.
libs/portals/my-pages/core/src/components/IntroWrapper/IntroWrapper.tsx Updated buttonGroup type from React.ReactNode to Array<React.ReactNode>.
libs/portals/my-pages/occupational-licenses/src/screens/v2/OccupationalLicensesDetail/OccupationalLicensesDetail.tsx Optimized rendering logic for action buttons based on res?.actions.

Possibly related PRs

Warning

Rate limit exceeded

@thorkellmani has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 6 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 921fb49 and 29ce36f.


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 35.86%. Comparing base (6b0e779) to head (29ce36f).
Report is 4 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #16906      +/-   ##
==========================================
+ Coverage   35.62%   35.86%   +0.23%     
==========================================
  Files        6914     6888      -26     
  Lines      145990   145278     -712     
  Branches    41446    41266     -180     
==========================================
+ Hits        52011    52101      +90     
+ Misses      93979    93177     -802     
Flag Coverage Δ
api 3.34% <ø> (ø)
application-system-api 38.72% <ø> (+<0.01%) ⬆️
application-template-api-modules 27.73% <ø> (+0.02%) ⬆️
application-templates-car-recycling 3.12% <ø> (ø)
application-ui-shell 20.57% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 136 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57ca74a...29ce36f. Read the comment docs.

---- 🚨 Try these New Features:

@datadog-island-is
Copy link

datadog-island-is bot commented Nov 15, 2024

Datadog Report

All test runs e37264a 🔗

5 Total Test Services: 0 Failed, 4 Passed
🔻 Test Sessions change in coverage: 1 decreased (-0.02%), 34 no change

Test Services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Code Coverage Change Test Service View
api 0 0 0 4 0 2.39s 1 no change Link
application-system-api 0 0 0 34 0 2m 17.69s 1 no change Link
application-template-api-modules 0 0 0 116 0 2m 4.01s 1 decreased (-0.02%) Link
application-ui-shell 0 0 0 74 0 31.86s 1 no change Link
download-service 0 0 0 0 0 501.55ms 1 no change Link

🔻 Code Coverage Decreases vs Default Branch (1)

  • application-template-api-modules - jest 30.11% (-0.02%) - Details

@thorkellmani thorkellmani marked this pull request as ready for review November 22, 2024 08:58
@thorkellmani thorkellmani requested review from a team as code owners November 22, 2024 08:58
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Outside diff range and nitpick comments (16)
libs/api/domains/vehicles/src/lib/dto/vehiclesListInputV3.ts (1)

11-13: Consider adding JSDoc documentation for the new field.

Adding documentation would improve maintainability and provide better context for API consumers.

+  /**
+   * When true, filters the vehicle list to show only vehicles
+   * that require mileage registration.
+   */
   @Field({ nullable: true })
   filterOnlyRequiredMileageRegistration?: boolean
libs/api/domains/vehicles/src/lib/models/v3/mileageRegistration.model.ts (1)

14-15: Consider adding JSDoc documentation for the new field.

Adding documentation would help other developers understand the purpose and usage of the internalId field in the context of bulk mileage operations.

+  /** Internal identifier used for bulk mileage operations */
   @Field(() => Int, { nullable: true })
   internalId?: number
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/types.ts (1)

Line range hint 15-26: Consider extracting a shared type for mileage registration data.

The lastMileageRegistration and items in registrationHistory share a similar structure. Consider extracting a common type to improve maintainability and reduce duplication.

+export interface MileageRegistrationData {
+  date: Date
+  origin: string
+  mileage: number
+  internalId?: number
+}
+
 export interface VehicleType extends VehicleProps {
   mileageUploadedFromFile?: number
   isCurrentlyEditing?: boolean
-  lastMileageRegistration?: {
-    date: Date
-    origin: string
-    mileage: number
-    internalId?: number
-  }
-  registrationHistory?: Array<{
-    date: Date
-    origin: string
-    mileage: number
-  }>
+  lastMileageRegistration?: MileageRegistrationData
+  registrationHistory?: Array<Omit<MileageRegistrationData, 'internalId'>>
 }
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageTable.tsx (1)

Line range hint 42-48: Consider improving error handling and accessibility.

A few suggestions to enhance the component:

  1. Add null check before accessing rows.length
  2. Add proper ARIA labels for the table
  3. Consider adding purpose for the form element or remove if not needed

Consider applying these changes:

- {(!rows.length || loading) && (
+ {(!rows?.length || loading) && (
   <EmptyTable
     loading={loading}
     message={formatMessage(vehicleMessage.noVehiclesFound)}
   />
 )}

- <T.Table>
+ <T.Table aria-label={formatMessage(vehicleMessage.vehicleListTableLabel)}>
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.graphql (1)

74-74: Well-structured fragment enhancement

The addition of internalId to the mileageRegistration fragment is consistent with its usage in mutations and ensures proper record tracking across operations. Good use of fragment reusability across queries.

Consider documenting the format and significance of internalId in comments to help other developers understand its purpose in tracking mileage records.

libs/portals/my-pages/core/src/components/IntroWrapper/IntroWrapper.tsx (1)

84-86: Consider extracting button group wrapper to a separate component.

While the implementation is correct, consider extracting the button group wrapper into a reusable component if this pattern is used across different pages. This would improve maintainability and ensure consistent spacing across the application.

+ // In a new file ButtonGroupWrapper.tsx
+ interface ButtonGroupWrapperProps {
+   children: Array<React.ReactNode>
+ }
+ 
+ export const ButtonGroupWrapper = ({ children }: ButtonGroupWrapperProps) => (
+   <Box marginTop={4}>
+     <Inline space={2}>{children}</Inline>
+   </Box>
+ )

- <Box marginTop={4}>
-   <Inline space={2}>{props.buttonGroup}</Inline>
- </Box>
+ <ButtonGroupWrapper>{props.buttonGroup}</ButtonGroupWrapper>
libs/portals/my-pages/occupational-licenses/src/screens/v2/OccupationalLicensesDetail/OccupationalLicensesDetail.tsx (3)

70-86: Extract FILE action button into a reusable component

The FILE action button logic can be extracted into a separate component to improve reusability and maintainability. This aligns with the coding guidelines for libs/ directory.

interface FileActionButtonProps {
  url?: string;
  text: string;
  index: number;
}

const FileActionButton: React.FC<FileActionButtonProps> = ({ url, text, index }) => (
  <Button
    key={`button-file-${index}`}
    variant="utility"
    iconType="outline"
    onClick={() => url && formSubmit(url)}
    icon="download"
  >
    {text}
  </Button>
);

92-98: Extract path construction logic

The document path construction logic should be moved to a utility function for better maintainability and reuse.

const getDocumentPath = (type: OccupationalLicensesV2LinkType, url?: string) => {
  if (type === OccupationalLicensesV2LinkType.DOCUMENT && url) {
    return DocumentsPaths.ElectronicDocumentSingle.replace(':id', url);
  }
  return url;
};

101-105: Use a constant for icon mapping

Extract the icon mapping logic into a constant to improve maintainability and reduce the complexity of the JSX.

const ACTION_TYPE_ICON_MAP: Record<OccupationalLicensesV2LinkType, IconType> = {
  [OccupationalLicensesV2LinkType.DOCUMENT]: 'mailOpen',
  [OccupationalLicensesV2LinkType.FILE]: 'download',
  [OccupationalLicensesV2LinkType.LINK]: 'open',
};
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.tsx (3)

Line range hint 32-34: Consider using a more specific type for FormData

The current interface uses a string index signature which is too permissive. Consider defining specific keys if they are known at compile time for better type safety.

-interface FormData {
-  [key: string]: number
-}
+interface FormData {
+  mileage: Record<string, number> // if mileage is stored by vehicle ID
+}

80-97: Simplify optional chaining for better readability

The current nested optional chaining can be simplified while maintaining type safety.

-const lastMileageRegistration =
-  v.mileageDetails?.lastMileageRegistration &&
-  v.mileageDetails.lastMileageRegistration.date &&
-  v.mileageDetails.lastMileageRegistration.mileage &&
-  v.mileageDetails.lastMileageRegistration.originCode
-    ? {
+const lastMileageRegistration = 
+  v.mileageDetails?.lastMileageRegistration?.date &&
+  v.mileageDetails?.lastMileageRegistration?.mileage &&
+  v.mileageDetails?.lastMileageRegistration?.originCode
+    ? {
🧰 Tools
🪛 Biome (1.9.4)

[error] 81-82: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


168-170: Remove console.log statement

Debug statement should be removed before production.

  onFilterClear={() => {
-   console.log('clear')
+   setSearch('')
+   setFilterValue(false)
  }}
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageRow.tsx (3)

244-248: LGTM! Consider extracting the date formatting logic.

The implementation correctly handles undefined values and follows TypeScript best practices. However, for better reusability, consider extracting the date formatting logic into a custom hook or utility function since it's used in multiple places.

+ const formatMileageDate = (date?: string) => date ? formatDate(date) : '-'
  {
    value: formatMileageDate(vehicle.lastMileageRegistration?.date)
  },

Line range hint 1-450: Consider breaking down this component for better maintainability and reusability.

The component has grown quite large and handles multiple responsibilities. Consider:

  1. Extracting the validation logic into a custom hook (e.g., useVehicleMileageValidation)
  2. Moving mutation handling into a separate service
  3. Creating smaller sub-components for the form and history table

This would improve reusability across different NextJS apps and make the code more maintainable.


Line range hint 1-20: Optimize imports for better tree-shaking.

Consider lazy loading components that are conditionally rendered, such as AlertMessage and NestedFullTable. This would improve initial bundle size and align with effective tree-shaking practices.

const AlertMessage = lazy(() => import('@island.is/island-ui/core').then(module => ({ default: module.AlertMessage })));
const NestedFullTable = lazy(() => import('@island.is/portals/my-pages/core').then(module => ({ default: module.NestedFullTable })));
libs/api/domains/vehicles/src/lib/services/vehicles.service.ts (1)

146-159: Consider extracting lastMileageRegistration mapping to a helper function

The lastMileageRegistration object construction logic could be moved to a separate helper function to improve readability and reusability.

+private mapLastMileageRegistration(
+  latestOriginCode?: string,
+  latestMileage?: number,
+  latestMileageReadDate?: Date,
+  latestMileageInternalId?: string
+): MileageRegistration | undefined {
+  if (!latestOriginCode || !latestMileage || !latestMileageReadDate) {
+    return undefined;
+  }
+  return {
+    originCode: latestOriginCode,
+    mileage: latestMileage,
+    date: latestMileageReadDate,
+    internalId: latestMileageInternalId ?? undefined,
+  };
+}

 let lastMileageRegistration: MileageRegistration | undefined
-if (
-  d.latestOriginCode &&
-  d.latestMileage &&
-  d.latestMileageReadDate
-) {
-  lastMileageRegistration = {
-    originCode: d.latestOriginCode,
-    mileage: d.latestMileage,
-    date: d.latestMileageReadDate,
-    internalId: d.latestMileageInternalId ?? undefined,
-  }
-}
+lastMileageRegistration = this.mapLastMileageRegistration(
+  d.latestOriginCode,
+  d.latestMileage,
+  d.latestMileageReadDate,
+  d.latestMileageInternalId
+);

Also applies to: 167-167

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 57ca74a and 921fb49.

📒 Files selected for processing (15)
  • libs/api/domains/vehicles/src/lib/dto/vehiclesListInputV3.ts (1 hunks)
  • libs/api/domains/vehicles/src/lib/models/v3/mileageDetails.model.ts (2 hunks)
  • libs/api/domains/vehicles/src/lib/models/v3/mileageRegistration.model.ts (1 hunks)
  • libs/api/domains/vehicles/src/lib/models/v3/mileageRegistrationHistory.model.ts (0 hunks)
  • libs/api/domains/vehicles/src/lib/services/vehicles.service.ts (4 hunks)
  • libs/clients/vehicles/src/clientConfig.json (5 hunks)
  • libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.graphql (2 hunks)
  • libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.tsx (3 hunks)
  • libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageRow.tsx (1 hunks)
  • libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageTable.tsx (1 hunks)
  • libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/types.ts (1 hunks)
  • libs/portals/my-pages/assets/src/screens/VehicleBulkMileageJobDetail/VehicleBulkMileageJobDetail.tsx (1 hunks)
  • libs/portals/my-pages/assets/src/screens/VehicleBulkMileageUpload/VehicleBulkMileageUpload.tsx (1 hunks)
  • libs/portals/my-pages/core/src/components/IntroWrapper/IntroWrapper.tsx (3 hunks)
  • libs/portals/my-pages/occupational-licenses/src/screens/v2/OccupationalLicensesDetail/OccupationalLicensesDetail.tsx (1 hunks)
💤 Files with no reviewable changes (1)
  • libs/api/domains/vehicles/src/lib/models/v3/mileageRegistrationHistory.model.ts
🧰 Additional context used
📓 Path-based instructions (14)
libs/api/domains/vehicles/src/lib/dto/vehiclesListInputV3.ts (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/api/domains/vehicles/src/lib/models/v3/mileageDetails.model.ts (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/api/domains/vehicles/src/lib/models/v3/mileageRegistration.model.ts (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/api/domains/vehicles/src/lib/services/vehicles.service.ts (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/clients/vehicles/src/clientConfig.json (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.graphql (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.tsx (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageRow.tsx (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageTable.tsx (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/types.ts (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/portals/my-pages/assets/src/screens/VehicleBulkMileageJobDetail/VehicleBulkMileageJobDetail.tsx (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/portals/my-pages/assets/src/screens/VehicleBulkMileageUpload/VehicleBulkMileageUpload.tsx (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/portals/my-pages/core/src/components/IntroWrapper/IntroWrapper.tsx (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/portals/my-pages/occupational-licenses/src/screens/v2/OccupationalLicensesDetail/OccupationalLicensesDetail.tsx (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
🪛 Biome (1.9.4)
libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.tsx

[error] 81-82: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


[error] 147-152: Missing key property for this element in iterable.

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

(lint/correctness/useJsxKeyInIterable)


[error] 153-158: Missing key property for this element in iterable.

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

(lint/correctness/useJsxKeyInIterable)

libs/portals/my-pages/assets/src/screens/VehicleBulkMileageJobDetail/VehicleBulkMileageJobDetail.tsx

[error] 119-129: Missing key property for this element in iterable.

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

(lint/correctness/useJsxKeyInIterable)

libs/portals/my-pages/assets/src/screens/VehicleBulkMileageUpload/VehicleBulkMileageUpload.tsx

[error] 130-130: Missing key property for this element in iterable.

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

(lint/correctness/useJsxKeyInIterable)

🔇 Additional comments (18)
libs/api/domains/vehicles/src/lib/dto/vehiclesListInputV3.ts (1)

11-13: LGTM! Well-structured GraphQL input field implementation.

The implementation follows TypeScript and NestJS best practices with proper type definitions and GraphQL decorators. The optional field is correctly marked as nullable, which maintains API flexibility.

libs/api/domains/vehicles/src/lib/models/v3/mileageRegistration.model.ts (1)

14-15: LGTM! Type definitions are properly implemented.

The new internalId field is correctly typed with proper GraphQL and TypeScript decorators. The nullable flag ensures backward compatibility.

libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/types.ts (1)

15-20: LGTM! The type definitions are well-structured.

The changes follow TypeScript best practices with proper type definitions and exports, allowing for effective tree-shaking and reuse across different NextJS apps.

libs/api/domains/vehicles/src/lib/models/v3/mileageDetails.model.ts (2)

3-3: LGTM! Clean import statement

The import is properly defined and follows TypeScript best practices.


13-14: LGTM! Well-structured field addition

The new lastMileageRegistration field is well-defined with proper TypeScript typing and NestJS decorators. The nullable property ensures backward compatibility, and the field name clearly describes its purpose in the context of bulk mileage list functionality.

libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageTable.tsx (2)

36-36: LGTM! Column addition aligns with data model changes.

The addition of the last registration column header aligns with the changes in the data model and GraphQL schema mentioned in the summary.


Line range hint 1-56: Verify component reusability across NextJS apps.

The component follows good practices for reusability but let's verify its usage across different apps in the monorepo.

✅ Verification successful

Let me try another search strategy to better understand the component's usage and reusability.


Component is correctly scoped to its specific feature

The VehicleBulkMileageTable component is appropriately placed within the VehicleBulkMileage feature screen and is only used by its parent component (VehicleBulkMileage.tsx). The component follows good practices by:

  • Using shared UI components from island-ui/core
  • Utilizing common portal components from my-pages/core
  • Being properly scoped to its specific vehicle bulk mileage feature

No reusability concerns were found as this is a feature-specific component that shouldn't be shared across different apps.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for imports of VehicleBulkMileageTable across the codebase
rg -l "VehicleBulkMileageTable" --type ts --type tsx

# Check for any duplicate implementations
ast-grep --pattern 'const VehicleBulkMileageTable = $_'

Length of output: 4919


Script:

#!/bin/bash
# Search for imports or references to this component
rg "import.*VehicleBulkMileageTable|from.*VehicleBulkMileageTable" --type typescript

# Look for the screen/page where this component is used
fd -e tsx -e ts | xargs rg "<VehicleBulkMileageTable"

# Check the structure of the my-pages portal to understand component organization
fd . libs/portals/my-pages/assets/src/screens

Length of output: 5939

libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.graphql (1)

14-16: LGTM! Efficient addition of latest mileage information

The addition of lastMileageRegistration to the vehiclesList query is a clean way to fetch the most recent mileage alongside other vehicle details, reducing the need for separate queries.

libs/portals/my-pages/core/src/components/IntroWrapper/IntroWrapper.tsx (1)

10-10: LGTM! Well-typed changes for button group handling.

The changes properly define the type for multiple buttons and add the necessary UI component for layout. The exported types maintain good TypeScript practices and component reusability.

Also applies to: 28-28

libs/portals/my-pages/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.tsx (1)

Line range hint 1-236: Implementation aligns with coding guidelines

The component successfully meets the requirements for libs/**/* files:

  • ✓ Reusable across NextJS apps
  • ✓ Proper TypeScript usage
  • ✓ Effective tree-shaking through named imports
🧰 Tools
🪛 Biome (1.9.4)

[error] 81-82: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

libs/portals/my-pages/assets/src/screens/VehicleBulkMileageJobDetail/VehicleBulkMileageJobDetail.tsx (1)

Line range hint 1-269: LGTM! Component follows coding guidelines.

The component demonstrates good practices:

  • Proper TypeScript usage with well-defined types
  • Utilizes shared UI components from the design system
  • Implements proper error handling and loading states
  • Follows React best practices for component structure
🧰 Tools
🪛 Biome (1.9.4)

[error] 119-129: Missing key property for this element in iterable.

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

(lint/correctness/useJsxKeyInIterable)

libs/api/domains/vehicles/src/lib/services/vehicles.service.ts (3)

49-49: LGTM! Clean import addition

The MileageRegistration model import enhances type safety for mileage-related operations.


115-115: LGTM! Clean filter implementation

The onlyMileageRequiredVehicles filter is correctly mapped from the input parameter.


Line range hint 405-419: LGTM! Robust null checking and filtering

The implementation properly validates the permno consistency and required fields before mapping the history items.

libs/clients/vehicles/src/clientConfig.json (4)

506-595: LGTM: Well-structured endpoint definition

The new /VehicleHistoryV2 endpoint is properly defined with:

  • Appropriate query parameters for filtering
  • Clear response structure
  • Proper security configurations
  • Standard error responses

1197-1380: LGTM: Comprehensive schema definition

The AllVehiclesForPersidnoV2 schema is well-defined with:

  • Clear property types and nullability
  • Comprehensive vehicle information coverage
  • Proper OpenAPI format compliance

3046-3063: LGTM: Well-structured mileage tracking fields

The new mileage tracking fields in CurrentVehiclesWithMilageAndNextInspDto are properly defined with appropriate types and nullability.


3667-3670: LGTM: Re-added engine field

The engine field has been properly re-added to the VehicleSearchDto schema.

Copy link
Member

@disaerna disaerna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with one comment about Introwrapper 💬 💯

@thorkellmani thorkellmani added the deprecated:automerge (Disabled) Merge this PR as soon as all checks pass label Nov 22, 2024
@kodiakhq kodiakhq bot merged commit 421f147 into main Nov 22, 2024
75 of 76 checks passed
@kodiakhq kodiakhq bot deleted the feat/bulk-mileage-search branch November 22, 2024 09:29
jonnigs pushed a commit that referenced this pull request Nov 26, 2024
* feat: add search

* fix: search works

* feat: add filter

* feat: add mileage last

* fix: add keys

* fix: add key again
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy-feature Deploys features to dev deprecated:automerge (Disabled) Merge this PR as soon as all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants