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
15 changes: 2 additions & 13 deletions app/components/event_card_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<% cache @event.cache_key_with_version do %>
<% cache [@event.cache_key_with_version, I18n.locale, :v2] do %>
<div class="card mb-4" data-test="event">
<div class="card-body">
<div class="d-md-flex justify-content-md-between">
Expand All @@ -8,18 +8,7 @@
<%= link_to @event.chapter.name, @event.chapter.slug, class: "text-light text-decoration-none" %>
</span>
<% end %>
<% if user_presenter %>
<% if user_presenter.attending?(@event) %>
<span class="badge bg-success mb-3 mb-md-0">
<%= link_to "Attending", @event.path, class: "text-light text-decoration-none" %>
</span>
<% end %>
<% if user_presenter.organiser? && user_presenter.event_organiser?(@event) %>
<span class="badge bg-secondary mb-3 mb-md-0">
<%= link_to "Manage", @event.admin_path, class: "text-light text-decoration-none" %>
</span>
<% end %>
<% end %>

</div>
<div class="order-md-1">
<h3 class="h5">
Expand Down
8 changes: 1 addition & 7 deletions app/components/event_card_component.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
class EventCardComponent < ViewComponent::Base
def initialize(event_card:, user: nil)
def initialize(event_card:)
super()
@event = event_card
@user = user
end

# Wraps raw Member in MemberPresenter; double-wrapping a presenter is a no-op.
def user_presenter
@user && MemberPresenter.new(@user)
end
end
4 changes: 2 additions & 2 deletions app/controllers/chapter_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ def slug
end

def upcoming_events_by_chapter(chapter)
workshops = chapter.upcoming_workshops.includes(:sponsors)
events = chapter.events.upcoming
workshops = chapter.upcoming_workshops.eager_load(:sponsors, :organisers, :permissions, :workshop_host)
events = chapter.events.upcoming.eager_load(:venue, :sponsors, :sponsorships, :permissions, :organisers)

[*workshops, *events].uniq.compact.sort_by(&:date_and_time).group_by(&:date)
end
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/events_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,12 @@ def load_events(rows)
(hash[row['event_type']] ||= []) << row['id'].to_i
end

workshops = Workshop.includes(:chapter, :sponsors, :host, :permissions)
workshops = Workshop.eager_load(:chapter, :sponsors, :organisers, :permissions, :workshop_host)
.where(id: grouped['Workshop'])
.to_a.index_by(&:id)
meetings = Meeting.includes(:venue).where(id: grouped['Meeting'])
meetings = Meeting.eager_load(:venue, :organisers, :permissions).where(id: grouped['Meeting'])
.to_a.index_by(&:id)
events = Event.includes(:venue, :sponsors, :sponsorships, :permissions)
events = Event.eager_load(:venue, :sponsors, :sponsorships, :permissions, :organisers)
.where(id: grouped['Event'])
.to_a.index_by(&:id)

Expand Down
5 changes: 0 additions & 5 deletions app/models/concerns/invitation_concerns.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ module InvitationConcerns
scope :taken_place, -> { where('date_and_time < ?', Time.zone.now) }

before_create :set_token
after_save :clear_member_cache, if: :saved_change_to_attending?
end

module InstanceMethods
Expand All @@ -40,10 +39,6 @@ def for_participant?

private

def clear_member_cache
member.clear_attending_event_ids_cache!
end

def set_token
self.token = loop do
random_token = SecureRandom.urlsafe_base64(nil, false)
Expand Down
13 changes: 0 additions & 13 deletions app/models/member.rb
Original file line number Diff line number Diff line change
Expand Up @@ -156,19 +156,6 @@ def past_rsvps
@past_rsvps ||= rsvps(period: :past).reverse
end

def attending_event_ids
@attending_event_ids ||= begin
event_ids = invitations.accepted.pluck(:event_id)
workshop_ids = workshop_invitations.accepted.pluck(:workshop_id)
meeting_ids = meeting_invitations.accepted.pluck(:meeting_id)
(event_ids + workshop_ids + meeting_ids).to_set
end
end

def clear_attending_event_ids_cache!
@attending_event_ids = nil
end

def flag_to_organisers?
return admin_workshop_flags[:flag_to_organisers] if admin_workshop_flags

Expand Down
26 changes: 0 additions & 26 deletions app/presenters/member_presenter.rb
Original file line number Diff line number Diff line change
@@ -1,25 +1,10 @@
class MemberPresenter < BasePresenter
def organiser?
@organiser ||= has_role? :organiser, :any
end

def event_organiser?(event)
event_types = %w[Workshop Meeting Event]
organising_events.slice(*event_types).values.any? { |ids| ids.include?(event.id) } ||
(event.chapter && organising_events['Chapter']&.include?(event.chapter.id)) ||
admin?
end

