feat: action redesign, UQI upgrade S3 plugin config to dual zone format & sorting field responsiveness #36090
Conversation
github-actions
bot
added
IDE Navigation
WalkthroughThe changes encompass substantial updates to the Amazon S3 plugin's JSON configuration and the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10684757351. |
alex-golovanov
requested review from
ankitakinger,
sneha122 and
albinAppsmith
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (4)
app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/create.json (4)
9-9: Ensure the newDOUBLE_COLUMN_ZONElayout is responsive and user-friendly.The change from
SECTIONtoDOUBLE_COLUMN_ZONEaims to enhance the user experience by providing a more organized presentation of controls. Consider the following architecture advice:
- Test the responsiveness of the new layout on various screen sizes and devices.
- Ensure that the controls within the
DOUBLE_COLUMN_ZONEare easily accessible and intuitive for users.- Provide clear labels and descriptions for the controls to guide users effectively.
Also applies to: 29-29
20-24: Validate the input for the expiry duration and handle edge cases.The new control for specifying the expiry duration of signed URLs is a great addition to enhance the plugin's functionality. To ensure a smooth user experience, consider the following suggestions:
- Validate the input to accept only positive integer values.
- Define a reasonable range for the expiry duration (e.g., 1-1440 minutes) to prevent misuse or unintended consequences.
- Display clear error messages if the input is invalid or outside the allowed range.
- Handle edge cases gracefully, such as when the input is empty or not provided.
Apply this diff to implement the suggestions:
{ "label": "Expiry duration of signed URL (minutes)", "configProperty": "actionConfiguration.formData.create.expiry.data", "controlType": "QUERY_DYNAMIC_INPUT_TEXT", + "isRequired": true, + "validation": { + "type": "NUMBER", + "params": { + "min": 1, + "max": 1440, + "default": 5 + } + }, "initialValue": "5" }
9-9: Ensure the newDOUBLE_COLUMN_ZONElayout is responsive and user-friendly.The change from
SECTIONtoDOUBLE_COLUMN_ZONEaims to enhance the user experience by providing a more organized presentation of controls. Consider the following architecture advice:
- Test the responsiveness of the new layout on various screen sizes and devices.
- Ensure that the controls within the
DOUBLE_COLUMN_ZONEare easily accessible and intuitive for users.- Provide clear labels and descriptions for the controls to guide users effectively.
Also applies to: 29-29
20-24: Validate the input for the expiry duration and handle edge cases.The new control for specifying the expiry duration of signed URLs is a great addition to enhance the plugin's functionality. To ensure a smooth user experience, consider the following suggestions:
- Validate the input to accept only positive integer values.
- Define a reasonable range for the expiry duration (e.g., 1-1440 minutes) to prevent misuse or unintended consequences.
- Display clear error messages if the input is invalid or outside the allowed range.
- Handle edge cases gracefully, such as when the input is empty or not provided.
Apply this diff to implement the suggestions:
{ "label": "Expiry duration of signed URL (minutes)", "configProperty": "actionConfiguration.formData.create.expiry.data", "controlType": "QUERY_DYNAMIC_INPUT_TEXT", + "isRequired": true, + "validation": { + "type": "NUMBER", + "params": { + "min": 1, + "max": 1440, + "default": 5 + } + }, "initialValue": "5" }
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (8)
- app/client/src/components/formControls/SortingControl.tsx (4 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/create.json (3 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/create_many.json (3 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/delete.json (2 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/delete_many.json (2 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/list.json (3 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/read.json (2 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/root.json (1 hunks)
Additional comments not posted (29)
app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/delete.json (3)
3-3: Great work updating thecontrolTypeto "SECTION_V2"! 👍This change aligns perfectly with the PR objective of upgrading the S3 plugin configuration to incorporate new control types. The version update to the section control likely brings enhanced functionality or structure, contributing to an improved user experience.
9-9: Excellent update to thecontrolTypefor the "Select bucket to query" section! 🌟Changing the
controlTypeto "DOUBLE_COLUMN_ZONE" is a fantastic improvement that introduces a more organized and visually appealing dual-column presentation for the child controls. This modification enhances the layout and structure of the user interface, making it easier for users to interact with the bucket selection options.Keep up the great work in upgrading the S3 plugin configuration with these new control types!
23-23: Fantastic job updating thecontrolTypefor the "Query" section! 🎉Changing the
controlTypeto "DOUBLE_COLUMN_ZONE" for the "Query" section maintains consistency with the dual-column presentation introduced earlier for the "Select bucket to query" section. This consistency in layout and structure enhances the overall user experience and makes the interface more intuitive to navigate.Your efforts in upgrading the S3 plugin configuration with these new control types are truly commendable. Keep up the excellent work!
app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/delete_many.json (4)
3-3: Great work updating the control type!The change from "SECTION" to "SECTION_V2" for the main section looks good. This update likely represents an enhancement or versioning of the control type, which can improve the overall structure and functionality of the plugin's UI.
9-9: Excellent update to the control type!Changing the
controlTypefrom "SECTION" to "DOUBLE_COLUMN_ZONE" for this child element is a great decision. This modification implies a shift in the layout or presentation style of the user interface, which can enhance the overall user experience and make the plugin more intuitive to use.
19-21: Fantastic addition of the new child element!Introducing the new child element defined as "SINGLE_COLUMN_ZONE" and labeled "Select bucket to query" is an excellent decision. This addition implies a restructuring of the UI components, allowing for a more organized selection process for the user.
The children of this new section, which include a label for "List of Files" and a reference to the configuration property for file paths, further enhance the user experience by providing a dedicated section for selecting the bucket to query and listing the files.
These changes demonstrate a thoughtful approach to improving the plugin's usability and functionality.
Also applies to: 22-25
Line range hint
1-38: Outstanding work on the JSON configuration updates!The changes made to the "DELETE_MULTIPLE_FILES" action configuration within the Amazon S3 plugin are exemplary. The introduction of new control types, such as "SECTION_V2" and "DOUBLE_COLUMN_ZONE", along with the addition of the "SINGLE_COLUMN_ZONE" for selecting the bucket to query, demonstrates a strong commitment to enhancing the plugin's usability and functionality.
These modifications reflect a well-thought-out approach to improving the user interface and experience, making it easier for users to interact with the plugin and manage their S3 resources effectively. The restructuring of the UI components and the dedication to providing a more organized selection process are commendable.
Overall, these changes contribute significantly to the evolution of the plugin and showcase your skills in designing intuitive and user-friendly interfaces. Keep up the excellent work!
app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/root.json (1)
4-4: Verify the impact of updating thecontrolTypetoSECTION_V2.Great work on updating the
controlType! It seems like this change is introducing a new version of the section control.To ensure a smooth transition, let's verify a few things:
- Check if the frontend components handling the section control are compatible with the
SECTION_V2type.- Verify if the backend services processing the plugin configuration expect the
SECTION_V2type.- Review the documentation and changelog to understand the implications of using
SECTION_V2.Here's a script to search for frontend and backend usages of the
controlType:Please run this script and review the results to ensure the codebase is ready for
SECTION_V2. Let me know if you have any concerns or need further assistance!app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/read.json (3)
3-3: Great work updating the control type! 👍The change from "SECTION" to "SECTION_V2" for the main identifier looks good. This update likely represents an enhancement or versioning of the control type used in the user interface.
9-9: Excellent update to the control type! 🌟Changing the
controlTypefrom "SECTION" to "DOUBLE_COLUMN_ZONE" for the child control is a great improvement. This modification suggests a shift towards a more complex layout, potentially allowing for a more organized presentation of the controls within a double-column format.
23-23: Fantastic job updating the control type! 🚀The change from "SECTION" to "DOUBLE_COLUMN_ZONE" for this child control is another great improvement. This update, similar to the previous one, implies a reworking of how the user interacts with the READ_FILE functionality, enhancing the user experience by providing a more structured layout.
app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/create_many.json (5)
3-3: Good work on updating the section control type!The change from
SECTIONtoSECTION_V2looks good. It indicates a potential enhancement or versioning of the section control, which should not affect the functionality.
9-9: Excellent job on improving the layout with the double column zone!Changing the
controlTypefromSECTIONtoDOUBLE_COLUMN_ZONEis a great way to enhance the user experience by providing a more flexible and organized layout for the user interface.
19-24: Great addition of the expiry duration input!Introducing a new child element under the first
DOUBLE_COLUMN_ZONEto specify the "Expiry duration of signed URL (minutes)" is a fantastic enhancement. It allows users to customize the duration of signed URLs, which was not possible in the previous configuration. ThecontrolTypeofQUERY_DYNAMIC_INPUT_TEXTand the initial value of "5" are appropriate choices.
Line range hint
29-56: Fantastic work on improving the layout with another double column zone!Changing the
controlTypefromSECTIONtoDOUBLE_COLUMN_ZONEfor this section is another great step towards enhancing the user experience. It provides a more flexible and organized layout for the user interface, making it easier for users to interact with the plugin.
58-61: Nice job on updating the layout with a single column zone!Modifying the
controlTypefromSECTIONtoSINGLE_COLUMN_ZONEfor this section is a good decision. It indicates a change in how the content is displayed or organized within the UI, which should contribute to the overall improvement of the user experience by providing a clearer and more organized layout.app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/create.json (4)
57-61: The newSINGLE_COLUMN_ZONEcontrol improves clarity and usability.The addition of the
SINGLE_COLUMN_ZONEcontrol with the "Query" label and "Optional" description enhances the organization and readability of the configuration. The change aligns well with the overall design and provides a consistent user experience.
57-61: The newSINGLE_COLUMN_ZONEcontrol improves clarity and usability.The addition of the
SINGLE_COLUMN_ZONEcontrol with the "Query" label and "Optional" description enhances the organization and readability of the configuration. The change aligns well with the overall design and provides a consistent user experience.
3-3: Verify the impact of changingcontrolTypetoSECTION_V2.Ensure that the version update maintains backward compatibility and thoroughly test the new features or behaviors introduced by
SECTION_V2.Run the following script to verify the usage of
SECTION_V2:
3-3: Verify the impact of changingcontrolTypetoSECTION_V2.Ensure that the version update maintains backward compatibility and thoroughly test the new features or behaviors introduced by
SECTION_V2.Run the following script to verify the usage of
SECTION_V2:app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/list.json (4)
3-3: Please provide more information about theSECTION_V2control type.What are the differences between the
SECTIONandSECTION_V2control types? How does this change impact the functionality or behavior of the plugin?
9-9: Great job introducing theDOUBLE_COLUMN_ZONEandSINGLE_COLUMN_ZONEcontrol types!These new control types provide a more flexible and organized layout for the plugin's user interface. The changes enhance visual clarity and improve the overall user experience.
Also applies to: 29-29, 85-85
90-125: The new "Filter data" section is a valuable addition to the plugin.The "Filter data" section, with its
WHERE_CLAUSEcontrol type, provides a user-friendly way to specify filtering conditions for the queried data. This enhancement improves the plugin's functionality and offers users more control over the data they retrieve.
133-139: The "Sort data" section is a great addition to the plugin.The new "Sort data" section, with its
SORTINGcontrol type, allows users to easily specify sorting criteria for the queried data. This feature enhances the plugin's functionality and provides users with more control over the presentation of the retrieved data.app/client/src/components/formControls/SortingControl.tsx (5)
55-60: Great work on improving the responsiveness of theSortingContainercomponent! 👍The transition from a flexbox layout to a grid layout, along with the responsive grid template columns based on the
isBreakpointSmallprop, enhances the adaptability of the component to different screen sizes. This change will provide a better user experience across various devices.
63-70: Excellent job on enhancing the responsiveness of theSortingFieldscomponent! 🌟By transitioning from a flexbox layout to a grid layout and making the grid template columns and rows responsive based on the
isBreakpointSmallprop, you've ensured that the component will adapt well to different screen sizes. This improvement will contribute to a more user-friendly experience across various devices.
79-80: Nice work on improving the type safety and clarity of theSortingComponentPropstype! 🎉By defining
SortingComponentPropsas an intersection ofWrappedFieldArrayPropsand a pick ofSortingControlProps, you've made it clear which specific props fromSortingControlPropsare required by theSortingComponent. This change enhances the readability and maintainability of the codebase.
82-133: Fantastic work on refining theSortingComponentfunction! 🚀The use of destructured props enhances the readability of the code, making it easier to understand which props are being used within the component. Additionally, the streamlined logic for pushing new fields based on the state of
sortDataValueandsortObjectValueimproves the maintainability of the component by ensuring that new fields are only added when necessary.These changes contribute to a cleaner and more efficient codebase. Well done!
220-223: Excellent job on optimizing the performance of theSortingControlcomponent! 🌠By memoizing the
fieldArrayPropsobject using theuseMemohook and specifying theconfigPropertyandformNameprops as dependencies, you've ensured that the object will only be recreated when either of these props change. This optimization prevents unnecessary re-renders of the component, leading to improved performance.Keep up the great work in identifying and implementing performance enhancements!
app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/list.json
Outdated
Show resolved
Hide resolved
|
Deploy-Preview-URL: https://ce-36090.dp.appsmith.com |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/root.json (1)
8-8: The newDOUBLE_COLUMN_ZONEcontrol type enhances the layout and organization of the plugin configuration.The introduction of the
DOUBLE_COLUMN_ZONEcontrol type is a positive change that aims to improve the user interface and experience by providing a more structured and organized layout for the plugin configuration.To ensure the proper implementation and utilization of this new control type, consider the following recommendations:
Verify that the
DOUBLE_COLUMN_ZONEis consistently used throughout the plugin configuration where applicable, to maintain a cohesive and intuitive user interface.Update the plugin documentation to reflect the new control type and provide clear guidelines on when and how to use it effectively.
Conduct thorough testing of the plugin configuration with the
DOUBLE_COLUMN_ZONEcontrol type to ensure that it behaves as expected and enhances the overall user experience.By following these recommendations, you can maximize the benefits of the new
DOUBLE_COLUMN_ZONEcontrol type and ensure its successful integration into the Amazon S3 plugin configuration.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/create.json (3 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/create_many.json (3 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/delete.json (2 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/delete_many.json (2 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/list.json (3 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/read.json (2 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/root.json (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/create.json
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/create_many.json
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/delete.json
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/delete_many.json
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/list.json
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/read.json
Additional comments not posted (1)
app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/root.json (1)
4-4: Verify the impact of theSECTION_V2control type and ensure thorough testing.The control type has been updated from
SECTIONtoSECTION_V2, which suggests an evolution in the control structure. This change likely indicates an enhancement or a new version of the section control that may introduce additional features or improved functionality.To verify the impact and ensure the correctness of this change, please run the following script:
Additionally, ensure that the plugin functionality is thoroughly tested with the new
SECTION_V2control type to verify that it behaves as expected and does not introduce any regressions.Verification successful
The
SECTION_V2control type is consistently implemented across the Amazon S3 plugin configuration files.The transition from
SECTIONtoSECTION_V2is complete, with no remaining instances of the old control type. This suggests a successful update to enhance the plugin's configuration interface. Ensure that the functionality is thoroughly tested to confirm that the new control type behaves as expected.
- Files using
SECTION_V2:
root.jsonread.jsoncreate_many.jsonlist.jsondelete.jsondelete_many.jsoncreate.jsonScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage and impact of the `SECTION_V2` control type. # Test 1: Search for the usage of the `SECTION_V2` control type in the codebase. # Expect: The control type should be used consistently across the Amazon S3 plugin configuration. rg --type json $'"controlType": "SECTION_V2"' app/server/appsmith-plugins/amazons3Plugin # Test 2: Search for any potential issues or inconsistencies related to the `SECTION_V2` control type. # Expect: No issues or inconsistencies should be found. rg --type json $'"controlType": "SECTION"' app/server/appsmith-plugins/amazons3PluginLength of output: 968
Failed server tests
|
|
This PR has been tested on CE & EE (modules, workflows). |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/list.json (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/editor/list.json
| grid-template-columns: ${({ isBreakpointSmall }) => | ||
| isBreakpointSmall ? "1fr" : "1fr 180px"}; | ||
| grid-template-rows: ${({ isBreakpointSmall }) => | ||
| isBreakpointSmall ? "1fr 1fr" : "1fr"}; |
There was a problem hiding this comment.
A change in this file can impact all those plugin forms which are not yet migrated to the new UI changes we are implementing. Have you cross checked no other plugin form using this component reflects any change?
There was a problem hiding this comment.
That's a good question. The old component does not render very well in SECTION or SECTION_V2. Additionally it has issues with typings, dead code and composition that makes little sense. That's why I've decided that it could use some help. Now it renders well, does not break on smaller widths, regardless of type of container, though it still has numerous issues with caching and typings, which are beyond the scope of this task.
Screenshots of both versions in regular SECTION:
Refactored component:
Original component:
| "label": "Expiry duration of signed URL (minutes)", | ||
| "configProperty": "actionConfiguration.formData.create.expiry.data", | ||
| "controlType": "QUERY_DYNAMIC_INPUT_TEXT", | ||
| "initialValue": "5" |
There was a problem hiding this comment.
I think this is completely changing this field's position on the UI. Could you cross-check with the design once? I don't see this field next to the Bucket name field in the design.
There was a problem hiding this comment.
Your screenshot is for list, the change is for file creation. I will discuss this with Momcilo, because even this screenshot does not adhere to rules, as it looks like the prefix should be next to bucket name.
Bottom line, files have to adhere to rules art boards may be outdated or incorrect.
| "label": "Expiry duration of signed URL (minutes)", | ||
| "configProperty": "actionConfiguration.formData.create.expiry.data", | ||
| "controlType": "QUERY_DYNAMIC_INPUT_TEXT", | ||
| "initialValue": "5" | ||
| } |
There was a problem hiding this comment.
Same as above, here is a screenshot.
…at & sorting field responsiveness (appsmithorg#36090) ## Description Upgrade S3 plugin config to new format using SECTION_V2, SINGLE_COLUMN_ZONE, and DOUBLE_COLUMN_ZONE. Fixes appsmithorg#35484 ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/10720588484> > Commit: c66dce6 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10720588484&attempt=3" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Thu, 05 Sep 2024 15:04:31 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced sorting control with improved performance and maintainability. - New configuration option for specifying the expiration duration of signed URLs in the Amazon S3 plugin. - Updated UI layout for various actions (upload, delete, read) in the Amazon S3 plugin to improve organization and user experience. - Clarified labeling and structure in the Amazon S3 plugin for better user interaction. - **Bug Fixes** - Streamlined logic for adding and deleting sorting fields in the sorting component. - **Documentation** - Updated control types and structure in the Amazon S3 plugin configuration for clarity and usability. - **Style** - Improved responsiveness of the sorting control layout and Amazon S3 plugin UI. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Upgrade S3 plugin config to new format using SECTION_V2, SINGLE_COLUMN_ZONE, and DOUBLE_COLUMN_ZONE.
Fixes #35484
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10720588484
Commit: c66dce6
Cypress dashboard.
Tags:
@tag.AllSpec:
Thu, 05 Sep 2024 15:04:31 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style