PERSON: Abo-Filter nach Alter und Sprache #290

diegosteiner · 2023-09-15T11:40:12Z

Resolves hitobito#1919

diegosteiner · 2023-09-15T11:41:46Z

app/domain/pbs/export/tabular/people/participation_nds_row.rb

-  def first_language_with_correspondence_language
-    lang = entry.correspondence_language.presence
-    lang ? lang.upcase : 'DE'
+  # TBD: move to youth wagon? https://github.com/hitobito/hitobito_youth/blob/3322054e2d64db19f8b049773e61f68e7f614546/app/domain/export/tabular/people/participation_nds_row.rb#L70


@carlobeltrame Das könnte meiner Meinung nach in den youth-Wagon übernommen werden. https://github.com/hitobito/hitobito_youth/blob/3322054e2d64db19f8b049773e61f68e7f614546/app/domain/export/tabular/people/participation_nds_row.rb#L70

Ja, finde ich auch, aber dann würde ich die Logik noch restriktiver schreiben. In der Spezifikation von J+S sind nur DE, FR, IT und Andere als Beispielwerte aufgeführt. Wenn wir das in den Youth Wagon verschieben gibt es potenziell noch mehr Sprachen (z.B. bei der Stiftung junger Auslandschweizer) die da in Zukunft mal zu ungültigen NDS-Exports führen könnten. Also wenn verschieben, dann lieber in einem Mapping nachschauen, und auf "Andere" zurückfallen wenns nicht de, fr oder it ist.

Erledigt im PR hitobito/hitobito_youth#35

app/jobs/event/camp_reminder_job.rb

app/serializers/pbs/mailing_list_subscriber_serializer.rb

config/settings.yml

diegosteiner · 2023-09-15T11:48:36Z

db/migrate/20230915105810_replace_correspondance_language_with_core_language.rb

@@ -0,0 +1,7 @@
+class ReplaceCorrespondanceLanguageWithCoreLanguage < ActiveRecord::Migration[6.1]
+  def change
+    Person.where(correspondence_language: nil).update_all(correspondence_language: :de)


@carlobeltrame hier liegt das für den Moment vermeitlich grösste Problem: das Feld correspondence_language kann null sein, während language not null ist. Damit wird dieses Feld neu ein Pflichtfeld, was ja jeweils auf pfadipolitischer Ebene nicht unumstritten ist. Ich nehme hier auch gleich @simfeld dazu.

Was meint ihr?

Ja gute Frage. Man könnte natürlich beginnen, den Kantonalverband der Haupt-Rolle jeder Person herauszusuchen, und anhand von dem die Sprache intelligent zu setzen... Kann man aber beliebig weit treiben weil es auch noch Kantone mit mehreren Sprachen gibt... Da hätte ich auch gerne einen Rat von @simfeld oder @Michael-Schaer wie weit wir hier gehen sollen.

Was ist denn die Default-Korrespondenzsprache beim Versand von E-Mails wenn man keine gesetzt hat? Ich nehme an auch Deutsch? Insofern wäre für mich folgendes Vorgehen in Ordnung:

Migration setzt per Default language auf DE

PBS-Release-Notes kündigen dies an und man kann das somit verhindern indem man seine Korrespondenzsprache setzt vor dem Release

Ich glaube die Mails achten bisher nicht auf die Korrespondenzsprache, könnte aber falsch liegen.
Deinen Vorschlag finde ich gut.

Swing, Cosinus und ich haben uns heute auch noch abgesprochen. Wenn möglich, wäre ein Matching nach Kanton gut. Da die KVs Kantone zugeordnet haben, sollte ein Matching gut möglich sein. Hier der Vorschlag für die Aufteilung:

DE
Aargau Appenzell Innerrhoden Appenzell Ausserrhoden Bern Basel-Land Basel-Stadt
Glarus Graubünden Luzern Nidwalden Obwalden St. Gallen Schaffhausen Solothurn Schwyz Thurgau Uri Zug Zürich

FR
Freiburg Genf Jura Waadt Wallis Neuenburg

IT
Tessin

Ich habe das noch entsprechend in der Tech Spec ergänzt

diegosteiner · 2023-10-20T09:29:59Z

db/migrate/20230915105810_replace_correspondance_language_with_core_language.rb

