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

Transaction Log Implementation #640

Merged
merged 40 commits into from
Aug 15, 2024

Conversation

ItsMurumba
Copy link

@ItsMurumba ItsMurumba commented Jul 4, 2024

This PR handle the basic setup of the Trasnaction Log Components. The tickets handled in this PR are:

Summary by CodeRabbit

  • New Features

    • Enhanced the documentation with a "Tech Used" section and a note on local development setup.
    • Introduced a transaction log microfrontend with improved API functionality for transactions and channels.
    • Added new routing for the transaction log and sidebar applications.
    • Implemented new components for filtering and displaying transaction logs.
  • Bug Fixes

    • Updated navigation links to ensure proper routing to the new transaction log.
  • Chores

    • Added configuration files for ESLint, Babel, Prettier, Jest, and TypeScript to improve code quality and maintainability.
    • Introduced .gitignore and .prettierignore files to streamline version control and formatting processes.

Copy link

coderabbitai bot commented Jul 4, 2024

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This update significantly enhances the OpenHIM Console by expanding API functionality, improving documentation, and introducing new components for transaction management. Key additions include a transaction log interface, improved routing for microfrontends, and robust configuration files for TypeScript and Babel. These changes aim to streamline development workflows and enhance user experience, making it easier for developers to manage transactions while also providing clear guidance in the documentation.

Changes

Files Change Summary
README.md Added "Tech Used" section, emphasized environment configuration, and included development access info.
packages/openhim-core-api/src/jembi-openhim-core-api.ts Introduced Transactions interface, new functions for fetching transactions and channels, and modified existing functions.
packages/root-config/src/index.ejs Added module import for @jembi/transaction-log for enhanced functionality.
packages/root-config/src/microfrontend-layout.html Added new routes for #!/transactionsxyz to integrate sidebar and transaction log applications.
packages/sidebar-app/src/menu.component.tsx Updated navigation links for transaction logs.
packages/transaction-log/.eslintrc Introduced ESLint configuration tailored for TypeScript and React.
packages/transaction-log/.gitignore Created .gitignore to exclude unnecessary files from version control.
packages/transaction-log/.prettierignore Introduced .prettierignore to specify files to be excluded from Prettier formatting.
packages/transaction-log/babel.config.json Established Babel configuration for JSX and TypeScript.
packages/transaction-log/jest.config.js Configured Jest for testing environment with support for React and TypeScript.
packages/transaction-log/package.json Set up scripts for development, testing, and added key dependencies.
packages/transaction-log/src/app.css Added styles for the application body.
packages/transaction-log/src/components/App.tsx Created main component for managing transaction logs with filtering capabilities.
packages/transaction-log/src/components/BasicFilters.tsx Implemented filtering UI for transaction logs with various criteria.
packages/transaction-log/src/components/CustomFilters.tsx Developed additional filtering options for transactions.
packages/transaction-log/src/components/TransactionLogTable.tsx Created table component to display transactions dynamically.
packages/transaction-log/src/declarations.d.ts Added TypeScript declarations for image imports and external modules.
packages/transaction-log/src/jembi-transaction-log.tsx Integrated transaction log as a microfrontend with lifecycle management.
packages/transaction-log/src/root.component.tsx Established root component for transaction log features.
packages/transaction-log/tsconfig.json Introduced TypeScript configuration tailored for SPA development.
packages/transaction-log/webpack.config.js Configured Webpack for project build process.

Poem

🐇 In the garden where code does bloom,
New features leap and old bugs zoom.
With logs and filters, oh what a sight,
OpenHIM's journey takes flight!
So hop along, let’s build and grow,
In this digital meadow, our skills will show!
🌼


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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

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.

@ItsMurumba ItsMurumba marked this pull request as ready for review July 19, 2024 05:04
@ItsMurumba ItsMurumba changed the title CU_86bz9np6a: Create Transaction log components Datatable, Basic and Custom Filter Tabs Implementation Jul 29, 2024
Copy link
Collaborator

@marvinIsSacul marvinIsSacul left a comment

Choose a reason for hiding this comment

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

Looks good overall.

Just a couple of changes requested and the CI pipeline is failing.

