fix: conditionally show LLM API key - #4220
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe course creation form now displays the LLM API key field only when LLM grading is enabled. The field remains optional, uses password masking, and updates ChangesCourse creation form
Estimated code review effort: 1 (Trivial) | ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commons/dropdown/DropdownCreateCourse.tsx`:
- Around line 263-282: Update the LLM grading toggle handling in
DropdownCreateCourse and the submitHandler payload so llmApiKey is cleared or
omitted whenever enableLlmGrading is false, preventing a previously entered key
from being dispatched. Add a test covering entering llmApiKey, disabling LLM
grading, and submitting the configuration.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d03c9c15-f097-497a-a7ae-e6eabb5d54f2
📒 Files selected for processing (1)
src/commons/dropdown/DropdownCreateCourse.tsx
| {courseConfig.enableLlmGrading && ( | ||
| <FormGroup | ||
| helperText="API Key for LLM endpoint. This key will be encrypted and will not be retrievable on the frontend after." | ||
| label={'LLM API Key'} | ||
| labelInfo="(optional)" | ||
| labelFor="llmApiKey" | ||
| > | ||
| <InputGroup | ||
| id="llmApiKey" | ||
| type="password" | ||
| value={courseConfig.llmApiKey} | ||
| onChange={e => | ||
| setCourseConfig({ | ||
| ...courseConfig, | ||
| llmApiKey: e.target.value, | ||
| }) | ||
| } | ||
| /> | ||
| </FormGroup> | ||
| )} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not submit the API key when LLM grading is disabled.
After a user enters llmApiKey and disables LLM grading, the key remains in courseConfig. submitHandler dispatches the entire configuration, so hiding this input does not prevent the key from being submitted. Clear llmApiKey when disabling LLM grading, or omit it from the payload when enableLlmGrading is false. Add a test for this sequence.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commons/dropdown/DropdownCreateCourse.tsx` around lines 263 - 282, Update
the LLM grading toggle handling in DropdownCreateCourse and the submitHandler
payload so llmApiKey is cleared or omitted whenever enableLlmGrading is false,
preventing a previously entered key from being dispatched. Add a test covering
entering llmApiKey, disabling LLM grading, and submitting the configuration.
Coverage Report for CI Build 30694647102Coverage increased (+0.03%) to 44.738%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
I thought the plan we agreed on was just to remove the option to enable/disable from the create course dropdown? In that case course admins would still be able to enable it afterwards, and then add in the required API key. |
3 participants
Summary
llmApiKeywas being dispatched with the create-course payload even after the user toggled LLM grading off. Hiding the input was purely a UI concern.DropdownCreateCourse.submitHandlernow omitsllmApiKeyfrom the payload whenenableLlmGradingisfalse.DropdownCreateCourse.test.tsxcovering:Validation
yarn tsc -bcleanyarn eslint src/commons/dropdown/DropdownCreateCourse.{tsx,test.tsx}cleanyarn test— 91 files / 748 tests pass