Skip to content

UX: show complete URL path if website domain is same as instance domain#5

Open
ShashankFC wants to merge 1 commit into
url-handling-prefrom
url-handling-post
Open

UX: show complete URL path if website domain is same as instance domain#5
ShashankFC wants to merge 1 commit into
url-handling-prefrom
url-handling-post

Conversation

@ShashankFC

@ShashankFC ShashankFC commented Jan 22, 2026

Copy link
Copy Markdown

Test 6

Summary by CodeRabbit

  • Refactor

    • Reorganized code architecture to improve maintainability and better separate concerns.
  • Tests

    • Enhanced test coverage for website domain name handling with new test cases covering different domain matching scenarios, including host name derivation and path normalization based on domain configuration relationships.

✏️ Tip: You can customize this high-level summary in your review settings.


Replicated from ai-code-review-evaluation/discourse-coderabbit#6


Note

Medium Risk
Introduces new URL/host parsing and domain-comparison logic in UserSerializer, which could affect profile rendering for edge-case URLs or unusual hostnames; changes are localized and covered by added tests.

Overview
User profile website display now uses a new serialized field, website_name, instead of computing websiteName in the Ember controller.

UserSerializer adds website_name with hostname/path-aware logic (show full path when the website is the same domain or a parent/related domain of the instance) and includes it only when website is present; the profile template is updated to render model.website_name, and serializer specs add coverage for the domain-matching scenarios.

Written by Cursor Bugbot for commit 267d8be. Configure here.

@ShashankFC

Copy link
Copy Markdown
Author

bugbot run

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.


def include_website_name
website.present?
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing ? suffix on conditional include method

High Severity

The method include_website_name is missing the trailing ? that ActiveModel::Serializer 0.8 expects for conditional attribute inclusion. Every other conditional include method in this file uses the ? convention (include_email?, include_card_image_badge_id?, include_stats?, etc.), and the framework's own macros (staff_attributes, private_attributes, untrusted_attributes) all generate methods with ?. Without it, the serializer framework never calls this method, so website_name is always serialized — even when the website is blank.

Fix in Cursor Fix in Web

:bio_cooked,
:created_at,
:website,
:website_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.

Website name leaks for hidden untrusted profiles

Medium Severity

The website attribute is listed in untrusted_attributes, hiding it from anonymous users viewing TL0 profiles. However, website_name is not similarly protected. Previously, the client-side computed websiteName relied on model.website being present in the payload, which it wouldn't be for restricted users. Now website_name is computed server-side and always serialized, leaking the user's website hostname even when website itself is intentionally hidden.

Additional Locations (1)

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants