Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add profile option to always view full names in event view #566

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
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
2 changes: 1 addition & 1 deletion app/controllers/members_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ def member_params
params.require(:member).permit(
:password, :password_confirmation, :email, :namefirst, :namelast, :namenick, :title, :callsign,
:shirt_size, :phone, :payrate, :role, :tracker_dev, :receives_comment_emails,
:on_payroll, :pronouns, :favorite_entropy_drink, :major, :grad_year, :interests, :officer_position,
:on_payroll, :pronouns, :favorite_entropy_drink, :prefers_full_name, :major, :grad_year, :interests, :officer_position,
:super_tics_attributes => [:id, :_destroy, :day]
)
end
Expand Down
13 changes: 12 additions & 1 deletion app/helpers/events_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,10 @@ def show_run_position(er, hover)
elsif er.appliable and not er.assigned? and can? :create, er.applications.build(member: current_member)
return link_to("you?", new_application_url(er.event, event_role_id: er.id, format: :js), :remote => true) unless hover
"you?" if hover
elsif !hover and current_member and current_member.prefers_full_name
return er.assigned_to :both_names
elsif !hover and current_member
er.assigned_to use_display_name: true
return er.assigned_to :display_name
else
er.assigned_to
end
Comment on lines 151 to 158
Copy link
Member

Choose a reason for hiding this comment

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

Can we use run_position_name_preference() here too to be consistent? Maybe:

elsif !hover and current_member
  return er.assigned_to run_position_name_preference(current_member)

That way if we add more options or change things in the future it lowers the number of places in the code to update.

Expand All @@ -166,4 +168,13 @@ def weeks_from_now(weeks)
end
end

def run_position_name_preference(member)
if !member
return :full_name
elsif member.prefers_full_name
return :both_names
else
return :display_name
end
end
end
12 changes: 8 additions & 4 deletions app/models/event_role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,16 @@ def to_s
role + ": " + assigned_to
end

def assigned_to(options = {})
def assigned_to(mode = :full_name)
# Logger
Copy link
Member

Choose a reason for hiding this comment

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

Logger?

if assigned?
if options[:use_display_name]
DaAwesomeP marked this conversation as resolved.
Show resolved Hide resolved
member.display_name
else
case mode
when :full_name
member.fullname
when :display_name
member.display_name
when :both_names
"#{member.display_name} (#{member.fullname})"
end
else
"(unassigned)"
Expand Down
6 changes: 3 additions & 3 deletions app/views/events/_event_role_fields.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
<td><%= link_to_remove_fields image_tag("cross.png"), f, f.object.new_record? %></td>
<% elsif f.object.member == current_member %>
<td><%= f.object.role %></td>
<td><%= f.object.assigned_to use_display_name: true %></td>
<td><%= f.object.assigned_to(run_position_name_preference(current_member)) %></td>
<td></td>
<td><%= link_to_remove_fields image_tag("cross.png"), f %></td>
<% else %>
<td><%= f.object.role %></td>
<td><%= f.object.assigned_to use_display_name: true %></td>
<td><%= f.object.assigned_to(run_position_name_preference(current_member)) %></td>
DaAwesomeP marked this conversation as resolved.
Show resolved Hide resolved
<% end %>
</tr>
</tr>
7 changes: 6 additions & 1 deletion app/views/members/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,16 @@
<dd><%= f.text_field :interests, class: "wide-input" %></dd>
</dl>

<dl>
<dt><%= f.label :prefers_full_name, "Show full names of members:" %></dt>
<dd><%= f.check_box :prefers_full_name %></dd>
Copy link
Contributor

Choose a reason for hiding this comment

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

likewise I think it is a little more straightforward to say something like :show_real_names, as we aren't really showing "full names" in the traditional sense and "prefers" is more ambiguous as to what the result is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree somewhat with this. Technically in the code what is happening is that the fullname variable is being displayed. I think it's better to use only one name for this in the code.

Copy link
Member

Choose a reason for hiding this comment

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

One small edit request: Make it "Always show full names of members:" The new distinction is the always.

</dl>

</fieldset>

<fieldset>
<legend>Notifications</legend>

<dl>
<dt><%= f.check_box :receives_comment_emails %></dt>
<dd><%= f.label :receives_comment_emails, "Email me when comments are posted on events I have run positions for" %></dd>
Expand Down
6 changes: 6 additions & 0 deletions app/views/members/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,12 @@
<th>Interests</td>
<td><%= @member.interests %></td>
</tr>
<% if @member == current_member %>
<tr>
<th>Show real names of members</td>
<td><%= @member.prefers_full_name ? "YES" : "NO" %></td>
</tr>
<% end %>
</table>
</td>
<% if can? :read, Timecard %>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddProfileOptionViewFullName < ActiveRecord::Migration[6.1]
def change
# Change members table; Add column prefers_full_name
add_column :members, :prefers_full_name, :boolean, default: false
Copy link
Member

Choose a reason for hiding this comment

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

Please also add null: false.

end
end
3 changes: 2 additions & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.