FEATURE: Can edit category/host relationships for embedding #10

everettbu · 2025-07-26T19:33:33Z

Test 10

Summary by CodeRabbit

New Features
- Introduced a new admin interface for managing embeddable hosts, including adding, editing, and deleting allowed hosts and assigning them to categories.
- Added new API endpoints and admin navigation for embedding settings.
- Created new models, controllers, serializers, and database tables to support embeddable host management.
Improvements
- Embeddable host validation and assignment are now handled via dedicated records rather than site settings.
- Category assignment for embedded posts is now dynamic based on the host.
Bug Fixes
- Enhanced hydration logic to support plural relationships in data models.
Documentation
- Added and updated translation strings for new embedding features.
Tests
- Added and updated tests for new models, controllers, and embedding logic.
Chores
- Removed obsolete site settings and related code for embedding hosts and categories.

coderabbitai · 2025-07-26T19:33:42Z

Walkthrough

This change set introduces a new embeddable hosts management feature in the admin interface. It adds new models, controllers, serializers, database migrations, and JavaScript components to support CRUD operations for embeddable hosts, replacing previous site setting-based logic. Associated tests, routes, and localization entries are updated or added accordingly.

Changes

Cohort / File(s)	Change Summary
Admin Embedding JS Feature `app/assets/javascripts/admin/adapters/embedding.js.es6`, `app/assets/javascripts/admin/components/embeddable-host.js.es6`, `app/assets/javascripts/admin/controllers/admin-embedding.js.es6`, `app/assets/javascripts/admin/routes/admin-embedding.js.es6`, `app/assets/javascripts/admin/routes/admin-route-map.js.es6`, `app/assets/javascripts/admin/templates/components/embeddable-host.hbs`, `app/assets/javascripts/admin/templates/customize.hbs`, `app/assets/javascripts/admin/templates/embedding.hbs`	Adds adapter, component, controller, route, templates, and navigation for managing embeddable hosts in the admin UI.
REST Adapter & Model Hydration `app/assets/javascripts/discourse/adapters/rest.js.es6`, `app/assets/javascripts/discourse/models/store.js.es6`	Updates admin model normalization and enhances embedded array hydration for related objects.
Backend: Controllers & Serializers `app/controllers/admin/embeddable_hosts_controller.rb`, `app/controllers/admin/embedding_controller.rb`, `app/serializers/embeddable_host_serializer.rb`, `app/serializers/embedding_serializer.rb`	Introduces new controllers and serializers for embeddable hosts and embedding, supporting CRUD and data serialization.
Backend: Models & Logic `app/models/embeddable_host.rb`, `app/models/topic_embed.rb`, `app/models/topic.rb`	Adds `EmbeddableHost` model, updates topic embedding logic to use per-host categories, and removes embeddable hosts check from topic expansion.
Legacy Logic Removal `app/models/site_setting.rb`, `config/site_settings.yml`, `config/locales/server.en.yml`	Removes embeddable host and embed category site settings and related methods.
Routing `config/routes.rb`	Adds admin routes for embedding management and embeddable hosts resources.
Database Migration `db/migrate/20150818190757_create_embeddable_hosts.rb`	Creates `embeddable_hosts` table and migrates data from old site settings.
Localization `config/locales/client.en.yml`	Adds new translation keys for embedding UI.
Embed Controller & Topic Retriever `app/controllers/embed_controller.rb`, `lib/topic_retriever.rb`	Refactors host validation to use the new `EmbeddableHost` model.
Specs: Controller & Model `spec/controllers/admin/embeddable_hosts_controller_spec.rb`, `spec/controllers/admin/embedding_controller_spec.rb`, `spec/controllers/embed_controller_spec.rb`, `spec/models/embeddable_host_spec.rb`, `spec/models/topic_embed_spec.rb`, `spec/models/topic_spec.rb`	Adds and updates tests for new controllers, models, and embedding logic.
Specs: Fabricators `spec/fabricators/category_fabricator.rb`, `spec/fabricators/embeddable_host_fabricator.rb`	Replaces category fabricators with embeddable host fabricator and moves category fabricators to a new file.
Specs: Site Setting `spec/models/site_setting_spec.rb`	Removes tests for deleted `allows_embeddable_host?` method.
JS Store and Test Helpers `test/javascripts/helpers/create-pretender.js.es6`, `test/javascripts/models/store-test.js.es6`	Updates test data and assertions to support embedded array hydration and related color associations.

Sequence Diagram(s)

