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

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

@ShashankFC

Copy link
Copy Markdown
Author

@cubic-dev-ai review this pull request

@cubic-dev-ai

cubic-dev-ai Bot commented Feb 13, 2026

Copy link
Copy Markdown

@cubic-dev-ai review this pull request

@ShashankFC I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai 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.

2 issues found across 5 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="app/serializers/user_serializer.rb">

<violation number="1" location="app/serializers/user_serializer.rb:43">
P1: Potential information leak: `website_name` is not listed in `untrusted_attributes`, but `website` is. This means `website_name` will be visible to anonymous users viewing TL0 profiles, even though `website` is hidden for them. Consider adding `:website_name` to the `untrusted_attributes` list.</violation>

<violation number="2" location="app/serializers/user_serializer.rb:153">
P1: Bug: Missing `?` suffix on `include_website_name`. ActiveModelSerializers expects `include_<attr>?` (with question mark) for conditional attribute inclusion. Without it, this method is never called by AMS, so `website_name` will always be serialized — even when `website` is blank — resulting in `null` values in the JSON response.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

:bio_cooked,
:created_at,
:website,
:website_name,

@cubic-dev-ai cubic-dev-ai Bot Feb 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Potential information leak: website_name is not listed in untrusted_attributes, but website is. This means website_name will be visible to anonymous users viewing TL0 profiles, even though website is hidden for them. Consider adding :website_name to the untrusted_attributes list.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/serializers/user_serializer.rb, line 43:

<comment>Potential information leak: `website_name` is not listed in `untrusted_attributes`, but `website` is. This means `website_name` will be visible to anonymous users viewing TL0 profiles, even though `website` is hidden for them. Consider adding `:website_name` to the `untrusted_attributes` list.</comment>

<file context>
@@ -40,6 +40,7 @@ def self.untrusted_attributes(*attrs)
              :bio_cooked,
              :created_at,
              :website,
+             :website_name,
              :profile_background,
              :card_background,
</file context>
Fix with Cubic

end
end

def include_website_name

@cubic-dev-ai cubic-dev-ai Bot Feb 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Bug: Missing ? suffix on include_website_name. ActiveModelSerializers expects include_<attr>? (with question mark) for conditional attribute inclusion. Without it, this method is never called by AMS, so website_name will always be serialized — even when website is blank — resulting in null values in the JSON response.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/serializers/user_serializer.rb, line 153:

<comment>Bug: Missing `?` suffix on `include_website_name`. ActiveModelSerializers expects `include_<attr>?` (with question mark) for conditional attribute inclusion. Without it, this method is never called by AMS, so `website_name` will always be serialized — even when `website` is blank — resulting in `null` values in the JSON response.</comment>

<file context>
@@ -133,6 +134,26 @@ def website
+    end
+  end
+
+  def include_website_name
+    website.present?
+  end
</file context>
Suggested change
def include_website_name
def include_website_name?
Fix with Cubic

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