Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/controllers/profiles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def set_profile_social_preview

def profile_social_description
return @user.profile_bio.to_s.squish.truncate(180) if @user.profile_bio.present?
"View #{@user.display_name_override.presence || @user.display_name}'s Hackatime coding profile."
"View #{@user.display_name}'s Hackatime coding profile."
end

def ensure_profile_og_image!
Expand Down Expand Up @@ -159,7 +159,7 @@ def public_profile_og_heatmap
end

def profile_summary_payload
{ display_name: @user.display_name_override.presence || @user.display_name,
{ display_name: @user.display_name,
username: @user.username || "", avatar_url: @user.avatar_url,
trust_level: @user.public_trust_level, bio: @user.profile_bio,
social_links: profile_social_links, github_profile_url: @user.github_profile_url,
Expand Down
5 changes: 4 additions & 1 deletion app/controllers/settings/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ def common_props(active_section:)
page_title: (is_own ? "My Settings" : "Settings | #{@user.display_name}"),
heading: (is_own ? "Settings" : "Settings for #{@user.display_name}"),
subheading: "Manage your profile, appearance, editors, integrations, privacy, goals, and data tools.",
errors: { full_messages: @user.errors.full_messages, username: @user.errors[:username] } }
errors: { full_messages: @user.errors.full_messages,
display_name_override: @user.errors[:display_name_override],
username: @user.errors[:username] } }
end