sequenceDiagram
    participant AdminUser
    participant AdminUI (Ember)
    participant API
    participant EmbeddableHost Model
    participant Database

    AdminUser->>AdminUI (Ember): Navigates to Embedding Admin Page
    AdminUI (Ember)->>API: GET /admin/customize/embedding
    API->>EmbeddableHost Model: Fetch all embeddable hosts
    EmbeddableHost Model->>Database: SELECT * FROM embeddable_hosts
    Database-->>EmbeddableHost Model: Hosts data
    EmbeddableHost Model-->>API: Hosts data
    API-->>AdminUI (Ember): JSON response with hosts
    AdminUI (Ember)-->>AdminUser: Renders host list

    AdminUser->>AdminUI (Ember): Adds/Edits/Deletes Host
    AdminUI (Ember)->>API: POST/PUT/DELETE /admin/embeddable_hosts
    API->>EmbeddableHost Model: Create/Update/Delete host
    EmbeddableHost Model->>Database: INSERT/UPDATE/DELETE
    Database-->>EmbeddableHost Model: Result
    EmbeddableHost Model-->>API: Updated hosts
    API-->>AdminUI (Ember): JSON response
    AdminUI (Ember)-->>AdminUser: Updates UI

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Poem

In the warren of code, a new path appears,
Hosts embeddable, managed without fears.
From settings to models, the logic now flows,
With tables and routes where the new data grows.
A hop, a skip, and tests all in line—
This rabbit’s quite proud of the embedding design! 🐇✨

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.

✨ Finishing Touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch rest-serializer-enhancement-post

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai generate unit tests to generate unit tests for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

everettbu · 2025-07-29T16:44:18Z

@coderabbitai review

coderabbitai · 2025-07-29T16:44:25Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2025-07-29T16:46:00Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 12

🧹 Nitpick comments (14)

spec/controllers/admin/embedding_controller_spec.rb (1)
4-4: Fix RuboCop style violations.

Remove the extra empty lines at the beginning and end of the describe block.
 describe Admin::EmbeddingController do
-
   it "is a subclass of AdminController" do
     expect(Admin::EmbeddingController < Admin::AdminController).to eq(true)
   end
-
 end
Also applies to: 8-8
spec/controllers/admin/embeddable_hosts_controller_spec.rb (1)
4-4: Fix RuboCop style violations.

Remove the extra empty lines consistent with the previous file.
 describe Admin::EmbeddableHostsController do
-
   it "is a subclass of AdminController" do
     expect(Admin::EmbeddableHostsController < Admin::AdminController).to eq(true)
   end
-
 end
Also applies to: 8-8
config/locales/client.en.yml (1)

2494-2501: Indentation & namespace sanity-check for the new embedding keys

The new embedding: block is added as a sibling to emoji: under admin_js.

Ensure the 6-space indentation level is intentional so the full i18n path becomes js.admin_js.embedding.*.

Run rake i18n:js:export (or the project-specific task) to verify there’s no collision with an existing admin_js.embedding namespace coming from plugins or core.

Because YAML has no ordering guarantee, keeping related admin sections alphabetically sorted helps future merges.

No functional changes required—just make sure the file still passes yamllint and that the keys load in the Ember templates.

app/assets/javascripts/admin/routes/admin-embedding.js.es6 (1)

1-9: Unable to determine Ember Data version—please verify
I couldn’t locate your Ember or Ember Data versions in the repo. To confirm whether store.find() is deprecated (and should be replaced with store.findAll() or another modern API), please check your project’s Ember Data version (e.g. in package.json, yarn.lock, or ember-cli-build.js).
app/serializers/embeddable_host_serializer.rb (1)
1-16: Simplify by removing redundant attribute methods.

The serializer implementation is correct but can be simplified. ActiveModel::Serializer automatically creates methods for declared attributes, so the explicit methods are redundant.
 class EmbeddableHostSerializer < ApplicationSerializer
   attributes :id, :host, :category_id
-
-  def id
-    object.id
-  end
-
-  def host
-    object.host
-  end
-
-  def category_id
-    object.category_id
-  end
 end
The simplified version will have identical functionality since the serializer framework automatically creates these accessor methods based on the attributes declaration.
app/serializers/embedding_serializer.rb (1)
1-8: Consider removing redundant id method.

The id method on lines 5-7 is redundant since id is already declared in the attributes list on line 2. ActiveModelSerializers automatically handles the id attribute.
-  def id
-    object.id
-  end
spec/models/embeddable_host_spec.rb (1)
3-40: Fix formatting issues identified by RuboCop.

Remove extra empty lines in block bodies for consistent formatting.
 describe EmbeddableHost do
