Skip to content

Fixed the bug 1563 and 1666#1686

Merged
johbaxter merged 3 commits intodevfrom
bug/notebook-cell-1563
Aug 13, 2025
Merged

Fixed the bug 1563 and 1666#1686
johbaxter merged 3 commits intodevfrom
bug/notebook-cell-1563

Conversation

@Gowrishankar-Palanisamy
Copy link
Copy Markdown
Contributor

This PR contains the following changes

--> Regarding 1563 - Made the default language as Python when the user delete the cell
--> Regarding 1666 - Updated the validation state functionality

@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

Fixed the bug 1563 and 1666


User description

This PR contains the following changes

--> Regarding 1563 - Made the default language as Python when the user delete the cell
--> Regarding 1666 - Updated the validation state functionality


PR Type

Bug fix, Enhancement


Description

  • Default new code cell type set to py

  • Fix password rules validation on input

  • Convert static password rules to function

  • Improve form validation robustness


Diagram Walkthrough

flowchart LR
  A["StateStore newCell default"] -- "type pixel -> py" --> B["Default Python cell"]
  C["LoginPage password rules"] -- "static object -> function" --> D["Per-input validation"]
  D -- "used in Controller validate" --> E["Accurate password feedback"]
Loading

File Walkthrough

Relevant files
Enhancement
state.store.ts
Default new cell language set to Python                                   

libs/renderer/src/store/state/state.store.ts

  • Change default new cell code type to py
  • Ensures new cells default to Python instead of Pixel
+1/-1     
Bug fix
LoginPage.tsx
Fix and refactor password validation logic                             

packages/client/src/pages/LoginPage.tsx

  • Replace static password rules object with function
  • Validate using current input value in Controller
  • Fix incorrect references to outdated rules state
+14/-11 

@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /review

@QodoAI-Agent
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Behavior Change

Default cell type switches from pixel to py when creating the fallback cell. Confirm this aligns with product expectations and does not break existing notebooks or parsing/rendering logic that assumes pixel as default.

