feat: add show author option to portal settings and update related views#225
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe changes introduce a "Show Author" feature that allows portal administrators to toggle whether article authors are displayed on category pages. This involves backend config handling, frontend UI components, API updates, conditional view rendering, and localization. Changes
Sequence DiagramsequenceDiagram
actor User
participant Frontend as Frontend UI<br/>(Vue Component)
participant API as Backend API<br/>(PortalsController)
participant Model as Portal Model
participant DB as Database
participant Views as Public Views
User->>Frontend: Toggle "Show Author" switch
Frontend->>API: PUT /portals/{slug} with config:{show_author: boolean}
API->>API: merged_portal_params()<br/>merges new config with<br/>existing `@portal.config`
API->>Model: update(merged_portal_params)
Model->>DB: persist merged config
DB-->>Model: confirmation
Model->>API: updated portal instance
API->>API: render portal.json<br/>includes show_author field
API-->>Frontend: JSON response with config
Frontend-->>User: UI updated
User->>Views: Visit category page
Views->>Model: check `@portal.show_author`?
alt show_author is true
Views->>Views: Render authors block
else show_author is false
Views->>Views: Skip authors block
end
Views-->>User: Display category with/without authors
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai preview |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 11855
📋 OverviewFeature: Add "Show Author" option to portal settings Stats: 9 files changed (+47/-4 lines) 🎯 What This PR DoesThis PR adds a new configuration option that allows portal administrators to control whether article author information is displayed on category pages in the help center. 🔧 Key ChangesBackend (Ruby/Rails)
Frontend (Vue.js)
View Templates (ERB)
Tests
✨ Default BehaviorAuthor information is shown by default ( |
There was a problem hiding this comment.
Pull request overview
Adds a new portal setting to control whether article authors are displayed on public help center category pages, exposing the flag via the accounts portal API and wiring it into the dashboard settings UI.
Changes:
- Introduces
show_authoras a portal config flag (defaulting to enabled) and exposes it in the portal JSONconfig. - Adds a dashboard toggle (with en/pt-BR copy) to update
config.show_author. - Updates public category views to conditionally render the authors block based on the portal setting.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
app/models/portal.rb |
Adds show_author to allowed config keys and defines show_author? default behavior. |
app/controllers/api/v1/accounts/portals_controller.rb |
Permits config.show_author and merges config updates to avoid overwriting other config keys. |
app/views/api/v1/accounts/portals/_portal.json.jbuilder |
Includes show_author inside the config payload. |
app/views/public/api/v1/portals/categories/_category-hero.html.erb |
Conditionally renders authors section based on the portal flag. |
app/views/public/api/v1/portals/_category-block.html.erb |
Conditionally renders authors section based on the portal flag. |
app/javascript/dashboard/components-next/HelpCenter/Pages/PortalSettingsPage/PortalBaseSettings.vue |
Adds a switch to edit config.show_author and initializes it from API payload. |
app/javascript/dashboard/i18n/locale/en/helpCenter.json |
Adds English label/help text for the new toggle. |
app/javascript/dashboard/i18n/locale/pt_BR/helpCenter.json |
Adds pt-BR label/help text for the new toggle. |
spec/controllers/api/v1/accounts/portals_controller_spec.rb |
Updates expectation to include config.show_author in the response. |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
spec/controllers/api/v1/accounts/portals_controller_spec.rb (1)
134-135: Consider adding a negative-path assertion forshow_author: false.This expectation covers the default path, but a companion update test with
config: { show_author: false }would protect the new toggle behavior end-to-end.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/controllers/api/v1/accounts/portals_controller_spec.rb` around lines 134 - 135, Add a negative-path spec that exercises the toggle when config: { show_author: false } and asserts the controller response contains 'show_author' => false; specifically, in spec/controllers/api/v1/accounts/portals_controller_spec.rb add a test case (mirroring the existing positive case) that builds the request payload with config: { 'show_author' => false }, calls the same action (the example that currently returns the hash with 'show_author' => true) and expects the parsed response JSON to include 'show_author' => false to ensure the toggle is honored end-to-end.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/controllers/api/v1/accounts/portals_controller.rb`:
- Around line 86-89: In merged_portal_params, guard against `@portal.config` being
nil before calling merge: when update_params has a 'config' key, merge the
incoming config into a safe hash derived from `@portal.config` (e.g. use
(`@portal.config` || {}).merge(update_params['config']) or
`@portal.config.to_h.merge`(...)) so the method never calls merge on nil; update
the logic in merged_portal_params to use that safe fallback.
In `@app/models/portal.rb`:
- Around line 49-51: The show_author? method currently only checks for literal
false and misinterprets string values like "false"; update show_author? to
coerce config['show_author'] into a real boolean before comparing (e.g., use
ActiveModel::Type::Boolean.new.cast or similar) so values like "false" or "0"
are treated as false; reference the show_author? method and
config['show_author'] key when making the change so the method reliably returns
a boolean regardless of whether the config value came from the frontend, API, or
raw SQL.
---
Nitpick comments:
In `@spec/controllers/api/v1/accounts/portals_controller_spec.rb`:
- Around line 134-135: Add a negative-path spec that exercises the toggle when
config: { show_author: false } and asserts the controller response contains
'show_author' => false; specifically, in
spec/controllers/api/v1/accounts/portals_controller_spec.rb add a test case
(mirroring the existing positive case) that builds the request payload with
config: { 'show_author' => false }, calls the same action (the example that
currently returns the hash with 'show_author' => true) and expects the parsed
response JSON to include 'show_author' => false to ensure the toggle is honored
end-to-end.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/controllers/api/v1/accounts/portals_controller.rbapp/javascript/dashboard/components-next/HelpCenter/Pages/PortalSettingsPage/PortalBaseSettings.vueapp/javascript/dashboard/i18n/locale/en/helpCenter.jsonapp/javascript/dashboard/i18n/locale/pt_BR/helpCenter.jsonapp/models/portal.rbapp/views/api/v1/accounts/portals/_portal.json.jbuilderapp/views/public/api/v1/portals/_category-block.html.erbapp/views/public/api/v1/portals/categories/_category-hero.html.erbspec/controllers/api/v1/accounts/portals_controller_spec.rb
Pull Request Template
Description
Please include a summary of the change and issue(s) fixed. Also, mention relevant motivation, context, and any dependencies that this change requires.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
Checklist:
This change is
Summary by CodeRabbit
Release Notes