Skip to content

Fix issues with renaming a style with no ID#3120

Merged
Crabcyborg merged 2 commits into
masterfrom
fix_rename_a_style_with_no_id_does_not_work
Jun 3, 2026
Merged

Fix issues with renaming a style with no ID#3120
Crabcyborg merged 2 commits into
masterfrom
fix_rename_a_style_with_no_id_does_not_work

Conversation

@Crabcyborg

@Crabcyborg Crabcyborg commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

A style has no ID when it's new and hasn't been saved yet, or when you're duplicating a style.

Before
It would make a rename request with an empty style_id.

Screen Shot 2026-06-02 at 9 53 10 AM

Resulting in a Invalid route error.
Screen Shot 2026-06-02 at 9 53 34 AM

Screen.Recording.2026-06-02.at.9.52.15.AM.mov

After

Screen.Recording.2026-06-02.at.9.54.18.AM.mov

Summary by CodeRabbit

  • New Features
    • Improved style naming workflow when creating new styles or duplicating existing ones.
    • Optimized style renaming to process locally when appropriate, reducing unnecessary server requests.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a42b375c-8075-4943-8e42-505b8a928ff8

📥 Commits

Reviewing files that changed from the base of the PR and between 4fbc706 and 92bf568.

📒 Files selected for processing (1)
  • classes/controllers/FrmStylesController.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • classes/controllers/FrmStylesController.php

📝 Walkthrough

Walkthrough

JS rename now short-circuits for new/duplicated styles (no styleId), updating the posted title input and DOM locally; the PHP controller prefers the posted frm_style_setting[post_title] when rendering duplicate/new style pages.

Changes

New Style Rename Flow

Layer / File(s) Summary
Client-side short-circuit and server-side title consumption
js/admin/style.js, classes/controllers/FrmStylesController.php
renameStyle skips the rename_style AJAX call when styleId is falsy or '0', updates frm_style_setting[post_title] and frm_style_name in the DOM and shows the success notice. FrmStylesController::render_style_page() now prefers the posted frm_style_setting[post_title] (sanitized) for duplicate and new_style views, falling back to style_name.

Sequence Diagram(s)

sequenceDiagram
  participant RenameModal
  participant BrowserJS
  participant FrmStylesController
  RenameModal->>BrowserJS: submit rename (styleId missing or '0') with frm_style_setting[post_title]
  BrowserJS->>BrowserJS: set frm_style_setting[post_title] input\nupdate frm_style_name DOM\nshow success notice (no AJAX)
  BrowserJS->>FrmStylesController: later page render includes posted frm_style_setting[post_title]
  FrmStylesController->>FrmStylesController: sanitize and use posted title for $style->post_title
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • Strategy11/formidable-forms#3116: Modifies FrmStylesController::render_style_page() in duplicate/new_style flows to change how derived style names are computed and applied.

Suggested reviewers

  • garretlaxton

Poem

"I hopped upon the rename trail, no ID to see —
I nudged the title in the form, updated DOM with glee.
The server reads the posted text, sanitized and neat,
A quiet rename, no AJAX call — a rabbit's tidy feat!" 🐰

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing the issue where renaming a style without an ID fails. Both the PHP controller and JavaScript files implement client-side handling for styles with no ID, directly addressing the stated problem.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix_rename_a_style_with_no_id_does_not_work

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Crabcyborg Crabcyborg requested a review from garretlaxton June 2, 2026 12:55
@deepsource-io

deepsource-io Bot commented Jun 2, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in d176a4f...92bf568 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
PHP Jun 2, 2026 12:59p.m. Review ↗
JavaScript Jun 2, 2026 12:59p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

$posted_title = sanitize_text_field( wp_unslash( $_POST['frm_style_setting']['post_title'] ) );
}

$style->post_title = '' !== $posted_title ? $posted_title : FrmAppHelper::simple_get( 'style_name' );

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

@garretlaxton garretlaxton left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks, this fix works great!

@Crabcyborg

Copy link
Copy Markdown
Contributor Author

Thank you Garret!

🚀

@Crabcyborg Crabcyborg merged commit d5d5ed9 into master Jun 3, 2026
21 of 22 checks passed
@Crabcyborg Crabcyborg deleted the fix_rename_a_style_with_no_id_does_not_work branch June 3, 2026 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants