FEATURE: Can edit category/host relationships for embedding

[hussam789]

User description

PR #10

---

PR Type

Enhancement

---

Description

• Migrate embeddable hosts from site settings to database model

• Add admin UI for managing embeddable hosts and categories

• Replace string-based host validation with database queries

• Update embed controller to use new EmbeddableHost model

---

Diagram Walkthrough

flowchart LR
  A["SiteSetting<br/>embeddable_hosts"] -->|migrate| B["EmbeddableHost<br/>Model"]
  B -->|admin CRUD| C["Admin UI<br/>Controllers"]
  C -->|serialize| D["EmbeddableHostSerializer"]
  E["EmbedController"] -->|validate| B
  F["TopicEmbed"] -->|lookup category| B

Loading

File Walkthrough

	Relevant files
Enhancement	20 files embeddable_host.rb New model for managing embeddable hosts                                    +24/-0    embeddable_hosts_controller.rb Admin controller for CRUD operations                                          +34/-0    embedding_controller.rb Admin controller for embedding settings                                    +21/-0    embeddable_host_serializer.rb Serializer for embeddable host API responses                          +16/-0    embedding_serializer.rb Serializer for embedding configuration                                      +8/-0      embed_controller.rb Update host validation to use model                                            +1/-2      site_setting.rb Remove embeddable host validation method                                  +0/-14    topic.rb Simplify expandable_first_post check                                          +1/-1      topic_embed.rb Use EmbeddableHost for category lookup                                      +3/-1      topic_retriever.rb Update to use EmbeddableHost validation                                    +1/-1      admin-embedding.js.es6 Ember controller for embedding admin UI                                    +18/-0    admin-embedding.js.es6 Ember route for embedding admin page                                          +9/-0      embeddable-host.js.es6 Ember component for host row editing                                          +63/-0    embedding.js.es6 REST adapter for embedding resource                                            +7/-0      embedding.hbs Template for embedding admin interface                                      +15/-0    embeddable-host.hbs Template for embeddable host row component                              +19/-0    customize.hbs Add embedding nav item to customize menu                                  +1/-0      admin-route-map.js.es6 Add embedding route to admin route map                                      +1/-0      rest.js.es6 Add embeddable-host to admin models list                                  +2/-2      store.js.es6 Support hydrating embedded arrays with IDs                              +14/-4
Configuration changes	3 files 20150818190757_create_embeddable_hosts.rb Migration to create embeddable_hosts table                              +33/-0    routes.rb Add routes for embedding admin pages                                          +3/-0      site_settings.yml Remove embeddable_hosts and embed_category settings            +0/-4
Documentation	2 files client.en.yml Add i18n strings for embedding UI                                                +8/-0      server.en.yml Remove deprecated site settings descriptions                          +0/-2
Tests	11 files embeddable_host_spec.rb Tests for EmbeddableHost model                                                      +40/-0    embeddable_hosts_controller_spec.rb Tests for embeddable hosts controller                                        +9/-0      embedding_controller_spec.rb Tests for embedding controller                                                      +9/-0      embed_controller_spec.rb Update tests to use EmbeddableHost model                                  +5/-6      site_setting_spec.rb Remove embeddable host validation tests                                    +0/-30    topic_embed_spec.rb Update tests for category lookup                                                  +3/-0      topic_spec.rb Update expandable_first_post tests                                              +22/-20  category_fabricator.rb Move category fabricator to separate file                                +3/-26    embeddable_host_fabricator.rb Add embeddable host fabricator                                                      +27/-0    create-pretender.js.es6 Add test data for embedded arrays                                                +9/-6      store-test.js.es6 Add tests for embedded array hydration                                      +20/-8

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	SQL injection Description: Raw SQL string interpolation inserts host values directly into an INSERT statement without sanitization, enabling SQL injection if site settings contain malicious input. 20150818190757_create_embeddable_hosts.rb [24-26] Referred Code records.each do |h| execute "INSERT INTO embeddable_hosts (host, category_id, created_at, updated_at) VALUES ('#{h}', #{category_id}, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)" end
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
	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
	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