const searchTerm = searchQuery.toLowerCase()
return (
transaction.channelName?.toLowerCase().includes(searchTerm) ||
'' ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

What are the || '' for?

packages/transaction-log/src/components/CustomFilters.tsx Outdated Show resolved Hide resolved

setTransactions(transactionsWithChannelDetails)
} catch (error) {
console.error('Error fetching logs:', error)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might need to handle errors and display them to user (dialogs, modals, etc.)

const channels = await fetchChannels()
setChannels(channels)
} catch (error) {
console.error('Error fetching channels:', error)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better error handling.

const channel = await response.json()
return channel.name
} catch (error) {
console.error('Error fetching logs:', error)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better error handling

const client = await response.json()
return client.name
} catch (error) {
console.error('Error fetching logs:', error)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better error handling.

"@types/react-dom": "^17.0.9",
"@types/systemjs": "^6.1.1",
"@types/webpack-env": "^1.16.2",
"axios": "^1.7.2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need axios?

Suggested change
"axios": "^1.7.2",

packages/transaction-log/src/components/App.tsx Outdated Show resolved Hide resolved
@drizzentic
Copy link
Collaborator

@coderabbitai review

Copy link

coderabbitai bot commented Aug 2, 2024

Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@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: 11

Outside diff range, codebase verification and nitpick comments (2)
README.md (2)

21-24: Consider changing unordered list style for consistency.

The static analysis tool suggests using asterisks instead of dashes for unordered lists. Ensure consistency throughout the document.

- - Node: v16.20.2
- - Lerna (microfrontend)
- - Docker
+ * Node: v16.20.2
+ * Lerna (microfrontend)
+ * Docker
Tools
Markdownlint

22-22: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


23-23: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


24-24: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


231-237: Improve the use of bare URL.

Consider using Markdown link syntax for the URL to improve readability.

- URL : http://localhost:9000/
+ URL : [http://localhost:9000/](http://localhost:9000/)
Tools
Markdownlint

234-234: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


235-235: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


236-236: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


234-234: null
Bare URL used

(MD034, no-bare-urls)


235-235: null
Bare URL used

(MD034, no-bare-urls)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e4e5048 and 1534f40.

Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
Files selected for processing (22)
  • README.md (3 hunks)
  • packages/openhim-core-api/src/jembi-openhim-core-api.ts (2 hunks)
  • packages/root-config/src/index.ejs (1 hunks)
  • packages/root-config/src/microfrontend-layout.html (2 hunks)
  • packages/sidebar-app/src/menu.component.tsx (1 hunks)
  • packages/transaction-log/.eslintrc (1 hunks)
  • packages/transaction-log/.gitignore (1 hunks)
  • packages/transaction-log/.prettierignore (1 hunks)
  • packages/transaction-log/babel.config.json (1 hunks)
  • packages/transaction-log/jest.config.js (1 hunks)
  • packages/transaction-log/package.json (1 hunks)
  • packages/transaction-log/src/app.css (1 hunks)
  • packages/transaction-log/src/components/App.tsx (1 hunks)
  • packages/transaction-log/src/components/BasicFilters.tsx (1 hunks)
  • packages/transaction-log/src/components/CustomFilters.tsx (1 hunks)
  • packages/transaction-log/src/components/TransactionLogTable.tsx (1 hunks)
  • packages/transaction-log/src/declarations.d.ts (1 hunks)
  • packages/transaction-log/src/jembi-transaction-log.tsx (1 hunks)
  • packages/transaction-log/src/root.component.test.tsx (1 hunks)
  • packages/transaction-log/src/root.component.tsx (1 hunks)
  • packages/transaction-log/tsconfig.json (1 hunks)
  • packages/transaction-log/webpack.config.js (1 hunks)
Files skipped from review due to trivial changes (6)
  • packages/transaction-log/.eslintrc
  • packages/transaction-log/.gitignore
  • packages/transaction-log/.prettierignore
  • packages/transaction-log/babel.config.json
  • packages/transaction-log/jest.config.js
  • packages/transaction-log/src/app.css
Additional context used
Biome
packages/openhim-core-api/src/jembi-openhim-core-api.ts

[error] 153-153: Don't use '{}' as a type.

Prefer explicitly define the object shape. '{}' means "any non-nullable value".

(lint/complexity/noBannedTypes)


[error] 171-171: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)


[error] 180-180: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)

packages/transaction-log/src/components/App.tsx

[error] 95-95: Don't use '{}' as a type.

Prefer explicitly define the object shape. '{}' means "any non-nullable value".

(lint/complexity/noBannedTypes)

Markdownlint
README.md

22-22: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


23-23: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