def newbie?
return model.admin_workshop_flags[:newbie] if model.admin_workshop_flags

!workshop_invitations.attended.exists?
end

def attending?(event)
model.attending_event_ids.include?(event.id)
end

def subscribed_to_newsletter?
opt_in_newsletter_at.present?
end
Expand All @@ -38,17 +23,6 @@ def displayed_dietary_restrictions

private

def organising_events
@organising_events ||= roles.where(name: 'organiser')
.pluck(:resource_type, :resource_id)
.group_by(&:first)
.transform_values { |pairs| pairs.map(&:last) }
end

def admin?
@admin ||= has_role?(:admin)
end

def coach_pairing_details(note)
[newbie?, full_name, 'Coach', 'N/A', note, skill_list.to_s]
end
Expand Down
2 changes: 1 addition & 1 deletion app/views/dashboard/dashboard.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@
There are no upcoming events announced for the chapters you are subscribed to.
- @ordered_events.each do |date, workshops|
%h4= date
= render EventCardComponent.with_collection(workshops, user: current_user)
= render EventCardComponent.with_collection(workshops)
2 changes: 1 addition & 1 deletion app/views/dashboard/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
%h2.h3.mb-4= t('homepage.events.upcoming')
- @upcoming_workshops.each do |date, workshops|
%h3.h5= date
= render EventCardComponent.with_collection(workshops, user: current_user)
= render EventCardComponent.with_collection(workshops)

- if @has_more_events
= link_to 'Explore all events →', upcoming_events_path, class: 'btn btn-outline-primary mt-3'
Expand Down
39 changes: 31 additions & 8 deletions spec/components/event_card_component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,40 @@
let(:presenter) { WorkshopPresenter.new(workshop) }
let(:member) { Fabricate(:member) }

it 'renders attending badge when user is attending (as presenter)' do
it 'renders user-independent output even with a warm cache' do
Fabricate(:workshop_invitation, workshop:, member:, attending: true)
user_presenter = MemberPresenter.new(member)
render_inline(described_class.new(event_card: presenter, user: user_presenter))
expect(page).to have_text('Attending')
cache = ActiveSupport::Cache::MemoryStore.new

# Regression: user-dependent badges (Attending/Manage) were rendered inside
# the cached fragment, so an organiser's dashboard render leaked badges to
# every other user, including signed-out visitors. The card must contain
# nothing user-specific, ever.
ActionController::Base.cache_store = cache
first = render_inline(described_class.new(event_card: presenter)).to_html
second = render_inline(described_class.new(event_card: presenter)).to_html

expect(first).not_to include('Attending')
expect(first).not_to include('Manage')
expect(first).to eq(second)

# The fragment key includes I18n.locale, so a render under another locale
# must not reuse (or poison) the :en fragment.
begin
I18n.locale = :fr
french = render_inline(described_class.new(event_card: presenter)).to_html
expect(french).not_to eq(first)
ensure
I18n.locale = :en
end
ensure
ActionController::Base.cache_store = :null_store
end

it 'renders attending badge when raw Member is passed' do
Fabricate(:workshop_invitation, workshop:, member:, attending: true)
render_inline(described_class.new(event_card: presenter, user: member))
expect(page).to have_text('Attending')
it 'no longer accepts a user (card must be user-agnostic)' do
# Pins the fix for #2869: badges rendered behind the removed `user:` kwarg,
# so a reintroduction must fail loudly here.
expect { described_class.new(event_card: presenter, user: member) }
.to raise_error(ArgumentError, /unknown keyword/)
end
end
end
28 changes: 28 additions & 0 deletions spec/features/member_portal_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,34 @@
expect(page).to have_text("#{presenter} at #{presenter.venue.name}", count: 1)
end

it 'does not leak attending badges through the shared card cache' do
# Regression for #2869: the first member's dashboard render warms the
# event-card fragment; a second member viewing the same card must not
# see the first member's badge baked into the cached fragment.
chapter = Fabricate(:chapter_with_groups)
workshop = Fabricate(:workshop, chapter:)
Fabricate(:subscription, member:, group: chapter.groups.first)
Fabricate(:attending_workshop_invitation, member:, workshop:)
other_member = Fabricate(:member)
Fabricate(:subscription, member: other_member, group: chapter.groups.first)
presenter = WorkshopPresenter.new(workshop)

ActionController::Base.cache_store = ActiveSupport::Cache::MemoryStore.new
begin
visit dashboard_path
expect(page).to have_text("#{presenter} at #{presenter.venue.name}", count: 1)

login(other_member)
visit dashboard_path
expect(page).to have_text("#{presenter} at #{presenter.venue.name}", count: 1)
# 'Manage' is legitimately in the nav ("Manage subscriptions"), so only
# the Attending badge is asserted absent — it never appears in chrome.
expect(page).to have_no_text('Attending')
ensure
ActionController::Base.cache_store = :null_store
end
end

