fix: Move expires_in field after scope in OAuth2 datasource configuration#41659
Open
xingzihai wants to merge 2 commits intoappsmithorg:releasefrom
Open
fix: Move expires_in field after scope in OAuth2 datasource configuration#41659xingzihai wants to merge 2 commits intoappsmithorg:releasefrom
xingzihai wants to merge 2 commits intoappsmithorg:releasefrom
Conversation
- Add table of contents for easy navigation - Add project architecture overview with tech stack - Add quick start checklist for new contributors - Add finding issues section with categorized links - Add development setup quick guide - Add pull request process guidelines - Add community support channels - Improve overall structure and organization
…tion This change moves the 'Authorization expires in (seconds)' input field to appear right after the 'Scope(s)' field in the OAuth2 datasource configuration form, making all authentication-related fields adjacent to each other for better user discoverability. Fixes appsmithorg#31059
Contributor
WalkthroughDocumentation for contribution guidelines was comprehensively reorganized with new sections covering project architecture, development setup, and pull request processes. Additionally, an OAuth2 form field ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~18 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
CONTRIBUTING.md (1)
55-76: Move checklist items into a feature-specific root task-list markdown file.The checklist is useful, but this repo rule asks task lists to live in a dedicated root markdown file named for the feature.
As per coding guidelines, "Create task lists in a markdown file (in the project root): Use a descriptive name relevant to the feature (e.g.,
ASSISTANT_CHAT.md)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` around lines 55 - 76, Move the entire "Quick Start Checklist" section out of CONTRIBUTING.md into a new root markdown task-list file named for the feature (e.g., QUICK_START_CHECKLIST.md or a descriptive feature name like ASSISTANT_ONBOARDING.md) and keep the same headings and checklist items ("Before You Start", "For Code Contributions", "For Documentation Contributions"). In CONTRIBUTING.md, replace the removed checklist with a short link/reference to the new root file (e.g., "See QUICK_START_CHECKLIST.md for the checklist") so readers are pointed to the dedicated task-list file; ensure the new file retains the original checklist formatting and links.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CONTRIBUTING.md`:
- Around line 29-42: The fenced architecture tree in CONTRIBUTING.md is missing
a language identifier; update the code fence that wraps the directory tree (the
triple-backtick block showing "appsmith/ ├── app/ ...") to include a language
tag (e.g., ```text) so the renderer and linters recognize it as plain text and
maintain formatting.
---
Nitpick comments:
In `@CONTRIBUTING.md`:
- Around line 55-76: Move the entire "Quick Start Checklist" section out of
CONTRIBUTING.md into a new root markdown task-list file named for the feature
(e.g., QUICK_START_CHECKLIST.md or a descriptive feature name like
ASSISTANT_ONBOARDING.md) and keep the same headings and checklist items ("Before
You Start", "For Code Contributions", "For Documentation Contributions"). In
CONTRIBUTING.md, replace the removed checklist with a short link/reference to
the new root file (e.g., "See QUICK_START_CHECKLIST.md for the checklist") so
readers are pointed to the dedicated task-list file; ensure the new file retains
the original checklist formatting and links.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f2732efc-ed37-47f4-a61d-2771c869eacb
📒 Files selected for processing (2)
CONTRIBUTING.mdapp/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx
Comment on lines
+29
to
+42
| ``` | ||
| appsmith/ | ||
| ├── app/ | ||
| │ ├── client/ # Frontend (React + TypeScript) | ||
| │ │ ├── src/ # React components, widgets, utilities | ||
| │ │ ├── cypress/ # Cypress integration tests | ||
| │ │ └── packages/ # Shared packages including RTS (Real-Time Server) | ||
| │ └── server/ # Backend (Java + Spring + WebFlux) | ||
| │ ├── appsmith-server/ # Main server application | ||
| │ └── appsmith-plugins/ # Database/API connectors | ||
| ├── deploy/ # Docker & Kubernetes deployment configs | ||
| ├── contributions/ # Contribution guides and documentation | ||
| └── static/ # Static assets for documentation | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language identifier to the fenced code block.
The architecture tree block should declare a language (for lint compliance and renderer consistency).
Suggested fix
-```
+```text
appsmith/
├── app/
│ ├── client/ # Frontend (React + TypeScript)
│ │ ├── src/ # React components, widgets, utilities
│ │ ├── cypress/ # Cypress integration tests
│ │ └── packages/ # Shared packages including RTS (Real-Time Server)
│ └── server/ # Backend (Java + Spring + WebFlux)
│ ├── appsmith-server/ # Main server application
│ └── appsmith-plugins/ # Database/API connectors
├── deploy/ # Docker & Kubernetes deployment configs
├── contributions/ # Contribution guides and documentation
└── static/ # Static assets for documentation</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 29-29: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CONTRIBUTING.md` around lines 29 - 42, The fenced architecture tree in
CONTRIBUTING.md is missing a language identifier; update the code fence that
wraps the directory tree (the triple-backtick block showing "appsmith/ ├── app/
...") to include a language tag (e.g., ```text) so the renderer and linters
recognize it as plain text and maintain formatting.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR moves the
Authorization expires in (seconds)input field to appear right after theScope(s)field in the OAuth2 datasource configuration form.Problem
Previously, the
expires_infield was rendered after the Custom Authentication Parameters section, making it harder for users to discover this authentication-related field.Solution
expiresInfield fromrenderOauth2AuthorizationCode()torenderOauth2Common()scopeStringfield, beforeClient AuthenticationTesting
Related Issue
Fixes #31059
Summary by CodeRabbit
Documentation
Refactor