24-24: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


234-234: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


235-235: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


236-236: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


234-234: null
Bare URL used

(MD034, no-bare-urls)


235-235: null
Bare URL used

(MD034, no-bare-urls)

Additional comments not posted (40)
packages/transaction-log/tsconfig.json (3)

3-6: Review compiler options for correctness.

The compiler options jsx, declarationDir, and their values should be verified for correctness and relevance.

  • "jsx": "react-jsx" is appropriate for React projects using JSX.
  • "declarationDir": "dist" should be verified to ensure the directory structure matches your build process.

7-9: Review file inclusion and exclusion patterns.

The files, include, and exclude patterns should be verified to ensure they match the intended project structure and do not omit necessary files or include unnecessary ones.

  • "files": ["src/jembi-transaction-log.tsx"] ensures the entry point is included.
  • "include": ["src/**/*"] includes all source files.
  • "exclude": ["src/**/*.test*"] excludes test files, which is standard practice.

Ensure these patterns align with your project structure.


2-2: Ensure ts-config-single-spa is correctly set up.

The ts-config-single-spa extension should be verified to ensure it is correctly set up and compatible with your project.

Verification successful

Ensure ts-config-single-spa is correctly set up.

The ts-config-single-spa configuration is correctly set up and referenced in multiple files across the repository, confirming its proper usage.

  • packages/sidebar-app/package.json
  • packages/sidebar-app/tsconfig.json
  • packages/users-app/tsconfig.json
  • packages/root-config/tsconfig.json
  • packages/transaction-log/tsconfig.json
  • packages/transaction-log/package.json
  • packages/root-config/package.json
  • packages/users-app/package.json
  • packages/openhim-theme/tsconfig.json
  • packages/openhim-theme/package.json
  • packages/header-app/tsconfig.json
  • packages/header-app/package.json
  • packages/openhim-core-api/tsconfig.json
  • packages/openhim-core-api/package.json
  • packages/portal-app/tsconfig.json
  • packages/portal-app/package.json
  • packages/footer-app/tsconfig.json
  • packages/footer-app/package.json
  • packages/portal-admin/package.json
  • packages/dashboard-app/tsconfig.json
  • packages/dashboard-app/package.json
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence and correctness of `ts-config-single-spa`.

# Test: Check if the configuration file `ts-config-single-spa` exists and is correctly set up.
fd ts-config-single-spa --type f

Length of output: 34


Script:

#!/bin/bash
# Description: Verify the existence and correctness of `ts-config-single-spa`.

# Test: Check for any files related to `ts-config-single-spa` in the repository.
fd 'ts-config-single-spa*' --type f

# Check for any mentions of `ts-config-single-spa` in the repository to find references or documentation.
rg 'ts-config-single-spa'

Length of output: 5363

packages/transaction-log/src/root.component.test.tsx (2)

1-2: Ensure correct imports for testing.

The imports from @testing-library/react and the Root component should be verified to ensure they are correct and necessary for the test.

  • render from @testing-library/react is correctly imported for rendering the component.
  • Root from ./root.component should be verified to ensure it matches the component being tested.

4-9: Basic test setup looks good.

The test suite and test case are set up correctly to check if the Root component is rendered in the document.

  • The describe block correctly groups the test cases for the Root component.
  • The it block defines a test case to check if the component is in the document.
  • The render function is used correctly to render the component.
  • The expect assertion checks if the text "Testapp is mounted!" is in the document.

Consider adding more test cases to cover different scenarios and edge cases.

packages/transaction-log/src/root.component.tsx (2)

1-4: Ensure correct imports for the root component.

The imports for React, App, ThemeProvider, and theme should be verified to ensure they are correct and necessary for the component.

  • React is correctly imported for using JSX.
  • App from ./components/App should be verified to ensure it matches the main component being rendered.
  • ThemeProvider from @emotion/react is correctly imported for theming.
  • theme from @jembi/openhim-theme should be verified to ensure it matches the intended theme.

6-14: Component setup looks good.

The TransactionsLogRootApp component is set up correctly to use React.StrictMode and ThemeProvider for theming.

  • The component function TransactionsLogRootApp takes props as an argument.
  • React.StrictMode is used to wrap the component tree, which helps identify potential problems.
  • ThemeProvider is used to apply the theme to the component tree.
  • The App component is rendered within the ThemeProvider.

Consider using props in the component or removing it if it is not needed.

