-
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
Configure service modal #6814
Configure service modal #6814
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> |
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 2d3a61b in 1 minute and 7 seconds
More details
- Looked at
780
lines of code in10
files - Skipped
0
files when reviewing. - Skipped posting
7
drafted comments based on config settings.
1. frontend/src/container/CloudIntegrationPage/HeroSection/components/components/RegionForm.tsx:40
- Draft comment:
Consider definingrefetchInterval
as a constant for better maintainability, as it is used in multiple places. - Reason this comment was not posted:
Confidence changes required:50%
TherefetchInterval
is hardcoded in multiple places. It would be better to define it as a constant for maintainability.
2. frontend/src/container/CloudIntegrationPage/ServicesSection/ConfigureServiceModal.tsx:94
- Draft comment:
TheisSaveDisabled
logic should check if the current values differ from the initial configuration to enable saving. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The current logic makes sense - if both metrics and logs are disabled, there's nothing to save. Comparing against initial values would prevent saving when no changes were made, but that's not necessarily bad UX - users might want to review and save current settings even if unchanged. The current approach is simpler and clearer.
The comment has a point - allowing saves when no changes were made could be considered inefficient. It could trigger unnecessary API calls.
While preventing unchanged saves could optimize API calls, it adds complexity for minimal benefit. The current approach is more intuitive - if at least one service is enabled, you can save.
The current implementation is reasonable and simple. The suggested change would add complexity without significant benefit.
3. frontend/src/container/CloudIntegrationPage/ServicesSection/ConfigureServiceModal.tsx:123
- Draft comment:
Remove the extra space inrootClassName
to avoid potential styling issues.
rootClassName="configure-service-modal"
- Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
While having an extra space in a className could potentially cause styling issues, the fact that this pattern is repeated elsewhere in the file (line 133) suggests it might be intentional. Without seeing the CSS files and understanding the styling system, we can't be certain this would cause any real problems. The build system would likely handle this fine either way.
I might be underestimating the impact of extra spaces in class names. Also, even if it works, it could still be considered a code quality issue worth fixing.
While code cleanliness is important, this seems like a very minor issue that doesn't clearly cause any problems, and the consistency of the pattern suggests it might be intentional.
Delete the comment as it's not clearly identifying a real problem, and the extra space might be intentional given its consistent usage in the file.
4. frontend/src/container/CloudIntegrationPage/HeroSection/components/components/RegionForm.tsx:34
- Draft comment:
Avoid using inline styles in React components. Use external stylesheets, CSS classes, or styled components instead. This is also applicable on line 143. - Reason this comment was not posted:
Comment was on unchanged code.
5. frontend/src/container/CloudIntegrationPage/ServicesSection/ConfigureServiceModal.styles.scss:6
- Draft comment:
Use design tokens or predefined color constants instead of hardcoding color values to maintain consistency in design and theming. This is also applicable on lines 15 and 25. - Reason this comment was not posted:
Comment looked like it was already resolved.
6. frontend/src/container/CloudIntegrationPage/ServicesSection/ConfigureServiceModal.styles.scss:15
- Draft comment:
Use design tokens or predefined color constants instead of hardcoding color values to maintain consistency in design and theming. This is also applicable on lines 6 and 25. - Reason this comment was not posted:
Marked as duplicate.
7. frontend/src/container/CloudIntegrationPage/ServicesSection/ConfigureServiceModal.styles.scss:25
- Draft comment:
Use design tokens or predefined color constants instead of hardcoding color values to maintain consistency in design and theming. This is also applicable on lines 6 and 15. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_zsCFHp3XEwQnu9DF
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.
`http://localhost:3000/api/v1/cloud-integrations/aws/services/${serviceId}/config`, | ||
payload, | ||
); | ||
console.log({ serviceId }); |
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 the console.log statement to clean up the code.
console.log({ serviceId }); |
075fdd2
to
7bb9a57
Compare
… to the integrations modal
… to the integrations modal
… to the integrations modal
…ails and ServicesList
2d3a61b
to
542a9fb
Compare
…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
88680fe
into
get-data-from-json-server-API-data-in-service-sections
* 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: 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: 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: 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
Add
ConfigureServiceModal
for AWS service settings, update related API and UI components.ConfigureServiceModal
component inConfigureServiceModal.tsx
for configuring AWS service settings.updateServiceConfig
function inindex.ts
for updating service configurations.useUpdateServiceConfig
hook inuseUpdateServiceConfig.ts
for handling service configuration updates.Spinner
component inServiceDetails.tsx
andServicesList.tsx
.ConfigureServiceModal
inConfigureServiceModal.styles.scss
.useRef
inRegionForm.tsx
and adjustrefetchInterval
.data.ts
.UpdateServiceConfigPayload
andUpdateServiceConfigResponse
types intypes.ts
.This description was created by for 2d3a61b. It will automatically update as commits are pushed.