-
   it "trims http" do
     eh = EmbeddableHost.new(host: 'http://example.com')
     expect(eh).to be_valid
     expect(eh.host).to eq('example.com')
   end

   it "trims https" do
     eh = EmbeddableHost.new(host: 'https://example.com')
     expect(eh).to be_valid
     expect(eh.host).to eq('example.com')
   end

   it "trims paths" do
     eh = EmbeddableHost.new(host: 'https://example.com/1234/45')
     expect(eh).to be_valid
     expect(eh.host).to eq('example.com')
   end

   describe "allows_embeddable_host" do
     let!(:host) { Fabricate(:embeddable_host) }

     it 'works as expected' do
       expect(EmbeddableHost.host_allowed?('http://eviltrout.com')).to eq(true)
       expect(EmbeddableHost.host_allowed?('https://eviltrout.com')).to eq(true)
       expect(EmbeddableHost.host_allowed?('https://not-eviltrout.com')).to eq(false)
     end

     it 'works with multiple hosts' do
       Fabricate(:embeddable_host, host: 'discourse.org')
       expect(EmbeddableHost.host_allowed?('http://eviltrout.com')).to eq(true)
       expect(EmbeddableHost.host_allowed?('http://discourse.org')).to eq(true)
     end
-
   end
-
 end
app/assets/javascripts/admin/templates/embedding.hbs (1)
1-12: Consider moving inline styles to CSS classes for better maintainability.

The inline style attributes on table headers work but make styling harder to maintain and override. Consider defining CSS classes instead.
    <tr>
-      <th style='width: 50%'>{{i18n "admin.embedding.host"}}</th>
-      <th style='width: 30%'>{{i18n "admin.embedding.category"}}</th>
-      <th style='width: 20%'>&nbsp;</th>
+      <th class="host-column">{{i18n "admin.embedding.host"}}</th>
+      <th class="category-column">{{i18n "admin.embedding.category"}}</th>
+      <th class="actions-column">&nbsp;</th>
    </tr>
Then define the widths in your CSS file:
.host-column { width: 50%; }
.category-column { width: 30%; }
.actions-column { width: 20%; }
app/assets/javascripts/admin/templates/components/embeddable-host.hbs (1)
3-3: Consider adding accessibility attributes to the input field.

For better accessibility, consider adding an aria-label or associating the input with a label.
-    {{input value=buffered.host placeholder="example.com" enter="save" class="host-name"}}
+    {{input value=buffered.host placeholder="example.com" enter="save" class="host-name" aria-label="Host name"}}
app/controllers/admin/embedding_controller.rb (1)
15-20: Consider using a proper model instead of OpenStruct.

Using OpenStruct with a hardcoded ID for the embedding object is unconventional. Consider creating a proper model or service object for better maintainability.
  def fetch_embedding
-    @embedding = OpenStruct.new({
-      id: 'default',
-      embeddable_hosts: EmbeddableHost.all.order(:host)
-    })
+    @embedding = Embedding.new # or create a proper Embedding model/service
  end
Alternatively, if this is truly just a container for embeddable_hosts, document why OpenStruct is used here.
app/models/embeddable_host.rb (1)
10-18: Consider using conventional rescue syntax for better readability.

The method logic is solid with proper URI parsing and case-insensitive lookup. However, the rescue modifier form flagged by static analysis could be improved for better readability.

Apply this diff to use conventional rescue syntax:
  def self.record_for_host(host)
-   uri = URI(host) rescue nil
+   begin
+     uri = URI(host)
+   rescue
+     uri = nil
+   end
    return false unless uri.present?
db/migrate/20150818190757_create_embeddable_hosts.rb (1)
2-32: Consider breaking down the complex migration method.

The static analysis tool flagged the high ABC complexity (24.43/23). The method handles multiple concerns: table creation, data migration, and cleanup.

Consider extracting helper methods:
def change
  create_embeddable_hosts_table
  migrate_existing_data
  cleanup_old_settings
end

private

def create_embeddable_hosts_table
  create_table :embeddable_hosts, force: true do |t|
    t.string :host, null: false
    t.integer :category_id, null: false
    t.timestamps
  end
end

def migrate_existing_data
  category_id = resolve_category_id
  migrate_embeddable_hosts(category_id) if category_id > 0
end

def cleanup_old_settings
  execute "DELETE FROM site_settings WHERE name IN ('embeddable_hosts', 'embed_category')"
end
spec/models/topic_spec.rb (1)
1407-1407: Fix typo in context description.
-    describe 'with an emeddable host' do
+    describe 'with an embeddable host' do
app/assets/javascripts/admin/components/embeddable-host.js.es6 (1)
30-41: Consider improving category handling in save action.

The save action works correctly but has a potential issue with direct model manipulation.
      const host = this.get('host');
      host.save(props).then(() => {
-        host.set('category', Discourse.Category.findById(this.get('categoryId')));
+        // Let the model handle category relationship updates through its response
+        // or refresh the model to get updated relationships
        this.set('editToggled', false);
      }).catch(popupAjaxError);