packages/transaction-log/webpack.config.js (4)

1-3: Good use of webpack-merge and single-spa-react-ts.

The imports are correctly set up to merge Webpack configurations and use Single SPA for React with TypeScript.


4-10: Initialize default Webpack configuration.

The module exports a function that initializes the default Webpack configuration using singleSpaDefaults.


12-14: Allow for custom Webpack configurations.

The merge function allows for additional custom Webpack configurations.


15-15: Good practice to return the merged configuration.

Returning the merged configuration ensures that custom configurations are applied on top of the default settings.

packages/transaction-log/src/jembi-transaction-log.tsx (3)

1-5: Correct imports and root component setup.

The imports and root component setup for the microfrontend are correctly done.


6-14: Proper lifecycle management with error boundary.

The lifecycles are correctly managed using singleSpaReact. The error boundary is customizable, which is a good practice for handling errors gracefully.


16-16: Export lifecycle functions.

The bootstrap, mount, and unmount functions are correctly exported for the microfrontend.

packages/transaction-log/src/declarations.d.ts (10)

1-4: Declare module for HTML files.

The declaration allows importing HTML files as raw strings.


6-9: Declare module for BMP files.

The declaration allows importing BMP files as strings.


11-14: Declare module for GIF files.

The declaration allows importing GIF files as strings.


16-19: Declare module for JPG files.

The declaration allows importing JPG files as strings.


21-24: Declare module for JPEG files.

The declaration allows importing JPEG files as strings.


26-29: Declare module for PNG files.

The declaration allows importing PNG files as strings.


31-34: Declare module for WEBP files.

The declaration allows importing WEBP files as strings.


36-39: Declare module for SVG files.

The declaration allows importing SVG files as strings.


41-42: Declare module for @jembi/openhim-core-api.

The declaration allows importing the @jembi/openhim-core-api module.


42-42: Declare module for @jembi/openhim-theme.

The declaration allows importing the @jembi/openhim-theme module.

packages/root-config/src/microfrontend-layout.html (2)

8-10: LGTM!

The new route #!/transactionsxyz includes an application named @jembi/openhim-sidebar and follows the existing pattern.


53-58: LGTM!

The new route #!/transactionsxyz includes an application named @jembi/openhim-sidebar and a nested application named @jembi/transaction-log. This follows the existing pattern.

packages/transaction-log/package.json (1)

17-67: Verify the necessity of axios and other dependencies.

The previous comment questioned the necessity of axios. Ensure that all dependencies listed are necessary and justified for the @jembi/transaction-log package.

Verification successful

Verified the necessity of axios and other dependencies.

The dependencies listed in the package.json file are necessary and justified for the @jembi/transaction-log package as they are used in the codebase.

  • axios is used in packages/openhim-core-api/src/jembi-openhim-core-api.ts.
  • @emotion/react is used in multiple files such as packages/portal-app/src/root.component.tsx and packages/users-app/src/root.component.tsx.
  • @mui/material and @mui/icons-material are used in various files like packages/transaction-log/src/components/TransactionLogTable.tsx and packages/users-app/src/screens/list.users.screen.tsx.
  • @mui/x-date-pickers is used in packages/transaction-log/src/components/CustomFilters.tsx.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of axios and other dependencies in the `@jembi/transaction-log` package.

# Test: Search for the usage of axios. Expect: Occurrences of axios usage.
rg --type js --type ts --type tsx 'axios'

# Test: Search for the usage of other dependencies. Expect: Occurrences of each dependency usage.
for dep in "@date-io/date-fns" "@emotion/react" "@emotion/styled" "@mui/icons-material" "@mui/material" "@mui/x-date-pickers" "@types/jest" "@types/react" "@types/react-dom" "@types/systemjs" "@types/webpack-env" "react" "react-dom" "single-spa" "single-spa-react"; do
  rg --type js --type ts --type tsx "$dep"
done

Length of output: 5258


Script:

#!/bin/bash
# Description: Verify the usage of axios and other dependencies in the `@jembi/transaction-log` package.

# Test: Search for the usage of axios. Expect: Occurrences of axios usage.
rg --type-add 'typescript:*.ts' --type-add 'typescript:*.tsx' --type js --type typescript 'axios'

