Sets the script to the default script instead of an empty string when reseting adaptive script#9740
Sets the script to the default script instead of an empty string when reseting adaptive script#9740HasiniSama wants to merge 3 commits intowso2:masterfrom
Conversation
… reseting adaptive script
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdated copyright year and changed adaptive script reset to populate a generated default script (based on current authentication steps) instead of clearing the source; added a patch changeset documenting this behavioral change and minor manifest edits. Changes
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@features/admin.applications.v1/components/settings/sign-on-methods/script-based-flow/script-based-flow.tsx`:
- Around line 567-571: resetAdaptiveScriptTemplateToDefaultHandler currently
only updates sourceCode, leaving internalScript populated so
resolveAdaptiveScript() can later prefer the old value; update the handler to
also clear/overwrite the internal editor buffer (e.g., call
setInternalScript(null) or setInternalScript(generatedScript)) after generating
the default via AdaptiveScriptUtils.generateScript(authenticationSteps + 1) so
resolveAdaptiveScript() will no longer restore the previous custom script.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: aaae9086-e3db-4776-9be3-6a9c89351b03
📒 Files selected for processing (1)
features/admin.applications.v1/components/settings/sign-on-methods/script-based-flow/script-based-flow.tsx
....applications.v1/components/settings/sign-on-methods/script-based-flow/script-based-flow.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/nasty-garlics-jog.md:
- Line 6: Change the typo "reseting" to "resetting" in the changeset note text
inside .changeset/nasty-garlics-jog.md so the user-facing release note reads
"Sets the script to the default script instead of an empty string when resetting
adaptive script"; update only the word in that sentence (leave surrounding
wording intact).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 25d773a5-7777-4bd1-90a6-9ee324840844
📒 Files selected for processing (1)
.changeset/nasty-garlics-jog.md
|
|
||
| const resetAdaptiveScriptTemplateToDefaultHandler = () => { | ||
| setSourceCode(""); | ||
| setSourceCode(AdaptiveScriptUtils.generateScript(authenticationSteps + 1)); |
There was a problem hiding this comment.
Are we expecting to save the default script when disabling the conditional authentication?
There was a problem hiding this comment.
This was the previous default behavior for the Classic Editor. It was changed when introducing the Script Update REST API feature [1]. Since this feature is disabled by default, this change introduced a regression in the Classic Editor.
Additionally, since the previous behavior aligns with the Visual Editor, we propose reverting to the earlier implementation to avoid breaking the current flow.
[1] a9c5cf7#diff-6eb727e98b131f02e3e21db95b23fadb0fe174733537a3a163a1e5e0e7faa5c7R579
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9740 +/- ##
=======================================
Coverage 56.01% 56.01%
=======================================
Files 42 42
Lines 1023 1023
Branches 231 246 +15
=======================================
Hits 573 573
+ Misses 450 416 -34
- Partials 0 34 +34
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Purpose
$subject
Changes introdcued:
Issues: wso2/product-is#26991