this.newCell(
	queryId,
	{
		parameters: {
			code: "",
			type: "py",
		},
		widget: "code",
	} as Omit<CellStateConfig, "id">,
Validation Edge Case

Password validation now relies on a helper that checks for specials [!@#$%^&*]. Ensure this set matches backend rules and consider unicode or additional symbols if required to prevent mismatch between client and server validations.

required:
	"Password is required",
validate: (value) => {
	const rules = regPasswordRules(value);
	if (
		!rules.length
	)
		return "Minimum 8 characters";
	if (
		!rules.upper
	)
		return "At least one uppercase letter";
	if (
		!rules.lower
	)
		return "At least one lowercase letter";
	if (
		!rules.special
	)
		return "At least one special character";
	return true;
Performance/Memoization

regPasswordRules is recreated each render and regexes run on every keystroke in validation. Consider memoizing or inlining minimal checks, and ensure the tooltip or other consumers use the same logic to avoid drift.

const regPasswordRules = (password: string) => {
	return {
		length: password.length >= 8,
		upper: /[A-Z]/.test(password),
		lower: /[a-z]/.test(password),
		special: /[!@#$%^&*]/.test(password),
	};
}

@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /improve

@QodoAI-Agent
Copy link
Copy Markdown

QodoAI-Agent commented Aug 12, 2025

PR Code Suggestions ✨

Latest suggestions up to a037b10

CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent undefined password access

Avoid calling .length on possibly undefined values. Provide a default empty string
to prevent runtime errors during initial render or when the field is cleared.

packages/client/src/pages/LoginPage.tsx [280-287]

 const regPasswordRules = (password: string) => {
+	const pwd = password ?? "";
 	return {
-		length: password.length >= 8,
-		upper: /[A-Z]/.test(password),
-		lower: /[a-z]/.test(password),
-		special: /[!@#$%^&*]/.test(password),
+		length: pwd.length >= 8,
+		upper: /[A-Z]/.test(pwd),
+		lower: /[a-z]/.test(pwd),
+		special: /[!@#$%^&*]/.test(pwd),
 	};
 }
Suggestion importance[1-10]: 7

__

Why: Wrapping the password with a nullish coalescing default avoids runtime errors if value is undefined, aligning with the new function signature; it’s a solid defensive improvement with moderate impact.

Medium
Normalize validator input safely

Ensure value is a string to avoid passing null/undefined to the validator. Normalize
the input to an empty string before validation to prevent crashes on empty or
uninitialized values.

packages/client/src/pages/LoginPage.tsx [953-972]

 validate: (value) => {
-	const rules = regPasswordRules(value);
+	const rules = regPasswordRules(value ?? "");
 	if (!rules.length) return "Minimum 8 characters";
 	if (!rules.upper) return "At least one uppercase letter";
 	if (!rules.lower) return "At least one lowercase letter";
 	if (!rules.special) return "At least one special character";
 	return true;
 },
Suggestion importance[1-10]: 7

__

Why: Normalizing value with ?? "" in the validator prevents passing undefined to regPasswordRules and improves robustness without altering behavior, offering a practical, low-risk enhancement.

Medium
Guard against breaking type change

Verify that the consumer logic expects the updated type value. If existing code
still branches on "pixel", this change will break default cell creation. Consider
keeping compatibility by mapping or falling back when the type is unrecognized.

libs/renderer/src/store/state/state.store.ts [1742]

-type: "py",
+type: "py", // TODO: ensure downstream logic supports "py" (was "pixel"); consider mapping for backward compatibility
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly flags a potentially breaking change from type: "pixel" to type: "py". It’s contextually relevant but advisory (verification/todo) rather than a concrete fix, so impact is moderate.

Low

Previous suggestions

Suggestions up to commit a037b10
CategorySuggestion                                                                                                                                    Impact
Possible issue
Add input type guarding

Guard against non-string value in validation to prevent runtime errors when the
field is undefined or not yet controlled. Coerce to string before checks to ensure
regex and length tests are safe.

packages/client/src/pages/LoginPage.tsx [950-972]

-control={
-	registerControl
-}
+control={registerControl}
 rules={{
-	required:
-		"Password is required",
-	validate: (value) => {
-		const rules = regPasswordRules(value);
-		if (
-			!rules.length
-		)
-			return "Minimum 8 characters";
-		if (
-			!rules.upper
-		)
-			return "At least one uppercase letter";
-		if (
-			!rules.lower
-		)
-			return "At least one lowercase letter";
-		if (
-			!rules.special
-		)
-			return "At least one special character";
-		return true;
-	},
+  required: "Password is required",
+  validate: (value) => {
+    const pwd = typeof value === "string" ? value : "";
+    const rules = regPasswordRules(pwd);
+    if (!rules.length) return "Minimum 8 characters";
+    if (!rules.upper) return "At least one uppercase letter";
+    if (!rules.lower) return "At least one lowercase letter";
+    if (!rules.special) return "At least one special character";
+    return true;
+  },
 }}
Suggestion importance[1-10]: 7

__

Why: Adding a type guard for value improves robustness against undefined/non-string inputs and prevents regex/length runtime errors; relevant and accurate to the new validation logic.

Medium
Use canonical engine identifier

Verify that the engine type string matches accepted values; changing from "pixel" to
"py" can break creation if "py" is unrecognized. If the system expects "python", use
that canonical identifier to avoid runtime errors.

libs/renderer/src/store/state/state.store.ts [1737-1746]

 this.newCell(
 	queryId,
 	{
 		parameters: {
 			code: "",
-			type: "py",
+			type: "python",
 		},
 		widget: "code",
 	} as Omit<CellStateConfig, "id">,
 	"",
Suggestion importance[1-10]: 3

__

Why: The suggestion raises a valid concern but lacks evidence from the diff that "python" is the correct canonical type; it's speculative and proposes a change that may be incorrect.

Low
General
Remove unused watched value

Remove the unused regPassword to avoid stale reads and unnecessary re-renders.
Keeping only the pure function regPasswordRules prevents potential runtime issues
when regPassword is undefined during initial render.

packages/client/src/pages/LoginPage.tsx [278-287]

-const regPassword = watch("PASSWORD");
-
 const regPasswordRules = (password: string) => {
 	return {
 		length: password.length >= 8,
 		upper: /[A-Z]/.test(password),
 		lower: /[a-z]/.test(password),
 		special: /[!@#$%^&*]/.test(password),
 	};
-}
+};
Suggestion importance[1-10]: 5

__

Why: regPassword is no longer used after introducing regPasswordRules(value) and can be removed to avoid unnecessary watches; straightforward cleanup with low risk.

Low
Suggestions up to commit 6dbf889
CategorySuggestion                                                                                                                                    Impact
Possible issue
Restore compatible default type

Ensure the type value matches the accepted enum/string literals used elsewhere
(previously "pixel"). If downstream logic still expects "pixel", this change could
break default cell creation. Either map "py" to the correct internal type or keep
the original value to avoid runtime errors.

libs/renderer/src/store/state/state.store.ts [1740-1743]

 parameters: {
   code: "",
-  type: "py",
+  type: "pixel",
 },
Suggestion importance[1-10]: 7

__

Why: If other parts of the app still expect type: "pixel", changing to "py" could break default cell behavior. The suggestion is context-aware and could prevent runtime issues, though it depends on broader app expectations.

Medium
Normalize password input

Guard against undefined by normalizing password to a string; some form libraries can
pass undefined initially. Without this, calling .length or .test could throw at
runtime. Default to an empty string to keep validation stable.

packages/client/src/pages/LoginPage.tsx [280-287]

 const regPasswordRules = (password: string) => {
+	const pwd = typeof password === "string" ? password : "";
 	return {
-		length: password.length >= 8,
-		upper: /[A-Z]/.test(password),
-		lower: /[a-z]/.test(password),
-		special: /[!@#$%^&*]/.test(password),
+		length: pwd.length >= 8,
+		upper: /[A-Z]/.test(pwd),
+		lower: /[a-z]/.test(pwd),
+		special: /[!@#$%^&*]/.test(pwd),
 	};
 }
Suggestion importance[1-10]: 6

__

Why: Guarding against non-string values in regPasswordRules improves robustness and avoids potential runtime errors, especially during initial render. It's a reasonable defensive improvement with moderate impact.

Low
General
Trim input before checks

Trim whitespace before validation to prevent passwords that only meet rules via
leading/trailing spaces. This avoids users bypassing length checks with spaces and
keeps validation consistent with server-side norms.

packages/client/src/pages/LoginPage.tsx [953-972]

 validate: (value) => {
-	const rules = regPasswordRules(value);
-	if (!rules.length)
-		return "Minimum 8 characters";
-	if (!rules.upper)
-		return "At least one uppercase letter";
-	if (!rules.lower)
-		return "At least one lowercase letter";
-	if (!rules.special)
-		return "At least one special character";
+	const v = (typeof value === "string" ? value : "").trim();
+	const rules = regPasswordRules(v);
+	if (!rules.length) return "Minimum 8 characters";
+	if (!rules.upper) return "At least one uppercase letter";
+	if (!rules.lower) return "At least one lowercase letter";
+	if (!rules.special) return "At least one special character";
 	return true;
 },
Suggestion importance[1-10]: 6

__

Why: Trimming the password before validation prevents bypassing length checks with whitespace and aligns with common server-side validation, offering a sensible, low-risk enhancement.

Low

@Abhisheksaini129QA
Copy link
Copy Markdown

Hi @Gowrishankar-Palanisamy, I have review asked changes above, here is my observation -

  1. User is able to see the python as default option in the cell and after deleting the cell, it remains python as default.
  2. Password inline error is resolved for matched password check and creating native user successfully.
    video attached for better reference
    CC- @rameshpaulraj @Pranchaudhari

@Gowrishankar-Palanisamy Gowrishankar-Palanisamy marked this pull request as ready for review August 13, 2025 08:16
@Gowrishankar-Palanisamy Gowrishankar-Palanisamy requested a review from a team as a code owner August 13, 2025 08:16
Copy link
Copy Markdown
Contributor

@johbaxter johbaxter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is still a bug in this direction

  1. with the dropdown to switch languages when you
  2. when you delete cell the id disappers for the new cell that we add

@Gowrishankar-Palanisamy and @ehynd

@johbaxter
Copy link
Copy Markdown
Contributor

#1705

Follow up bug ticket

@johbaxter johbaxter merged commit c0e19af into dev Aug 13, 2025
3 checks passed
@johbaxter johbaxter deleted the bug/notebook-cell-1563 branch August 13, 2025 20:58
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /update_changelog

@QodoAI-Agent
Copy link
Copy Markdown

Changelog updates: 🔄

2025-08-13 *

Changed

  • Refined password validation logic for registration.

Fixed

  • Default notebook cell language set to Python after cell deletion.

to commit the new content to the CHANGELOG.md file, please type:
'/update_changelog --pr_update_changelog.push_changelog_changes=true'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect validation Native Register Notebook cell default is not consistent

4 participants