# Test: Search for the usage of other dependencies. Expect: Occurrences of each dependency usage.
for dep in "@date-io/date-fns" "@emotion/react" "@emotion/styled" "@mui/icons-material" "@mui/material" "@mui/x-date-pickers" "@types/jest" "@types/react" "@types/react-dom" "@types/systemjs" "@types/webpack-env" "react" "react-dom" "single-spa" "single-spa-react"; do
  rg --type-add 'typescript:*.ts' --type-add 'typescript:*.tsx' --type js --type typescript "$dep"
done

Length of output: 38792

packages/transaction-log/src/components/CustomFilters.tsx (2)

1-5: Imports look good.

The imports are appropriate and necessary for the functionality provided in the component.


7-23: Interface definition looks good.

The FilterProps interface is well-defined and includes all necessary properties for the component to function correctly.

packages/sidebar-app/src/menu.component.tsx (1)

61-73: Clarify the reason for having two similar ListItem components.

The changes introduce a new ListItem for "Transaction Log old". It would be helpful to clarify the reason for having two similar ListItem components and whether this is for transitional purposes.

packages/root-config/src/index.ejs (1)

82-82: Addition of @jembi/transaction-log looks good.

The addition of @jembi/transaction-log in the import map is appropriate for integrating the transaction log application.

packages/transaction-log/src/components/BasicFilters.tsx (4)

1-5: LGTM!

The imports are appropriate and necessary for the component's functionality.


7-23: LGTM!

The FilterProps interface is well-defined and covers all necessary props for the component.


25-60: LGTM!

The BasicFilters component is well-defined, and the handlers are correctly implemented to manage state changes.


62-179: LGTM!

The return JSX is clear and well-organized, using Material-UI components for a consistent UI.

packages/openhim-core-api/src/jembi-openhim-core-api.ts (2)

Line range hint 37-73:
LGTM!

The initialization functions are well-implemented with proper error handling and interceptors.


24-36: Specify the type of the channel object in the Transactions interface.

The channel object should have a defined type for better type safety.

interface Transactions {
  _id: string
  method: string
  host: string
  port: string
  path: string
  querystring: string
  channel: {
    _id: string
    name: string
  }
  timestamp: Date
}

Likely invalid or redundant comment.

packages/transaction-log/src/components/App.tsx (2)

1-20: LGTM!

The imports are appropriate and necessary for the component's functionality.


151-235: LGTM!

The return JSX is clear and well-organized, using Material-UI components for a consistent UI.

README.md (1)

50-51: LGTM!

The note about modifying the environment value is clear and helpful.

setLimit: (value: number) => void
reruns: string
setReruns: (value: string) => void
channels: any[]
Copy link

Choose a reason for hiding this comment

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

Use a more specific type for channels prop.

The channels prop should have a more specific type instead of any[] to improve type safety.

-  channels: any[]
+  channels: { _id: string, name: string }[]
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
channels: any[]
channels: { _id: string, name: string }[]

Comment on lines 45 to 59
<TableCell>
<svg
width="32"
height="32"
viewBox="0 0 32 32"
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
<rect width="32" height="32" rx="4" fill="#FECDD2" />
<path
d="M22 12H21V10C21 7.24 18.76 5 16 5C13.24 5 11 7.24 11 10V12H10C8.9 12 8 12.9 8 14V24C8 25.1 8.9 26 10 26H22C23.1 26 24 25.1 24 24V14C24 12.9 23.1 12 22 12ZM16 21C14.9 21 14 20.1 14 19C14 17.9 14.9 17 16 17C17.1 17 18 17.9 18 19C18 20.1 17.1 21 16 21ZM19.1 12H12.9V10C12.9 8.29 14.29 6.9 16 6.9C17.71 6.9 19.1 8.29 19.1 10V12Z"
fill="#C62828"
/>
</svg>
</TableCell>
Copy link

Choose a reason for hiding this comment

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

Consider using a separate component or an icon library for the SVG icon.

The inline SVG icon can be moved to a separate component or replaced with an icon from a library like Material-UI Icons to improve code readability and maintainability.