@@ -0,0 +1,23 @@
+class ReplaceCorrespondanceLanguageWithCoreLanguage < ActiveRecord::Migration[6.1]
+  CANTONS_LANGUAGE = {
+    # de: [:ag, :ai, :ar, :be, :bl, :bs, :gl, :gr, :lu, :nw, :ow, :sg, :sh, :so, :sz, :tg, :ur, :zg, :zh],


Das ist sowieso der default, kann ganz weggelassen werden

diegosteiner · 2023-10-20T09:32:16Z

db/migrate/20230915105810_replace_correspondance_language_with_core_language.rb

+  def infer_person_language(person)
+    return person.language if person.language.present?
+
+    canton = person.find_kantonalverband&.kantonalverband_cantons&.first&.to_sym


@carlobeltrame Ich habe im pbs wagon die Methode Person#find_kantonalverband gefunden, aber ich glaube sie berücksichtigt die gelöschten Rollen nicht wie in der Tech Spec vorgegeben. Was denkst du? soll ich diese Logik trotzdem nochmal bauen?

diegosteiner · 2023-10-20T12:32:45Z

app/domain/pbs/export/tabular/people/participation_nds_row.rb

-  def first_language_with_correspondence_language
-    lang = entry.correspondence_language.presence
-    lang ? lang.upcase : 'DE'
+  # TBD: move to youth wagon? https://github.com/hitobito/hitobito_youth/blob/3322054e2d64db19f8b049773e61f68e7f614546/app/domain/export/tabular/people/participation_nds_row.rb#L70


Erledigt im PR hitobito/hitobito_youth#35

TheWalkingLeek · 2023-12-11T12:49:25Z

db/migrate/20230915105810_replace_correspondance_language_with_core_language.rb

+  def change
+    Person.update_all("language = correspondence_language")
+    remove_column :people, :correspondence_language, :string, limit: 5
+
+    Person.where(language: nil).find_each do |person|
+      person.update!(language: infer_person_language(person))
+    end
+  end  


Denke hier macht es Sinn die #change Methode in eine #up und #down Methode zu ändern.

Und eine Spec für die Migration fände ich auch noch nice.
Beispiel: https://github.com/hitobito/hitobito_pbs/blob/master/spec/migrations/prolong_j_s_qualifications_2021_spec.rb

@TheWalkingLeek finde ich beides gut. Beim schreiben der migration spec bin ich auf folgendes Problem gestossen. In dieser Migration entferne ich das correspondence_language Feld, was das Testen enorm erschwert (wenn die DB auf dem letzten Stand ist, gibt es das Feld dann nicht mehr). Ich habe schon versucht, die Migration Spec so zu schreiben, dass die Migrationen danach down'd werden und dann getestet wird, aber es tauchen immer wieder Probleme auf. Damit ich hier nicht noch mehr Zeit verschwende, schlage ich vor einfach die #infer_person_language zu refactoren und diese dann zu testen, nicht die ganze Migration. Was meinst du?

Ja das klingt sinnvoll. Migrations-specs können dadurch manchmal etwas mühsam sein aber die meiste Logik passiert ja sowieso im #infer_person_language. Das wäre demnach meines Erachtens nach auch der wichtigste Teil für einen Test, sollte dann also passen :)

carlobeltrame · 2023-12-21T16:49:27Z

app/helpers/people_pbs_helper.rb

-  def format_person_salutation(person)
-    person.salutation_value
-  end


Ich glaube diese Methode dürfen wir nicht löschen, das wird noch gebraucht. Zwar nicht genau mit diesem Namen, aber im format_helper im Core irgendwo wird nach einer so benannten Methode gesucht, wenn ein salutation Feld auf einer Person gerendert wird.

carlobeltrame · 2023-12-21T17:34:37Z

app/domain/pbs/export/tabular/people/participation_nds_row.rb

-
-  included do
-    alias_method_chain :first_language, :correspondence_language


Wir müssen diese Methode weiterhin überschreiben, weil die Base-Implementation immer 'DE' zurückgibt (Jubla hat z.B. nur Gruppen in der Deutschschweiz).

diegosteiner changed the title ~~replace wagon specific correspondence_language with core language~~ [WIP] #1919 PERSON: Abo-Filter nach Alter und Sprache Sep 15, 2023

diegosteiner commented Sep 15, 2023

View reviewed changes

app/jobs/event/camp_reminder_job.rb Show resolved Hide resolved

diegosteiner commented Sep 15, 2023

View reviewed changes

app/serializers/pbs/mailing_list_subscriber_serializer.rb Outdated Show resolved Hide resolved

diegosteiner commented Sep 15, 2023

View reviewed changes

config/settings.yml Outdated Show resolved Hide resolved

diegosteiner commented Sep 15, 2023

View reviewed changes

diegosteiner force-pushed the #1919_abo-filter branch from 70a0cdb to c0b5af3 Compare September 21, 2023 12:47

diegosteiner commented Oct 20, 2023

View reviewed changes

diegosteiner mentioned this pull request Oct 20, 2023

add language mapping for nds export hitobito/hitobito_youth#35

Open

diegosteiner commented Oct 20, 2023

View reviewed changes

diegosteiner force-pushed the #1919_abo-filter branch from d37677e to d97649e Compare October 27, 2023 12:57

carlobeltrame linked an issue Nov 13, 2023 that may be closed by this pull request

PERSON: Abo-Filter nach Alter und Sprache hitobito/hitobito#1919

Closed

32 tasks

TheWalkingLeek requested changes Dec 11, 2023

View reviewed changes

diegosteiner force-pushed the #1919_abo-filter branch from d97649e to aae7c5a Compare December 15, 2023 14:29

diegosteiner changed the title ~~[WIP] #1919 PERSON: Abo-Filter nach Alter und Sprache~~ PERSON: Abo-Filter nach Alter und Sprache Dec 15, 2023

carlobeltrame approved these changes Dec 21, 2023

View reviewed changes

diegosteiner and others added 11 commits December 21, 2023 23:23

replace wagon specific correspondence_language with core language

451b4c9

keep deprecated correspondence_language for compatibility

ea4f52d

move nds language to youth wagon

e066d25

move nds language to youth wagon

d09928f

migrate pbs language field intelligently

083b13d

specs: add specs for migration

d2c05ee

spec: add specs for migration

adfd0d3

chore: update comment header

52a9dd8

Restore helper which is still in use

69af2df

Restore logic which is still necessary

70ee05e

Fix remaining tests

f08419e

carlobeltrame force-pushed the #1919_abo-filter branch from 20affc1 to 4f333ec Compare December 21, 2023 22:25

Adjust code which was changed in the meantime

e825a8b

carlobeltrame force-pushed the #1919_abo-filter branch from 4f333ec to e825a8b Compare December 21, 2023 22:36

carlobeltrame merged commit 7afcef4 into master Dec 21, 2023
5 checks passed

kronn deleted the #1919_abo-filter branch March 22, 2024 21:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PERSON: Abo-Filter nach Alter und Sprache #290

PERSON: Abo-Filter nach Alter und Sprache #290

diegosteiner commented Sep 15, 2023 •

edited

Loading

diegosteiner Sep 15, 2023

carlobeltrame Sep 18, 2023

diegosteiner Oct 20, 2023

diegosteiner Sep 15, 2023

carlobeltrame Sep 18, 2023

simfeld Sep 19, 2023

carlobeltrame Sep 21, 2023

Michael-Schaer Sep 30, 2023

carlobeltrame Oct 3, 2023

diegosteiner Oct 20, 2023

diegosteiner Oct 20, 2023

diegosteiner Oct 20, 2023

TheWalkingLeek Dec 11, 2023

diegosteiner Dec 15, 2023

TheWalkingLeek Dec 18, 2023

carlobeltrame Dec 21, 2023

carlobeltrame Dec 21, 2023 •

edited

Loading


		included do
		alias_method_chain :first_language, :correspondence_language

PERSON: Abo-Filter nach Alter und Sprache #290

PERSON: Abo-Filter nach Alter und Sprache #290

Conversation

diegosteiner commented Sep 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlobeltrame Dec 21, 2023 • edited Loading

Choose a reason for hiding this comment

diegosteiner commented Sep 15, 2023 •

edited

Loading

carlobeltrame Dec 21, 2023 •

edited

Loading