-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
AWS Integration: Integrate now modal #6807
AWS Integration: Integrate now modal #6807
Conversation
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
1 similar comment
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
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.
❌ Changes requested. Reviewed everything up to 060fc10 in 1 minute and 55 seconds
More details
- Looked at
1727
lines of code in22
files - Skipped
3
files when reviewing. - Skipped posting
6
drafted comments based on config settings.
1. frontend/src/api/integrations/aws/index.ts:22
- Draft comment:
Consider using a configurable base URL instead of hardcoding 'http://localhost:3000'. This will make the code more flexible and suitable for different environments. - Reason this comment was not posted:
Marked as duplicate.
2. frontend/src/api/integrations/aws/index.ts:36
- Draft comment:
Consider using a configurable base URL instead of hardcoding 'http://localhost:3000'. This will make the code more flexible and suitable for different environments. - Reason this comment was not posted:
Marked as duplicate.
3. frontend/src/api/integrations/aws/index.ts:48
- Draft comment:
Consider using a configurable base URL instead of hardcoding 'http://localhost:3000'. This will make the code more flexible and suitable for different environments. - Reason this comment was not posted:
Marked as duplicate.
4. frontend/src/hooks/integrations/aws/useAccountStatus.ts:15
- Draft comment:
Consider using a configurable base URL instead of hardcoding 'http://localhost:3000'. This will make the code more flexible and suitable for different environments. - Reason this comment was not posted:
Marked as duplicate.
5. frontend/src/hooks/integrations/aws/useAccountStatus.ts:17
- Draft comment:
Remove console.log statements or replace them with a proper logging mechanism for production use. - Reason this comment was not posted:
Marked as duplicate.
6. frontend/src/pages/Integrations/CloudIntegrationPage/HeroSection/HeroSection.tsx:24
- Draft comment:
Avoid using inline styles in React components. Move these styles to an external stylesheet or use styled components. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_hXbt39gmCRwfvwaM
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
|
||
export const getAwsAccounts = async (): Promise<CloudAccount[]> => { | ||
const response = await axios.get( | ||
'http://localhost:3000/api/v1/cloud-integrations/aws/accounts', |
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.
Consider using a configurable base URL instead of hardcoding 'http://localhost:3000'. This will make the code more flexible and suitable for different environments.
return useQuery<AccountStatusResponse, AxiosError>({ | ||
queryKey: ['accountStatus', accountId], | ||
queryFn: async () => { | ||
console.log('fetching account status'); |
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.
Remove console.log statements or replace them with a proper logging mechanism for production use.
import AlertMessage from '../AlertMessage'; | ||
import { ModalStateEnum, RegionFormProps } from '../types'; | ||
|
||
const allRegions = (): string[] => |
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.
This array transformation logic is duplicated. Consider reusing the existing implementation or extracting it to a shared utility.
- allRegions (useIntegrationModal.ts)
5e59c1f
to
6c3b326
Compare
060fc10
to
d044344
Compare
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
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.
❌ Changes requested. Incremental review on d044344 in 1 minute and 1 seconds
More details
- Looked at
899
lines of code in9
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. frontend/src/container/CloudIntegrationPage/HeroSection/components/CloudAccountSetupModal.tsx:60
- Draft comment:
ThehandleSubmit
function currently logs form values to the console but does not perform any further action. Ensure that the form submission logic is complete and handles the form data appropriately. - Reason this comment was not posted:
Comment did not seem useful.
2. frontend/src/container/CloudIntegrationPage/HeroSection/components/RegionForm.tsx:157
- Draft comment:
TheSelect
component for monitoring regions is set toopen={false}
. This might prevent users from interacting with the dropdown. Ensure this is the intended behavior or adjust accordingly. - Reason this comment was not posted:
Comment did not seem useful.
3. frontend/src/container/CloudIntegrationPage/HeroSection/components/RegionForm.tsx:93
- Draft comment:
Avoid using inline styles for thestyle
prop in theSelect
component. Use CSS classes or styled components instead. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_sWa9xt9wexwLpmBP
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
<Select | ||
placeholder="US East (N. Virginia)" | ||
suffixIcon={<ChevronDown size={16} color={Color.BG_VANILLA_400} />} | ||
style={{ height: '44px' }} |
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.
Avoid using inline styles for the style
prop in the Select
component. Use CSS classes or styled components instead. This issue is also present in other files like RegionForm.tsx
.
6c3b326
to
6f9b9c7
Compare
… to the integrations modal
d044344
to
3bae1b6
Compare
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
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.
❌ Changes requested. Incremental review on 3bae1b6 in 1 minute and 9 seconds
More details
- Looked at
916
lines of code in10
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. frontend/src/container/CloudIntegrationPage/HeroSection/components/RegionForm.tsx:155
- Draft comment:
open={false}
is not a valid prop for theSelect
component. Consider usingdisabled
or another method to prevent opening. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
2. frontend/src/container/CloudIntegrationPage/HeroSection/components/IntegrateNowFormSections.tsx:28
- Draft comment:
Avoid using inline styles. Use external stylesheets, CSS classes, or styled components instead. This issue is also present in other files likeCloudAccountSetupModal.tsx
. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_kSBaywd1aWbpaC4D
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
try { | ||
setIsLoading(true); | ||
const values = await form.validateFields(); | ||
console.log(values); |
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.
Remove console.log(values);
to avoid unnecessary console output in production.
block: activeView === 'form', | ||
}} | ||
cancelButtonProps={{ | ||
style: { display: activeView === 'form' ? 'none' : 'block' }, |
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.
Avoid using inline styles. Use external stylesheets, CSS classes, or styled components instead. This issue is also present in other files like IntegrateNowFormSections.tsx
.
* feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: integrate now modal UI * feat: integrate now modal states and json server API integration * feat: account settings * feat: service status UI * refactor: make account settings modal more readable and overall improvements * Get data from json server api data in service sections (#6809) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * refactor: make the cloud account setup modal readable / DRYer * feat: integrate now modal states and json server API integration * refactor: make account settings modal more readable and overall improvements * feat: integrate now modal states and json server API integration * feat: display error state if last_heartbeat_ts_ms is null even after 5 minutes * feat: get the services list and details from json server API response * feat: update account actions to set accountId in URL query on initial account load * Configure service modal (#6814) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * refactor: make the cloud account setup modal readable / DRYer * feat: integrate now modal states and json server API integration * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: integrate now modal states and json server API integration * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: display error state if last_heartbeat_ts_ms is null even after 5 minutes * feat: account settings * feat: service status UI * feat: get the services list and details from json server API response * feat: update account actions to set accountId in URL query on initial account load * feat: configure service modal UI * feat: configure service modal functionality and API changes * feat: replace loading indicators with Spinner component in ServiceDetails and ServicesList * fix: make the configure service modal work * Light mode support and overall improvements to AWS integration page (#6817) * refactor: make the cloud account setup modal readable / DRYer * feat: integrate now modal states and json server API integration * refactor: make account settings modal more readable and overall improvements * fix: integrate now modal button improvements * feat: aws integrations light mode * refactor: overall improvements * refactor: define react query keys in constant * feat: services filter * feat: render service overview as markdown * feat: integrate AWS integration page API (#6851) * feat: replace json-server APIs with actual APIs * fix: add null checks and fix the issues
f763e0f
into
account-setup-basic-UI-and-functionality
* feat: implement basic cloud account management UI in HeroSection * AWS Integration: Integrate now modal (#6807) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: integrate now modal UI * feat: integrate now modal states and json server API integration * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: display error state if last_heartbeat_ts_ms is null even after 5 minutes * chore: update import path for regions data in useRegionSelection hook * chore: move hero section components inside the HeroSection/components * feat: create a reusable modal component * refactor: make the cloud account setup modal readable / DRYer * AWS Integration: Account settings modal (#6808) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: integrate now modal UI * feat: integrate now modal states and json server API integration * feat: account settings * feat: service status UI * refactor: make account settings modal more readable and overall improvements * Get data from json server api data in service sections (#6809) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * refactor: make the cloud account setup modal readable / DRYer * feat: integrate now modal states and json server API integration * refactor: make account settings modal more readable and overall improvements * feat: integrate now modal states and json server API integration * feat: display error state if last_heartbeat_ts_ms is null even after 5 minutes * feat: get the services list and details from json server API response * feat: update account actions to set accountId in URL query on initial account load * Configure service modal (#6814) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * refactor: make the cloud account setup modal readable / DRYer * feat: integrate now modal states and json server API integration * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: integrate now modal states and json server API integration * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: display error state if last_heartbeat_ts_ms is null even after 5 minutes * feat: account settings * feat: service status UI * feat: get the services list and details from json server API response * feat: update account actions to set accountId in URL query on initial account load * feat: configure service modal UI * feat: configure service modal functionality and API changes * feat: replace loading indicators with Spinner component in ServiceDetails and ServicesList * fix: make the configure service modal work * Light mode support and overall improvements to AWS integration page (#6817) * refactor: make the cloud account setup modal readable / DRYer * feat: integrate now modal states and json server API integration * refactor: make account settings modal more readable and overall improvements * fix: integrate now modal button improvements * feat: aws integrations light mode * refactor: overall improvements * refactor: define react query keys in constant * feat: services filter * feat: render service overview as markdown * feat: integrate AWS integration page API (#6851) * feat: replace json-server APIs with actual APIs * fix: add null checks and fix the issues
* feat: add AWS integration in the integrations list and redirect to the new Cloud Integration page * feat: cloud integration details page header (i.e. breadcrumb and get help button) UI * feat: hero section UI * refactor: extract Header and HeroSection components from CloudIntegrationPage * feat: services tab bar and sidebar UI * feat: cloud integration details services UI * refactor: group and extract cloud integration components to files * fix: set default active service to the first service in the list if no service is specified * feat: add NEW flag for AWS integration in the integrations list page * chore: overall improvements * chore: move cloud integration pages to /container * fix: hero section background * AWS Integration: Account setup basic UI and functionality (#6806) * feat: implement basic cloud account management UI in HeroSection * AWS Integration: Integrate now modal (#6807) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: integrate now modal UI * feat: integrate now modal states and json server API integration * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: display error state if last_heartbeat_ts_ms is null even after 5 minutes * chore: update import path for regions data in useRegionSelection hook * chore: move hero section components inside the HeroSection/components * feat: create a reusable modal component * refactor: make the cloud account setup modal readable / DRYer * AWS Integration: Account settings modal (#6808) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: integrate now modal UI * feat: integrate now modal states and json server API integration * feat: account settings * feat: service status UI * refactor: make account settings modal more readable and overall improvements * feat: Get data from json server api data in service sections (#6809) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * refactor: make the cloud account setup modal readable / DRYer * feat: integrate now modal states and json server API integration * refactor: make account settings modal more readable and overall improvements * feat: integrate now modal states and json server API integration * feat: display error state if last_heartbeat_ts_ms is null even after 5 minutes * feat: get the services list and details from json server API response * feat: update account actions to set accountId in URL query on initial account load * feat: configure service modal (#6814) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * refactor: make the cloud account setup modal readable / DRYer * feat: integrate now modal states and json server API integration * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: integrate now modal states and json server API integration * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: display error state if last_heartbeat_ts_ms is null even after 5 minutes * feat: account settings * feat: service status UI * feat: get the services list and details from json server API response * feat: update account actions to set accountId in URL query on initial account load * feat: configure service modal UI * feat: configure service modal functionality and API changes * feat: replace loading indicators with Spinner component in ServiceDetails and ServicesList * fix: make the configure service modal work * Light mode support and overall improvements to AWS integration page (#6817) * refactor: make the cloud account setup modal readable / DRYer * feat: integrate now modal states and json server API integration * refactor: make account settings modal more readable and overall improvements * fix: integrate now modal button improvements * feat: aws integrations light mode * refactor: overall improvements * refactor: define react query keys in constant * feat: services filter * feat: render service overview as markdown * feat: integrate AWS integration page API (#6851) * feat: replace json-server APIs with actual APIs * fix: add null checks and fix the issues
Summary
Related Issues / PR's
Screenshots
NA
Affected Areas and Manually Tested Areas
Important
Integrates AWS account setup and region selection with new modals, forms, and API interactions in the frontend.
CloudIntegrationPage
for account setup and region selection.SignozModal
for consistent modal styling and behavior.RegionSelector
andRegionForm
for selecting AWS regions.SuccessView.tsx
.getAwsAccounts
,getAwsServices
,getServiceDetails
, andgenerateConnectionUrl
inindex.ts
for AWS API interactions.useAwsAccounts
,useServiceDetails
,useAccountStatus
, anduseGenerateConnectionUrl
for data fetching and mutation.SignozModal.style.scss
,CloudAccountSetupModal.style.scss
, andSuccessView.style.scss
.package.json
to includereact-lottie
andprop-types
dependencies.AccountActions
to use new hooks and components.This description was created by for 3bae1b6. It will automatically update as commits are pushed.