Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR discourse#36082

Type: Clean (correct implementation)

Original PR Title: UX: One step wizard
Original PR Description: Consolidates all three wizard screens into one. Cleans up some leftover, unused wizard elements (checkbox, image, canvas inputs).

Internal ticket: t/164852

CleanShot 2025-11-25 at 10 35 43@2x

Original PR URL: discourse#36082


PR Type

Enhancement


Description

  • Consolidates three-step wizard into single unified setup step

  • Combines introduction, privacy, and ready steps into one "setup" step

  • Removes unused wizard UI elements (step counter, sidebar, navigation buttons)

  • Simplifies frontend logic and styling for streamlined onboarding experience


Diagram Walkthrough

flowchart LR
  A["Three-step wizard<br/>introduction + privacy + ready"] -->|consolidate| B["Single setup step<br/>with all fields"]
  B -->|simplify UI| C["Streamlined onboarding<br/>direct to homepage"]
Loading

File Walkthrough

Relevant files
Enhancement
10 files
builder.rb
Merge three wizard steps into single setup step                   
+18/-63 
wizard_step_serializer.rb
Remove unused step serializer attributes                                 
+1/-26   
field.rb
Remove icon attribute from field and choice classes           
+2/-6     
wizard_field_serializer.rb
Remove unused field serializer attributes                               
+1/-35   
wizard_field_choice_serializer.rb
Remove icon attribute from choice serializer                         
+1/-9     
step_updater.rb
Remove refresh_required tracking from updater                       
+1/-6     
step.rb
Remove description_vars from step attributes                         
+1/-1     
wizard.js
Update wizard model for single setup step                               
+6/-10   
wizard-step.gjs
Simplify step component with single action flow                   
+28/-243
step.gjs
Simplify step template wrapper component                                 
+4/-57   
Tests
6 files
wizard_spec.rb
Update tests for consolidated single-step wizard                 
+32/-25 
step_updater_spec.rb
Refactor step updater tests for unified setup step             
+19/-39 
wizard_builder_spec.rb
Update builder specs for single setup step                             
+11/-29 
wizard_serializer_spec.rb
Update serializer tests for one-step wizard                           
+7/-7     
steps_controller_spec.rb
Update controller tests for setup step endpoint                   
+3/-8     
wizard-test.js
Remove unused test helper imports                                               
+1/-121 
Formatting
1 files
wizard.scss
Simplify wizard styles for single-step layout                       
+6/-227 
Documentation
2 files
server.en.yml
Update wizard step localization for setup step                     
+4/-95   
client.en.yml
Remove unused wizard UI localization strings                         
+1/-80   
Additional files
14 files
steps_controller.rb +0/-1     
checkbox.gjs +0/-37   
checkboxes.gjs +0/-55   
generic.gjs +0/-10   
index.js +0/-9     
logo-small.js +0/-94   
logo.js +0/-50   
image.gjs +0/-128 
index.js +0/-8     
-homepage-preview.js +0/-540 
-preview-base.gjs +0/-472 
index.gjs +0/-284 
wizard-canvas.gjs +0/-188 
wizard_step_spec.rb +0/-4     

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logs: The new unified wizard step updates multiple sensitive site settings (e.g., visibility and
locale) without adding any audit logging, which may be acceptable if handled elsewhere but
is not evident in the diff.

Referred Code
  type: "radio",
  value: SiteSetting.must_approve_users ? "yes" : "no",
) do |field|
  field.add_choice("no")
  field.add_choice("yes")
end

step.on_update do |updater|
  updater.ensure_changed(:title)

  updater.apply_settings(:title) if updater.errors.blank?

  old_locale = SiteSetting.default_locale
  updater.apply_setting(:default_locale)

  if old_locale != updater.fields[:default_locale]
    Scheduler::Defer.later "Reseed" do
      SeedData::Categories.with_default_locale.update(skip_changed: true)
      SeedData::Topics.with_default_locale.update(skip_changed: true)
    end
  end


 ... (clipped 5 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Weak client errors: The new single-step flow triggers a save and redirects on success but provides no visible
user feedback on validation failure beyond setting a flag, which may rely on external
components not shown.

Referred Code
async jumpIn() {
  const valid = await this.step.validate();
  if (!valid) {
    this.hasError = true;
    return;
  }

  const response = await this.step.save();
  if (response && response.success) {
    // We are not using Ember routing here because we always want to reload the app
    // ensure language, site title are properly set.
    document.location = getUrl("/");
  }
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input validation: The new setup step applies user-provided settings like title, default_locale, and access
controls with limited shown validation (only ensure_changed(:title)), and server-side
sanitization is not evident in the diff.

Referred Code
step.on_update do |updater|
  updater.ensure_changed(:title)

  updater.apply_settings(:title) if updater.errors.blank?

  old_locale = SiteSetting.default_locale
  updater.apply_setting(:default_locale)

  if old_locale != updater.fields[:default_locale]
    Scheduler::Defer.later "Reseed" do
      SeedData::Categories.with_default_locale.update(skip_changed: true)
      SeedData::Topics.with_default_locale.update(skip_changed: true)
    end
  end

  updater.update_setting(:login_required, updater.fields[:login_required] == "private")
  updater.update_setting(:invite_only, updater.fields[:invite_only] == "invite_only")
  updater.update_setting(:must_approve_users, updater.fields[:must_approve_users] == "yes")
end

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Handle save failures in wizard

Add error handling to the jumpIn action to provide user feedback when the save()
operation fails.

frontend/discourse/app/static/wizard/components/wizard-step.gjs [40-54]

 @action
 async jumpIn() {
   const valid = await this.step.validate();
   if (!valid) {
     this.hasError = true;
     return;
   }
 
   const response = await this.step.save();
   if (response && response.success) {
     // We are not using Ember routing here because we always want to reload the app
     // ensure language, site title are properly set.
     document.location = getUrl("/");
+  } else {
+    this.hasError = true;
   }
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a missing error handling path for save failures, which would leave the user without feedback, and proposes a valid fix.

Medium
  • More

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