Skip to content

Commit

Permalink
Implement phone number changes (#1563)
Browse files Browse the repository at this point in the history
* in SAC wagon, Person and Group can have only two phone numbers with distinct labels
* valid labels are "mobile" and "landline"
* form fields are fixed, not dynamic as in core
* public flag is not editable
* OIDC claim contains both numbers
* Sign-up wizards fill the phone_number with label "mobile"
  • Loading branch information
daniel-illi authored Feb 13, 2025
1 parent 863a054 commit 01d5a53
Show file tree
Hide file tree
Showing 40 changed files with 557 additions and 169 deletions.
30 changes: 30 additions & 0 deletions app/controllers/concerns/assigns_sac_phone_numbers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# frozen_string_literal: true

# Copyright (c) 2025, Schweizer Alpen-Club. This file is part of
# hitobito_sac_cas and licensed under the Affero General Public License version 3
# or later. See the COPYING file at the top-level directory or at
# https://github.com/hitobito/hitobito_sac_cas

module AssignsSacPhoneNumbers
extend ActiveSupport::Concern

private

def assign_attributes
super
mark_phone_numbers_for_destroy(entry) if action_name == "update"
end

# Mark phone numbers for destruction if they have an empty number field.
# This allows removing phone numbers by clearing the number field in the form.
def mark_phone_numbers_for_destroy(contactable)
PhoneNumber.predefined_labels.each do |label|
phone_number_assoc = :"phone_number_#{label}"
phone_number_params = model_params[:"#{phone_number_assoc}_attributes"]

if phone_number_params && phone_number_params[:number].blank?
contactable.send(phone_number_assoc)&.mark_for_destruction
end
end
end
end
11 changes: 11 additions & 0 deletions app/controllers/sac_cas/groups_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# frozen_string_literal: true

# Copyright (c) 2025, Schweizer Alpen-Club. This file is part of
# hitobito_sac_cas and licensed under the Affero General Public License version 3
# or later. See the COPYING file at the top-level directory or at
# https://github.com/hitobito/hitobito_sac_cas

module SacCas::GroupsController
extend ActiveSupport::Concern
prepend AssignsSacPhoneNumbers
end
1 change: 1 addition & 0 deletions app/controllers/sac_cas/people_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

module SacCas::PeopleController
extend ActiveSupport::Concern
prepend AssignsSacPhoneNumbers

LOOKUP_PREFIX = "people/neuanmeldungen"

Expand Down
17 changes: 2 additions & 15 deletions app/domain/export/tabular/people/sac_mitglied_row.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,9 @@ def layer_navision_id_padded
group.navision_id_padded
end

# Beliebige Telefonnummer mit dem angegebenen Label.
def phone_number(label)
entry.phone_numbers.find { |p| p.label.downcase == label.to_s }&.number
end
def phone_number_landline = entry.phone_number_landline&.number

def phone_number_main
phone_number("haupt-telefon")
end
def phone_number_mobile = entry.phone_number_mobile&.number

def postfach
entry.postbox
Expand All @@ -145,14 +140,6 @@ def empty
nil
end

def method_missing(method, *args)
(method.to_s =~ /^phone_number_(\w+)$/) ? phone_number(::Regexp.last_match(1)) : super
end

def respond_to_missing?(method, include_private = false)
method.to_s.start_with?("phone_number_") || super
end

private

def role_in_layer(*role_classes)
Expand Down
10 changes: 5 additions & 5 deletions app/domain/export/tabular/people/sac_mitglieder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ def attributes # rubocop:disable Metrics/MethodLength
:town,
:country,
:birthday,
:phone_number_main,
:phone_number_privat,
:empty, # 1 leere Spalte
:phone_number_mobil,
:phone_number_fax,
:phone_number_landline,
:empty, # 1 leere Spalte
:phone_number_mobile,
:empty, # 1 leere Spalte
:email,
:gender,
:empty, # 1 leere Spalte
Expand Down Expand Up @@ -68,7 +68,7 @@ def mitglieder
type: SacCas::MITGLIED_ROLES.map(&:sti_name)
})
.joins(:roles)
.includes(:phone_numbers, :roles_unscoped, roles: :group)
.includes(:phone_number_landline, :phone_number_mobile, :roles_unscoped, roles: :group)
.distinct
end

Expand Down
19 changes: 12 additions & 7 deletions app/domain/sac_cas.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,20 @@ module SacCas
.select { |c| c.to_s =~ /MAILING_LIST_.*INTERNAL_KEY/ }
.map { |c| const_get(c) }

MEMBERSHIP_OPERATIONS_GROUP_TYPES = [::Group::Sektion.sti_name, ::Group::Ortsgruppe.sti_name].freeze
AboCost = Data.define(:amount, :country)
ABO_COSTS = {
magazin: [
AboCost.new(amount: 60, country: :switzerland),
AboCost.new(amount: 76, country: :international)
],
tourenportal: [
AboCost.new(amount: 45, country: nil)
]
}