it 'can view upcoming workshops for their chapters' do
c1_workshop = Fabricate(:workshop, chapter: Fabricate(:chapter_with_groups))
Fabricate(:subscription, member:, group: c1_workshop.chapter.groups.first)
Expand Down
43 changes: 0 additions & 43 deletions spec/models/member_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -310,47 +310,4 @@
expect(managers.size).to eq(managers.distinct.size)
end
end

describe '#attending_event_ids' do
let(:member) { Fabricate(:member) }

it 'returns event IDs where member has accepted invitation' do
event = Fabricate(:event)
Fabricate(:invitation, member:, event:, attending: true)
expect(member.attending_event_ids).to include(event.id)
end

it 'does not include events where invitation is not accepted' do
event = Fabricate(:event)
Fabricate(:invitation, member:, event:, attending: false)
expect(member.attending_event_ids).not_to include(event.id)
end

it 'includes workshop IDs' do
workshop = Fabricate(:workshop)
Fabricate(:workshop_invitation, member:, workshop:, attending: true)
expect(member.attending_event_ids).to include(workshop.id)
end

it 'includes meeting IDs' do
meeting = Fabricate(:meeting)
Fabricate(:meeting_invitation, member:, meeting:, attending: true)
expect(member.attending_event_ids).to include(meeting.id)
end

it 'caches result in instance variable' do
event = Fabricate(:event)
Fabricate(:invitation, member:, event:, attending: true)
first_call = member.attending_event_ids
expect(member.attending_event_ids).to equal(first_call)
end

it 'can be cleared and re-queries on next call' do
event = Fabricate(:event)
Fabricate(:invitation, member:, event:, attending: true)
member.attending_event_ids
member.clear_attending_event_ids_cache!
expect(member.attending_event_ids).to include(event.id)
end
end
end
87 changes: 0 additions & 87 deletions spec/presenters/member_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,6 @@
let(:member) { Fabricate(:member, skill_list: 'java, ruby') }
let(:member_presenter) { described_class.new(member) }

it '#organiser?' do
allow(member).to receive(:has_role?).with(:organiser, :any)

member_presenter.organiser?

expect(member).to have_received(:has_role?).with(:organiser, :any)
end

it '#subscribed_to_newsletter?' do
allow(member).to receive(:opt_in_newsletter_at)

Expand All @@ -31,83 +23,4 @@
.to eq([member_presenter.newbie?, member.full_name, 'Coach', 'N/A', 'A note', 'java, ruby'])
end
end

describe '#attending?' do
let(:event) { Fabricate(:event) }

it 'returns true when member is attending event' do
Fabricate(:invitation, member:, event:, attending: true)
expect(member_presenter.attending?(event)).to be true
end

it 'returns false when member is not attending' do
expect(member_presenter.attending?(event)).to be false
end
end

describe '#event_organiser?' do
let(:chapter) { Fabricate(:chapter) }
let(:workshop) { Fabricate(:workshop_no_sponsor, chapter:) }

it 'returns true when user is admin' do
admin = Fabricate(:member)
admin.add_role(:admin)
presenter = described_class.new(admin)

expect(presenter.event_organiser?(workshop)).to be true
end

it 'returns true when user is direct organiser of the event' do
member.add_role(:organiser, workshop)

expect(member_presenter.event_organiser?(workshop)).to be true
end

it 'returns true when user is organiser of the event chapter' do
organiser = Fabricate(:member)
organiser.add_role(:organiser, chapter)
presenter = described_class.new(organiser)

expect(presenter.event_organiser?(workshop)).to be true
end

it 'returns false for a regular member' do
regular = Fabricate(:member)
presenter = described_class.new(regular)

expect(presenter.event_organiser?(workshop)).to be false
end

it 'returns false for organiser of a different chapter' do
other_chapter = Fabricate(:chapter)
Fabricate(:workshop_no_sponsor, chapter: other_chapter)
organiser = Fabricate(:member)
organiser.add_role(:organiser, other_chapter)
presenter = described_class.new(organiser)

expect(presenter.event_organiser?(workshop)).to be false
end

it 'handles events without a singular chapter association (e.g. Meeting) without crashing' do
meeting = Fabricate(:meeting)
meeting_presenter = MeetingPresenter.new(meeting)
admin = Fabricate(:member)
admin.add_role(:admin)
presenter = described_class.new(admin)

expect(presenter.event_organiser?(meeting_presenter)).to be true
end

it 'memoizes private methods across calls' do
member.add_role(:admin)

allow(member).to receive(:has_role?).with(:admin).once.and_call_original
# First call loads and caches
member_presenter.event_organiser?(workshop)
# Second call uses cache
member_presenter.event_organiser?(workshop)

expect(member).to have_received(:has_role?).with(:admin).once
end
end
end
Loading