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.

Sign up for GitHub

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

Jump to bottom

Implement phone number changes (#1563) #1634

Merged
merged 2 commits into from
Feb 13, 2025
Merged

Implement phone number changes (#1563) #1634

merged 2 commits into from
Feb 13, 2025

Conversation

daniel-illi
Copy link
Contributor

  • 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"

fixes #1563

@daniel-illi daniel-illi force-pushed the feature/sac/1563/phone-number-changes branch 4 times, most recently from 9b68b6c to 2f88878 Compare February 12, 2025 16:52
@daniel-illi
Copy link
Contributor Author

PR braucht core branch feature/sac/1563/phone-number-changes damit die Tests durchlaufen

@daniel-illi daniel-illi marked this pull request as ready for review February 12, 2025 17:01
@daniel-illi daniel-illi force-pushed the feature/sac/1563/phone-number-changes branch 4 times, most recently from c07d4be to c994136 Compare February 13, 2025 11:54
amaierhofer
amaierhofer approved these changes Feb 13, 2025
View reviewed changes
Copy link
Contributor

@amaierhofer amaierhofer left a comment

Choose a reason for hiding this comment

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

LGTM, ich frage mich noch ob wir nicht phone fürs erste im oidc claim behalten müssten uns hab noch ein paar Anregungen zu den specs hinterlassen

app/domain/sac_cas/oidc_claim_setup.rb Show resolved Hide resolved
app/domain/sac_cas/oidc_claim_setup.rb Show resolved Hide resolved
spec/features/event/sign_up_spec.rb Outdated Show resolved Hide resolved
spec/features/event_participation_spec.rb Outdated Show resolved Hide resolved
spec/features/groups/edit_spec.rb Outdated Show resolved Hide resolved
spec/features/people/edit_spec.rb Outdated Show resolved Hide resolved
spec/models/event/participation_contact_data_spec.rb Outdated Show resolved Hide resolved
spec/models/wizards/steps/signup/sektion/family_fields_spec.rb Show resolved Hide resolved
spec/models/wizards/steps/signup/sektion/person_fields_spec.rb Show resolved Hide resolved
app/domain/sac_cas/oidc_claim_setup.rb Show resolved Hide resolved
@daniel-illi daniel-illi force-pushed the feature/sac/1563/phone-number-changes branch from c994136 to 9bd7eff Compare February 13, 2025 13:02
@amaierhofer
Copy link
Contributor

Beim durchclicken ist mir noch eine kleine Unstimmigkeit beim Event aufgefallen und zwar werden die Telefonnummerfelder im Fehlerfall nicht rot markiert

image

@daniel-illi daniel-illi force-pushed the feature/sac/1563/phone-number-changes branch 2 times, most recently from a59be5b to 56ba4c8 Compare February 13, 2025 15:52
@daniel-illi
Implement phone number changes (#1563)
647c814 
* 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"

fixes #1563
@daniel-illi
Implement review feedback changes
5202e31
@daniel-illi daniel-illi force-pushed the feature/sac/1563/phone-number-changes branch from 56ba4c8 to 5202e31 Compare February 13, 2025 16:19
@daniel-illi daniel-illi enabled auto-merge (squash) February 13, 2025 16:25
@daniel-illi daniel-illi merged commit 01d5a53 into master Feb 13, 2025
6 checks passed
@daniel-illi daniel-illi deleted the feature/sac/1563/phone-number-changes branch February 13, 2025 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amaierhofer amaierhofer amaierhofer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Telefonnummern bereinigen inkl. Adressdatenexport für Sektionen (http://ftpserver.sac-cas.ch/)
2 participants
@daniel-illi @amaierhofer