MEMBERSHIP_OPERATIONS_GROUP_TYPES = %w[Group::Sektion Group::Ortsgruppe].freeze
MEMBERSHIP_OPERATIONS_EXCLUDED_IDS = [
2900, 3700, 2249, 2330, 2601, 3030, 3251,
3730, 3952, 3953, 3954, 4530, 4851, 5401
]

def main_phone_label
Settings.phone_number.predefined_labels.find { |l| l =~ /Haupt/ }
end

module_function :main_phone_label
end
15 changes: 9 additions & 6 deletions app/domain/sac_cas/oidc_claim_setup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,12 @@ module SacCas::OidcClaimSetup
def run
super

add_claim(:picture_url, scope: [:name, :with_roles])
add_claim(:membership_verify_url, scope: [:name, :with_roles])
add_claim(:phone, scope: [:name, :with_roles])
with_options(scope: [:name, :with_roles]) do
add_claim(:picture_url)
add_claim(:membership_verify_url)
add_claim(:phone_number_landline)
add_claim(:phone_number_mobile)
end

add_claim(:membership_years, scope: :with_roles)
add_claim(:user_groups, scope: :user_groups)
Expand All @@ -61,9 +64,9 @@ def membership_verify_url(owner)
People::Membership::VerificationQrCode.new(owner).verify_url if owner.sac_membership_anytime?
end

def phone(owner)
owner.phone_numbers.order(:id).find_by(label: SacCas.main_phone_label)&.number
end
def phone_number_landline(owner) = owner.phone_number_landline&.number

def phone_number_mobile(owner) = owner.phone_number_mobile&.number

def membership_years(owner)
owner.membership_years
Expand Down
24 changes: 24 additions & 0 deletions app/models/concerns/sac_phone_numbers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

# Copyright (c) 2025, Schweizer Alpen-Club. This file is part of
# hitobito_sac_cas and licensed under the Affero General Public License version 3
# or later. See the COPYING file at the top-level directory or at
# https://github.com/hitobito/hitobito_sac_cas

# In the sac wagon, we allow only a predefined set of phone numbers with their distinct label.
# For those we always show the form fields, even if the records don't exist yet.
# To simplify the form handling, we define a `has_one` association for each predefined number.
module SacPhoneNumbers
def self.prepended(base)
PhoneNumber.predefined_labels.each do |label|
phone_number_assoc = :"phone_number_#{label}"
# rubocop:disable Rails/HasManyOrHasOneDependent (handled on has_many :phone_numbers)
# rubocop:disable Rails/InverseOf (association not defined on opposite side)
base.has_one phone_number_assoc, -> { where(label: label) },
class_name: "PhoneNumber", as: :contactable
# rubocop:enable Rails/HasManyOrHasOneDependent, Rails/InverseOf

base.accepts_nested_attributes_for phone_number_assoc, allow_destroy: true
end
end
end
30 changes: 21 additions & 9 deletions app/models/sac_cas/event/participation_contact_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,39 +7,51 @@

module SacCas::Event::ParticipationContactData
extend ActiveSupport::Concern
include AssignsSacPhoneNumbers

prepended do
attr_reader :event

delegate :salutation_label, to: :person
delegate :phone_number_mobile, :build_phone_number_mobile,
:phone_number_landline, :build_phone_number_landline,
to: :person

delegate :subsidy?, :subsidizable?, to: :participation

class << self
delegate :human_attribute_name, to: Wizards::Steps::Signup::PersonFields
end

self.contact_attrs = [:first_name, :last_name, :email, :address_care_of, :street, :housenumber,
:postbox, :zip_code, :town, :country, :gender, :birthday, :phone_numbers]
self.contact_attrs = [:first_name, :last_name, :email, :address_care_of, :street,
:housenumber, :postbox, :zip_code, :town, :country, :gender, :birthday,
:phone_number_mobile, :phone_number_landline]

self.mandatory_contact_attrs = [:email, :first_name, :last_name, :birthday, :street,
:housenumber, :zip_code, :town, :country]
end

def initialize(event, person, model_params = {})
super
mark_phone_numbers_for_destroy(person)
end

private

def participation
@participation ||= Event::Participation.new(event: @event, person: @person)
end

def assert_required_contact_attrs_valid
super.tap do
person.phone_numbers.first.valid?
super

message = [
Wizards::Steps::Signup::PersonFields.human_attribute_name(:phone_number), t("errors.messages.blank")
].join(" ")
errors.add(:base, message) if person.phone_numbers.first.number.blank?
end
# Ensure that at least one phone number is present
return if PhoneNumber.predefined_labels
.map { |label| person.send(:"phone_number_#{label}") }
.select { |phone_number| !phone_number&.marked_for_destruction? }
.any?