🔴	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: SQL injection risk: User-controlled host values from site settings are interpolated directly into SQL INSERT without parameterization, introducing potential SQL injection during migration. Referred Code records.each do |h| execute "INSERT INTO embeddable_hosts (host, category_id, created_at, updated_at) VALUES ('#{h}', #{category_id}, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)" end
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing auditing: Creation, update, and deletion of embeddable hosts are not explicitly logged with user, action, and outcome details, which may hinder audit trail requirements. Referred Code def create save_host(EmbeddableHost.new) end def update host = EmbeddableHost.where(id: params[:id]).first save_host(host) end def destroy host = EmbeddableHost.where(id: params[:id]).first host.destroy render json: success_json end
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Edge case handling: The URL parsing in record_for_host returns false on invalid inputs but lacks logging/context and may not handle uppercase or trailing-dot hosts comprehensively. Referred Code def self.record_for_host(host) uri = URI(host) rescue nil return false unless uri.present? host = uri.host return false unless host.present? where("lower(host) = ?", host).first end def self.host_allowed?(host) record_for_host(host).present? end

Compliance status legend

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

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Use ActiveRecord to prevent SQLi Use a temporary ActiveRecord model for data insertion in the migration to prevent a potential SQL injection vulnerability from raw SQL string interpolation. db/migrate/20150818190757_create_embeddable_hosts.rb [24-26] -records.each do |h| - execute "INSERT INTO embeddable_hosts (host, category_id, created_at, updated_at) VALUES ('#{h}', #{category_id}, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)" +class TempEmbeddableHost < ActiveRecord::Base + self.table_name = :embeddable_hosts end +records.each do |h| + TempEmbeddableHost.create!(host: h, category_id: category_id) +end + Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a SQL injection vulnerability in the database migration and proposes a robust solution using a temporary ActiveRecord model, which is a Rails best practice for security.	High
High-level	Refactor data hydration for embedded arrays The data hydration logic for _ids arrays in store.js.es6 is too broad and risks causing unintended side effects. It should be refactored to be more specific or opt-in for models that require it. Examples: app/assets/javascripts/discourse/models/store.js.es6 [192-209] const m = /(.+)\_id(s?)$/.exec(k); if (m) { const subType = m[1]; if (m[2]) { const hydrated = obj[k].map(function(id) { return self._lookupSubType(subType, type, id, root); }); obj[self.pluralize(subType)] = hydrated || []; delete obj[k]; ... (clipped 8 lines) Solution Walkthrough: Before: // app/assets/javascripts/discourse/models/store.js.es6 _hydrateEmbedded(type, obj, root) { const self = this; Object.keys(obj).forEach(function(k) { // Regex only matches properties ending in `_id` const m = /(.+)\_id(s?)$/.exec(k); if (m) { if (m[2]) { // if it ends in `_ids` // ... logic to hydrate an array of IDs obj[self.pluralize(subType)] = hydrated || []; } else { // if it ends in `_id` // ... logic to hydrate a single ID } } }); } After: // app/assets/javascripts/discourse/models/store.js.es6 _hydrateEmbedded(type, obj, root) { const self = this; Object.keys(obj).forEach(function(k) { // Only hydrate `_ids` for models that have opted in if (/_ids$/.test(k) && self.modelFor(type).hasHydratableIds(k)) { // ... logic to hydrate an array of IDs } else if (/_id$/.test(k)) { // ... logic to hydrate a single ID } }); } // In a model definition (e.g., embedding.js.es6) export default Ember.Object.extend({ // Opt-in mechanism hasHydratableIds(key) { return key === 'embeddable_host_ids'; } }); Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a significant potential issue in store.js.es6, where a generic change to handle _ids properties could unintentionally affect other parts of the application, making it a valid and high-impact concern.	Medium
Possible issue	Filter out missing embedded records Add .filter(Boolean) after mapping embedded records to remove any null or undefined values that could cause runtime errors. app/assets/javascripts/discourse/models/store.js.es6 [196-202] if (m[2]) { const hydrated = obj[k].map(function(id) { return self._lookupSubType(subType, type, id, root); - }); + }).filter(Boolean); obj[self.pluralize(subType)] = hydrated || []; delete obj[k]; } else { Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that _lookupSubType can return null, leading to arrays with empty values, and proposes adding .filter(Boolean) to prevent potential runtime errors.	Medium
More