# Subclasses override this to provide section-specific props
Expand All @@ -45,6 +47,7 @@ def section_props = {}
USER_PROP_BUILDERS = {
id: ->(u) { u.id },
display_name: ->(u) { u.display_name },
display_name_override: ->(u) { u.display_name_override },
timezone: ->(u) { u.timezone },
country_code: ->(u) { u.country_code },
username: ->(u) { u.username },
Expand Down
5 changes: 4 additions & 1 deletion app/controllers/settings/profile_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class Settings::ProfileController < Settings::BaseController
def show = render_profile
def update_region = update_section(region_params)
def update_display_name = update_section(display_name_params)
def update_username = update_section(username_params)

private
Expand All @@ -12,7 +13,8 @@ def render_profile(status: :ok)
def section_props
{
username_max_length: User::USERNAME_MAX_LENGTH,
user: user_props(keys: %i[country_code timezone username]),
display_name_max_length: User::DISPLAY_NAME_MAX_LENGTH,
user: user_props(keys: %i[country_code timezone display_name display_name_override username]),
options: base_options(keys: %i[countries timezones]),
profile_url: (@user.username.present? ? "https://hackati.me/#{@user.username}" : nil),
emails: @user.email_addresses.map { |email|
Expand All @@ -38,5 +40,6 @@ def region_params
permitted
end

def display_name_params = params.require(:user).permit(:display_name_override)
def username_params = params.require(:user).permit(:username)
end
36 changes: 36 additions & 0 deletions app/javascript/pages/Users/Settings/Profile.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
heading,
subheading,
username_max_length,
display_name_max_length,
user,
options,
profile_url,
Expand Down Expand Up @@ -71,6 +72,41 @@
{/snippet}
</SectionCard>

<SectionCard
id="user_display_name"
title="Display Name"
description="This name appears across Hackatime instead of your Slack, GitHub, or username."
>
<Form
id="profile-display-name-form"
action={settingsProfile.updateDisplayName.path()}
method="patch"
class="space-y-3"
options={{ preserveScroll: true }}
>
<Field
inputId="display_name_override"
label="Display name"
error={errors.display_name_override[0]}
>
<input
id="display_name_override"
name="user[display_name_override]"
value={user.display_name_override || ""}
maxlength={display_name_max_length}
placeholder={user.display_name}
class={inputClass}
/>
</Field>
</Form>

{#snippet footer()}
<Button type="submit" variant="primary" form="profile-display-name-form"
>Save display name</Button
>
{/snippet}
</SectionCard>

<SectionCard
id="user_username"
title="Username"
Expand Down
18 changes: 16 additions & 2 deletions app/javascript/pages/Users/Settings/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export type GoalForm = {
export type UserProps = {
id: number;
display_name: string;
display_name_override?: string | null;
timezone: string;
country_code?: string | null;
username?: string | null;
Expand Down Expand Up @@ -209,6 +210,7 @@ export type HeartbeatImportStatusProps = {

export type ErrorsProps = {
full_messages: string[];
display_name_override: string[];
username: string[];
};

Expand All @@ -222,7 +224,15 @@ export type SettingsCommonProps = {

export type ProfilePageProps = SettingsCommonProps & {
username_max_length: number;
user: Pick<UserProps, "country_code" | "timezone" | "username">;
display_name_max_length: number;
user: Pick<
UserProps,
| "country_code"
| "timezone"
| "display_name"
| "display_name_override"
| "username"
>;
options: Pick<BaseOptionsProps, "countries" | "timezones">;
profile_url: string | null;
emails: EmailProps[];
Expand All @@ -238,7 +248,10 @@ export type AppearancePageProps = SettingsCommonProps & {
};

export type EditorsPageProps = SettingsCommonProps & {
user: Pick<UserProps, "hackatime_extension_text_type" | "show_goals_in_statusbar">;
user: Pick<
UserProps,
"hackatime_extension_text_type" | "show_goals_in_statusbar"
>;
options: Pick<BaseOptionsProps, "extension_text_types">;
};

Expand Down Expand Up @@ -309,6 +322,7 @@ export const buildSections = (): SettingsSection[] => [
const subsectionMap: Record<SectionId, SettingsSubsection[]> = {
profile: [
{ id: "user_region", label: "Region" },
{ id: "user_display_name", label: "Display name" },
{ id: "user_username", label: "Username" },
{ id: "user_email_addresses", label: "Email addresses" },
],
Expand Down
12 changes: 12 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ class User < ApplicationRecord
has_subscriptions

USERNAME_MAX_LENGTH = 21 # going over 21 overflows the navbar
DISPLAY_NAME_MAX_LENGTH = 80

has_paper_trail

after_create :subscribe_to_default_lists
after_create_commit :schedule_onboarding_check_in_email
after_update_commit :clear_leaderboard_page_cache, if: :saved_change_to_leaderboard_shadowban_state?
before_validation :normalize_username
before_validation :normalize_display_name_override
encrypts :slack_access_token, :github_access_token, :hca_access_token

validates :slack_uid, uniqueness: true, allow_nil: true
Expand All @@ -29,6 +31,7 @@ class User < ApplicationRecord
format: { with: /\A[A-Za-z0-9_-]+\z/, message: "may only include letters, numbers, '-', and '_'" },
uniqueness: { case_sensitive: false, message: "has already been taken" },
allow_nil: true
validates :display_name_override, length: { maximum: DISPLAY_NAME_MAX_LENGTH }, allow_nil: true
validates :leaderboard_shadowban_reason, presence: true, if: :leaderboard_shadowbanned?
validate :username_must_be_visible

Expand Down Expand Up @@ -306,6 +309,9 @@ def avatar_url
end

def display_name
name = display_name_override.presence
return name if name.present?
Comment on lines +312 to +313
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Redundant double-presence check: display_name_override.presence already returns nil when the value is blank, so name.present? on the next line always equals !name.nil?. The guard can be collapsed to a single expression, consistent with the natural reading of the method.

Suggested change
name = display_name_override.presence
return name if name.present?
return display_name_override if display_name_override.present?
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/models/user.rb
Line: 312-313

Comment:
Redundant double-presence check: `display_name_override.presence` already returns `nil` when the value is blank, so `name.present?` on the next line always equals `!name.nil?`. The guard can be collapsed to a single expression, consistent with the natural reading of the method.

```suggestion
    return display_name_override if display_name_override.present?
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


name = slack_username || github_username || username
return name if name.present?
email = email_addresses&.first&.email
Expand Down Expand Up @@ -394,4 +400,10 @@ def username_must_be_visible
return unless instance_variable_defined?(:@username_cleared_for_invisible) && @username_cleared_for_invisible
errors.add(:username, "must include visible characters")
end

def normalize_display_name_override
return if display_name_override.nil?

self.display_name_override = display_name_override.gsub(/\p{Cf}/, "").strip.presence
end
end
2 changes: 1 addition & 1 deletion app/services/profile_og_image_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def self.template
end

def display_name
@display_name ||= user.display_name_override.presence || user.display_name
@display_name ||= user.display_name
end

def username
Expand Down
1 change: 1 addition & 0 deletions config/initializers/js_from_routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ module JsFromRoutes
my_settings
my_settings_profile
my_settings_profile_region
my_settings_profile_display_name
my_settings_profile_username
my_settings_setup
my_settings_appearance
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ def matches?(request)
# Profile
get "my/settings/profile", to: "settings/profile#show", as: :my_settings_profile
patch "my/settings/profile/region", to: "settings/profile#update_region", as: :my_settings_profile_region
patch "my/settings/profile/display_name", to: "settings/profile#update_display_name", as: :my_settings_profile_display_name
patch "my/settings/profile/username", to: "settings/profile#update_username", as: :my_settings_profile_username

# Setup
Expand Down
37 changes: 37 additions & 0 deletions test/controllers/settings_profile_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,43 @@ class SettingsProfileControllerTest < ActionDispatch::IntegrationTest
assert_nil user.reload.country_code
end

test "display name update persists override" do
user = users(:one)
user.update!(slack_username: "slack_name")
sign_in_as(user)

patch my_settings_profile_display_name_path, params: { user: { display_name_override: "Custom Name" } }

assert_response :redirect
assert_redirected_to my_settings_profile_path
assert_equal "Custom Name", user.reload.display_name_override
assert_equal "Custom Name", user.display_name
end

test "display name update clears blank override" do
user = users(:one)
user.update!(display_name_override: "Custom Name", slack_username: "slack_name")
sign_in_as(user)

patch my_settings_profile_display_name_path, params: { user: { display_name_override: " " } }

assert_response :redirect
assert_nil user.reload.display_name_override
assert_equal "slack_name", user.display_name
end

test "display name update with invalid display name returns unprocessable entity" do
user = users(:one)
sign_in_as(user)

patch my_settings_profile_display_name_path, params: {
user: { display_name_override: "a" * (User::DISPLAY_NAME_MAX_LENGTH + 1) }
}

assert_response :unprocessable_entity
assert_inertia_component "Users/Settings/Profile"
end

test "username update with invalid username returns unprocessable entity" do
user = users(:one)
user.update!(username: "good_name")
Expand Down
40 changes: 40 additions & 0 deletions test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,46 @@ class UserTest < ActiveSupport::TestCase
assert_equal "User;#{user.id}", user.flipper_id
end

test "display name override takes precedence over synced provider names" do
user = User.create!(
timezone: "UTC",
username: "profile_user",
slack_username: "slack_user",
github_username: "github_user",
display_name_override: "Custom Name"
)

assert_equal "Custom Name", user.display_name
end

test "display name override is normalized before validation" do
user = User.create!(timezone: "UTC", slack_username: "slack_user", display_name_override: " Custom Name ")

assert_equal "Custom Name", user.display_name_override
end

test "slack profile sync does not replace display name override" do
user = User.create!(
timezone: "UTC",
slack_username: "old_slack",
display_name_override: "Custom Name"
)

user.apply_slack_profile_attributes({
"name" => "fallback",
"profile" => {
"display_name_normalized" => "new_slack",
"real_name_normalized" => "Real Name",
"image_192" => "https://example.com/avatar.png"
}
})
user.save!

assert_equal "new_slack", user.reload.slack_username
assert_equal "Custom Name", user.display_name_override
assert_equal "Custom Name", user.display_name
end

test "active remote heartbeat import run only counts remote imports" do
user = User.create!(timezone: "UTC")

Expand Down
Loading