message = [PhoneNumber.model_name.human, t("errors.messages.blank")].join(" ")
errors.add(:base, message)
end
end
5 changes: 5 additions & 0 deletions app/models/sac_cas/phone_number.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ module SacCas::PhoneNumber
included do
after_create :check_data_quality
after_destroy :check_data_quality

validates :label,
inclusion: {in: PhoneNumber.predefined_labels},
uniqueness: {scope: [:contactable_type, :contactable_id]},
allow_blank: false
end

private
Expand Down
4 changes: 2 additions & 2 deletions app/models/wizards/steps/signup/person_common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
module Wizards::Steps::Signup::PersonCommon
extend ActiveSupport::Concern

PHONE_NUMBER_LABEL = "Mobil"
PHONE_NUMBER_LABEL = "mobile"

included do
class_attribute :minimum_age, default: SacCas::Beitragskategorie::Calculator::AGE_RANGE_MINOR_FAMILY_MEMBER.begin
Expand All @@ -35,7 +35,7 @@ def person_attributes
attributes.compact.symbolize_keys.except(:phone_number).then do |attrs|
next attrs if phone_number.blank?

attrs.merge(phone_numbers_attributes: [{label: PHONE_NUMBER_LABEL, number: phone_number, id: phone_number_id}.compact])
attrs.merge(phone_number_mobile_attributes: {number: phone_number, id: phone_number_id})
end
end

Expand Down
10 changes: 10 additions & 0 deletions app/views/contactable/_phone_number_fields.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
- # Copyright (c) 2025, Schweizer Alpen-Club. This file is part of
- # hitobito_sac_cas and licensed under the Affero General Public License version 3
- # or later. See the COPYING file at the top-level directory or at
- # https://github.com/hitobito/hitobito_sac_cas.

%div.d-flex.flex-wrap.w-100.align-items-center
%div.col-11.col-md-5.me-3.mb-1
= f.input_field(:number, placeholder: PhoneNumber.human_attribute_name(:number))
%div.col-11.col-md-4.d-flex.flex-row.me-3
= f.label(:number, f.object.translated_label)
11 changes: 11 additions & 0 deletions app/views/contactable/_phone_numbers_fields.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
- # Copyright (c) 2025, Schweizer Alpen-Club. This file is part of
- # hitobito_sac_cas and licensed under the Affero General Public License version 3
- # or later. See the COPYING file at the top-level directory or at
- # https://github.com/hitobito/hitobito_sac_cas.

= f.labeled :phone_numbers do
- PhoneNumber.predefined_labels.each do |label|
- assoc = "phone_number_#{label}"
- number = entry.send(assoc) || entry.send("build_#{assoc}")
= f.fields_for assoc, number do |fields|
- render 'contactable/phone_number_fields', f: fields
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,7 @@
= f.labeled_input_fields :first_name, :last_name, :email
= f.labeled_date_field :birthday
= render 'contactable/address_fields', f: f
- phone_number = entry.phone_numbers.first || PhoneNumber.new(label: 'Mobile', public: true)
= f.fields_for(:phone_numbers, phone_number) do |ff|
= ff.labeled(:number, Wizards::Steps::Signup::PersonFields.human_attribute_name(:phone_number)) do
= ff.input_field(:number)
= ff.hidden_field(:public)
= ff.hidden_field(:translated_label)
= ff.hidden_field(:_destroy, value: false)
= render 'contactable/phone_numbers_fields', f: f
= render('people/privacy_policy_acceptance_field', policy_finder: @policy_finder, f: f)

.well.align-with-form=t('.overrides_person_data_info')
15 changes: 6 additions & 9 deletions config/locales/wagon.de.yml
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,8 @@ de:
attributes:
contact_account:
predefined_labels:
andere: Andere
arbeit: Arbeit
fax: Fax
"haupt-telefon": Haupt-Telefon
mobil: Mobil
mutter: Mutter
privat: Privat
vater: Vater
webseite: Webseite
landline: Festnetz
mobile: Mobil
cost_center:
label: Bezeichnung
event_kind_categories: Kurskategorien
Expand Down Expand Up @@ -407,6 +400,10 @@ de:
m: Herr
w: Frau
additional_information: Bemerkungen (z.B. Beruf)
person/phone_number_mobile:
number: Mobiltelefon
person/phone_number_landline:
number: Festnetztelefon
group:
type: Gruppentyp technisch
group_id: Gruppen-ID
Expand Down
11 changes: 3 additions & 8 deletions config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,9 @@ social_account:

phone_number:
predefined_labels:
- Haupt-Telefon
- Privat
- Mobil
- Arbeit
- Vater
- Mutter
- Fax
- Andere
- --
- landline
- mobile

impersonate:
notify: false
Loading

0 comments on commit 01d5a53

Please sign in to comment.