Alternatively, ensure the server response includes the updated category relationship to avoid manual model manipulation.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 913c3d6 and d1c6918.

📒 Files selected for processing (36)

app/assets/javascripts/admin/adapters/embedding.js.es6 (1 hunks)
app/assets/javascripts/admin/components/embeddable-host.js.es6 (1 hunks)
app/assets/javascripts/admin/controllers/admin-embedding.js.es6 (1 hunks)
app/assets/javascripts/admin/routes/admin-embedding.js.es6 (1 hunks)
app/assets/javascripts/admin/routes/admin-route-map.js.es6 (1 hunks)
app/assets/javascripts/admin/templates/components/embeddable-host.hbs (1 hunks)
app/assets/javascripts/admin/templates/customize.hbs (1 hunks)
app/assets/javascripts/admin/templates/embedding.hbs (1 hunks)
app/assets/javascripts/discourse/adapters/rest.js.es6 (2 hunks)
app/assets/javascripts/discourse/models/store.js.es6 (1 hunks)
app/controllers/admin/embeddable_hosts_controller.rb (1 hunks)
app/controllers/admin/embedding_controller.rb (1 hunks)
app/controllers/embed_controller.rb (1 hunks)
app/models/embeddable_host.rb (1 hunks)
app/models/site_setting.rb (0 hunks)
app/models/topic.rb (1 hunks)
app/models/topic_embed.rb (1 hunks)
app/serializers/embeddable_host_serializer.rb (1 hunks)
app/serializers/embedding_serializer.rb (1 hunks)
config/locales/client.en.yml (1 hunks)
config/locales/server.en.yml (0 hunks)
config/routes.rb (2 hunks)
config/site_settings.yml (0 hunks)
db/migrate/20150818190757_create_embeddable_hosts.rb (1 hunks)
lib/topic_retriever.rb (1 hunks)
spec/controllers/admin/embeddable_hosts_controller_spec.rb (1 hunks)
spec/controllers/admin/embedding_controller_spec.rb (1 hunks)
spec/controllers/embed_controller_spec.rb (3 hunks)
spec/fabricators/category_fabricator.rb (1 hunks)
spec/fabricators/embeddable_host_fabricator.rb (1 hunks)
spec/models/embeddable_host_spec.rb (1 hunks)
spec/models/site_setting_spec.rb (0 hunks)
spec/models/topic_embed_spec.rb (2 hunks)
spec/models/topic_spec.rb (3 hunks)
test/javascripts/helpers/create-pretender.js.es6 (2 hunks)
test/javascripts/models/store-test.js.es6 (1 hunks)

💤 Files with no reviewable changes (4)

config/site_settings.yml
app/models/site_setting.rb
spec/models/site_setting_spec.rb
config/locales/server.en.yml

🧰 Additional context used

🧬 Code Graph Analysis (10)

app/models/topic_embed.rb (1)

app/models/embeddable_host.rb (1)

record_for_host (10-18)

app/models/embeddable_host.rb (1)

app/models/topic_embed.rb (1)

belongs_to (3-163)

spec/models/embeddable_host_spec.rb (1)

app/models/embeddable_host.rb (1)

host_allowed? (20-22)

app/serializers/embeddable_host_serializer.rb (1)

app/serializers/embedding_serializer.rb (2)

attributes (1-8)

id (5-7)

app/serializers/embedding_serializer.rb (1)

app/serializers/embeddable_host_serializer.rb (2)

attributes (1-15)

id (4-6)

app/controllers/embed_controller.rb (1)

app/models/embeddable_host.rb (1)

host_allowed? (20-22)

spec/models/topic_spec.rb (1)

app/models/topic.rb (1)

expandable_first_post? (868-870)

app/controllers/admin/embedding_controller.rb (2)

app/controllers/admin/embeddable_hosts_controller.rb (2)

before_filter (1-34)

update (9-12)

app/controllers/application_controller.rb (1)

render_serialized (238-240)

lib/topic_retriever.rb (1)

app/models/embeddable_host.rb (1)

host_allowed? (20-22)

app/controllers/admin/embeddable_hosts_controller.rb (3)

app/controllers/admin/embedding_controller.rb (2)

before_filter (1-21)

update (9-11)

app/serializers/embeddable_host_serializer.rb (2)

host (8-10)

category_id (12-14)

app/controllers/application_controller.rb (3)

success_json (362-364)

render_serialized (238-240)

render_json_error (357-360)

🪛 RuboCop (1.76.1)

spec/controllers/admin/embeddable_hosts_controller_spec.rb

[convention] 4-5: Extra empty line detected at block body beginning.