-  <svg
-    width="32"
-    height="32"
-    viewBox="0 0 32 32"
-    fill="none"
-    xmlns="http://www.w3.org/2000/svg"
-  >
-    <rect width="32" height="32" rx="4" fill="#FECDD2" />
-    <path
-      d="M22 12H21V10C21 7.24 18.76 5 16 5C13.24 5 11 7.24 11 10V12H10C8.9 12 8 12.9 8 14V24C8 25.1 8.9 26 10 26H22C23.1 26 24 25.1 24 24V14C24 12.9 23.1 12 22 12ZM16 21C14.9 21 14 20.1 14 19C14 17.9 14.9 17 16 17C17.1 17 18 17.9 18 19C18 20.1 17.1 21 16 21ZM19.1 12H12.9V10C12.9 8.29 14.29 6.9 16 6.9C17.71 6.9 19.1 8.29 19.1 10V12Z"
-      fill="#C62828"
-    />
-  </svg>
+  import LockIcon from '@mui/icons-material/Lock';
+  ...
+  <TableCell>
+    <LockIcon style={{ fontSize: 32, color: '#FECDD2' }} />
+  </TableCell>

Committable suggestion was skipped due to low confidence.

}

const handleLimitChange = (event: React.ChangeEvent<HTMLInputElement>) => {
setLimit(Number(event.target.value))
Copy link

Choose a reason for hiding this comment

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

Validate input in handleLimitChange.

The handleLimitChange function should validate the input to ensure it is a valid number.

-    setLimit(Number(event.target.value))
+    const value = Number(event.target.value)
+    if (!isNaN(value) && value > 0) {
+      setLimit(value)
+    }
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
setLimit(Number(event.target.value))
const value = Number(event.target.value)
if (!isNaN(value) && value > 0) {
setLimit(value)
}

Comment on lines 152 to 154
<Button variant="outlined" color="primary" fullWidth>
CLEAR
</Button>
Copy link

Choose a reason for hiding this comment

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

Add onClick handler for CLEAR button.

The CLEAR button should have an onClick handler to clear the filters.

-  <Button variant="outlined" color="primary" fullWidth>
+  <Button variant="outlined" color="primary" fullWidth onClick={() => {
+    setStatus('');
+    setSearchQuery('');
+    setChannel('');
+    setStartDate(null);
+    setEndDate(null);
+    setLimit(10);
+    setReruns('');
+  }}>
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Button variant="outlined" color="primary" fullWidth>
CLEAR
</Button>
<Button variant="outlined" color="primary" fullWidth onClick={() => {
setStatus('');
setSearchQuery('');
setChannel('');
setStartDate(null);
setEndDate(null);
setLimit(10);
setReruns('');
}}>
CLEAR
</Button>

Comment on lines 130 to 135
<MenuItem value={10}>10</MenuItem>
<MenuItem value={20}>20</MenuItem>
<MenuItem value={50}>50</MenuItem>
<MenuItem value={100}>100</MenuItem>
<MenuItem value={200}>200</MenuItem>
<MenuItem value={500}>500</MenuItem>
Copy link

Choose a reason for hiding this comment

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

Add key prop to MenuItem components.

The MenuItem components in the Limit dropdown should have a key prop.

-  <MenuItem value={10}>10</MenuItem>
-  <MenuItem value={20}>20</MenuItem>
-  <MenuItem value={50}>50</MenuItem>
-  <MenuItem value={100}>100</MenuItem>
-  <MenuItem value={200}>200</MenuItem>
-  <MenuItem value={500}>500</MenuItem>
+  <MenuItem key={10} value={10}>10</MenuItem>
+  <MenuItem key={20} value={20}>20</MenuItem>
+  <MenuItem key={50} value={50}>50</MenuItem>
+  <MenuItem key={100} value={100}>100</MenuItem>
+  <MenuItem key={200} value={200}>200</MenuItem>
+  <MenuItem key={500} value={500}>500</MenuItem>
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<MenuItem value={10}>10</MenuItem>
<MenuItem value={20}>20</MenuItem>
<MenuItem value={50}>50</MenuItem>
<MenuItem value={100}>100</MenuItem>
<MenuItem value={200}>200</MenuItem>
<MenuItem value={500}>500</MenuItem>
<MenuItem key={10} value={10}>10</MenuItem>
<MenuItem key={20} value={20}>20</MenuItem>
<MenuItem key={50} value={50}>50</MenuItem>
<MenuItem key={100} value={100}>100</MenuItem>
<MenuItem key={200} value={200}>200</MenuItem>
<MenuItem key={500} value={500}>500</MenuItem>

/**
* Clients
*/
export async function fetchClientById(id: String): Promise<any> {
Copy link

Choose a reason for hiding this comment

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

Use lowercase string instead of String.

For consistency, use lowercase primitives.

export async function fetchClientById(id: String): Promise<any> {
+ export async function fetchClientById(id: string): Promise<any> {
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function fetchClientById(id: String): Promise<any> {
export async function fetchClientById(id: string): Promise<any> {
Tools
Biome

[error] 180-180: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)

* Transactions
* @returns
*/
export async function fetchTransactions(filters: { filterLimit: string, filterPage: number, filters: {} }): Promise<Transactions[]> {
Copy link

Choose a reason for hiding this comment

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

Avoid using {} as a type.

Prefer explicitly defining the object shape. {} means "any non-nullable value".

export async function fetchTransactions(filters: { filterLimit: string, filterPage: number, filters: {} }): Promise<Transactions[]> {
+ export async function fetchTransactions(filters: { filterLimit: string, filterPage: number, filters: Record<string, any> }): Promise<Transactions[]> {
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function fetchTransactions(filters: { filterLimit: string, filterPage: number, filters: {} }): Promise<Transactions[]> {
export async function fetchTransactions(filters: { filterLimit: string, filterPage: number, filters: Record<string, any> }): Promise<Transactions[]> {
Tools
Biome

[error] 153-153: Don't use '{}' as a type.

Prefer explicitly define the object shape. '{}' means "any non-nullable value".

(lint/complexity/noBannedTypes)

return response.data
}

export async function fetchTransactions(): Promise<any> {
export async function fetchChannelById(id: String): Promise<any> {
Copy link

Choose a reason for hiding this comment

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

Use lowercase string instead of String.

For consistency, use lowercase primitives.

export async function fetchChannelById(id: String): Promise<any> {
+ export async function fetchChannelById(id: string): Promise<any> {

Committable suggestion was skipped due to low confidence.

Tools
Biome

[error] 171-171: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)

Comment on lines 34 to 88
const fetchTransactionLogs = useCallback(async () => {
try {
const filters: {[key: string]: any} = {}

if (status !== 'NoFilter') {
filters.status = status
}

if (channel !== 'NoFilter') {
filters.channelID = channel
}

if (reruns !== 'NoFilter') {
if (reruns === 'Yes') {
filters.childIDs = JSON.stringify({$exists: true, $ne: []})
} else if (reruns === 'No') {
filters.childIDs = JSON.stringify({$eq: []})
}
}

if (startDate && endDate) {
filters['request.timestamp'] = JSON.stringify({
$gte: startDate.toISOString(),
$lte: endDate.toISOString()
})
}

const transactions = await fetchTransactions({
filterLimit: limit,
filterPage: 0,
filters: JSON.stringify(filters)
})

const transactionsWithChannelDetails = await Promise.all(
transactions.map(async transaction => {
const channelName = await fetchChannelDetails(transaction.channelID)
const clientName = await fetchClientDetails(transaction.clientID)
return {...transaction, channelName, clientName}
})
)

setTransactions(transactionsWithChannelDetails)
} catch (error) {
console.error('Error fetching logs:', error)
}
}, [status, channel, startDate, endDate, limit, reruns])

const fetchAvailableChannels = useCallback(async () => {
try {
const channels = await fetchChannels()
setChannels(channels)
} catch (error) {
console.error('Error fetching channels:', error)
}
}, [])
Copy link

Choose a reason for hiding this comment

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

Improve error handling in data fetching functions.

Consider displaying error messages to the user using dialogs or modals.

const fetchTransactionLogs = useCallback(async () => {
  try {
    const filters: {[key: string]: any} = {}

    if (status !== 'NoFilter') {
      filters.status = status
    }

    if (channel !== 'NoFilter') {
      filters.channelID = channel
    }

    if (reruns !== 'NoFilter') {
      if (reruns === 'Yes') {
        filters.childIDs = JSON.stringify({$exists: true, $ne: []})
      } else if (reruns === 'No') {
        filters.childIDs = JSON.stringify({$eq: []})
      }
    }

    if (startDate && endDate) {
      filters['request.timestamp'] = JSON.stringify({
        $gte: startDate.toISOString(),
        $lte: endDate.toISOString()
      })
    }

    const transactions = await fetchTransactions({
      filterLimit: limit,
      filterPage: 0,
      filters: JSON.stringify(filters)
    })

    const transactionsWithChannelDetails = await Promise.all(
      transactions.map(async transaction => {
        const channelName = await fetchChannelDetails(transaction.channelID)
        const clientName = await fetchClientDetails(transaction.clientID)
        return {...transaction, channelName, clientName}
      })
    )

    setTransactions(transactionsWithChannelDetails)
  } catch (error) {
    console.error('Error fetching logs:', error)
+    // Display error message to the user
+    alert('Failed to fetch transaction logs. Please try again later.')
  }
}, [status, channel, startDate, endDate, limit, reruns])
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const fetchTransactionLogs = useCallback(async () => {
try {
const filters: {[key: string]: any} = {}
if (status !== 'NoFilter') {
filters.status = status
}
if (channel !== 'NoFilter') {
filters.channelID = channel
}
if (reruns !== 'NoFilter') {
if (reruns === 'Yes') {
filters.childIDs = JSON.stringify({$exists: true, $ne: []})
} else if (reruns === 'No') {
filters.childIDs = JSON.stringify({$eq: []})
}
}
if (startDate && endDate) {
filters['request.timestamp'] = JSON.stringify({
$gte: startDate.toISOString(),
$lte: endDate.toISOString()
})
}
const transactions = await fetchTransactions({
filterLimit: limit,
filterPage: 0,
filters: JSON.stringify(filters)
})
const transactionsWithChannelDetails = await Promise.all(
transactions.map(async transaction => {
const channelName = await fetchChannelDetails(transaction.channelID)
const clientName = await fetchClientDetails(transaction.clientID)
return {...transaction, channelName, clientName}
})
)
setTransactions(transactionsWithChannelDetails)
} catch (error) {
console.error('Error fetching logs:', error)
}
}, [status, channel, startDate, endDate, limit, reruns])
const fetchAvailableChannels = useCallback(async () => {
try {
const channels = await fetchChannels()
setChannels(channels)
} catch (error) {
console.error('Error fetching channels:', error)
}
}, [])
const fetchTransactionLogs = useCallback(async () => {
try {
const filters: {[key: string]: any} = {}
if (status !== 'NoFilter') {
filters.status = status
}
if (channel !== 'NoFilter') {
filters.channelID = channel
}
if (reruns !== 'NoFilter') {
if (reruns === 'Yes') {
filters.childIDs = JSON.stringify({$exists: true, $ne: []})
} else if (reruns === 'No') {
filters.childIDs = JSON.stringify({$eq: []})
}
}
if (startDate && endDate) {
filters['request.timestamp'] = JSON.stringify({
$gte: startDate.toISOString(),
$lte: endDate.toISOString()
})
}
const transactions = await fetchTransactions({
filterLimit: limit,
filterPage: 0,
filters: JSON.stringify(filters)
})
const transactionsWithChannelDetails = await Promise.all(
transactions.map(async transaction => {
const channelName = await fetchChannelDetails(transaction.channelID)
const clientName = await fetchClientDetails(transaction.clientID)
return {...transaction, channelName, clientName}
})
)
setTransactions(transactionsWithChannelDetails)
} catch (error) {
console.error('Error fetching logs:', error)
// Display error message to the user
alert('Failed to fetch transaction logs. Please try again later.')
}
}, [status, channel, startDate, endDate, limit, reruns])

packages/transaction-log/src/components/App.tsx Outdated Show resolved Hide resolved
@ItsMurumba ItsMurumba marked this pull request as draft August 2, 2024 13:06
@ItsMurumba ItsMurumba changed the title Datatable, Basic and Custom Filter Tabs Implementation Transaction Log Implementation Aug 15, 2024
@ItsMurumba ItsMurumba marked this pull request as ready for review August 15, 2024 07:44
Comment on lines 163 to 170
// useEffect(() => {
// const intervalId = setInterval(async () => {
// const timestamp = new Date().toISOString()
// await fetchTransactionLogs('2024-08-14T15:50:25+03:00') // Auto-polling with timestamp filter, without filterLimit and filterPage
// }, 5000)

// return () => clearInterval(intervalId) // Cleanup interval on component unmount
// }, [fetchTransactionLogs])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove unused codes

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems we still have the commented code. Is it useful?

Copy link
Author

Choose a reason for hiding this comment

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

I have removed them and do not exist in my local machine. @drizzentic

packages/transaction-log/src/types/index.spec.ts Outdated Show resolved Hide resolved
packages/transaction-log/src/types/index.spec.ts Outdated Show resolved Hide resolved
@drizzentic drizzentic merged commit 3bd9c34 into jembi:poc-microfrontends Aug 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants