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

Feature/sac 574 export family recipients #904

Merged
merged 4 commits into from
Sep 16, 2024
Merged

Feature/sac 574 export family recipients #904

merged 4 commits into from
Sep 16, 2024

Conversation

TheWalkingLeek
Copy link
Member

No description provided.

@TheWalkingLeek TheWalkingLeek force-pushed the feature/sac_574_export_family_recipients branch 3 times, most recently from 2243282 to b0a2296 Compare September 3, 2024 12:42
@TheWalkingLeek TheWalkingLeek linked an issue Sep 3, 2024 that may be closed by this pull request
ABOS: Empfängerliste Haushaltsexport für physischen Versand #574
Closed
7 tasks
@hunchr hunchr force-pushed the feature/sac_574_export_family_recipients branch from b0a2296 to 45d0158 Compare September 4, 2024 07:32
@hunchr
Copy link
Contributor

hunchr commented Sep 10, 2024 •
edited
Loading

bei mir geht der export job nicht, wenn man via abos exportiert. reproduce:

  1. http://localhost:3000/de/groups/1/mailing_lists/2/subscriptions
  2. Export > CSV > Empfänger Familien
  3. Error:
[Worker(host:6bd170f2183d pid:1)] Job Export::SubscriptionsJob (id=714) RUNNING
super: no superclass method `name' for #<Group::Sektion >
/usr/src/app/hitobito/app/models/group.rb:372:in `name'
/usr/src/app/hitobito/app/models/group.rb:224:in `to_s'
/usr/src/app/hitobito/app/domain/export/tabular/people/person_row.rb:35:in `layer_group'
/usr/src/app/hitobito/app/domain/export/tabular/row.rb:33:in `value_for'
...
  1. Anderer Error? (noch nicht versucht zu reproduzieren)
[Worker(host:900ec1ebaa5d pid:1)] Job Export::SubscriptionsJob (id=1128) RUNNING
missing attribute: household_key
/usr/src/app/hitobito_sac_cas/app/domain/people/sac_membership.rb:114:in `family_id'
/usr/src/app/hitobito_sac_cas/app/models/sac_cas/person.rb:33:in `family_id'
/usr/src/app/hitobito/app/domain/export/tabular/row.rb:35:in `value_for'
/usr/src/app/hitobito/app/domain/export/tabular/row.rb:24:in `fetch'
/usr/src/app/hitobito/app/domain/export/tabular/base.rb:92:in `block in values'
/usr/src/app/hitobito/app/domain/export/tabular/base.rb:69:in `block in data_rows'
/usr/src/app/hitobito/app/domain/export/csv/generator.rb:40:in `block in generate'
/usr/src/app/hitobito/app/domain/export/tabular/base.rb:18:in `export'
/usr/src/app/hitobito/app/jobs/export/export_base_job.rb:42:in `data'
/usr/src/app/hitobito/app/jobs/export/subscriptions_job.rb:17:in `data'
/usr/src/app/hitobito_sac_cas/app/jobs/sac_cas/export/subscriptions_job.rb:14:in `data'
/usr/src/app/hitobito/app/jobs/export/export_base_job.rb:38:in `export_file'
/usr/src/app/hitobito/app/jobs/export/export_base_job.rb:22:in `perform'

@mtnstar mtnstar force-pushed the feature/sac_574_export_family_recipients branch from 45d0158 to 8c248af Compare September 12, 2024 07:46
mtnstar
mtnstar suggested changes Sep 12, 2024
View reviewed changes
Copy link
Contributor

@mtnstar mtnstar left a comment

Choose a reason for hiding this comment

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

sieht grundsätzlich super aus 🥳

  • household ist eigentlich sac_family im sac wagon, wir haben hier aber generell ein riesen Begriffwirwar und ich weiss auch nicht was hier vom Naming her am sinnvollsten ist. ich hätte vermutlich z.B. in der row sac_family verwendet.
  • die it statements in der row spec finde ich sehr unleserlich
  • den Export habe ich getestet und das sieht aus meiner Sicht super aus 🚀

app/domain/export/tabular/people/sac_recipient_household_row.rb
household.find { _1.email.present? } || household.first
end

def household
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def household
def sac_family_members

beim SAC entspricht ja der haushalt -> familie

ich habe gerade nicht wirklich eine starke Meinung dazu welchen Begriff wir hier beim SAC wagon verwenden sollten. einfach mal so als input/idee

spec/domain/export/tabular/people/sac_recipient_household_row_spec.rb
email: "[email protected]"
)
end
let(:family_member) do
Copy link
Contributor

Choose a reason for hiding this comment

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

es gibt bereits familienmitglied in den fixtures, das kann hier nicht verwendet werden, oder?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ich mags fast mehr dass alle Felder explizit im Spec file genannt werden weil ich die ja dann alle expecte
Zudem muss ich je nachdem sowieso den household_key anpassen und so, find ich dann angenehm wenn ich nicht an einer fixture Person rumeditiere

spec/domain/export/tabular/people/sac_recipient_household_row_spec.rb Outdated
Comment on lines 46 to 57
it("id") { expect(value(:id)).to eq 42 }
it("salutation") { expect(value(:salutation)).to be_nil }
it("first_name") { expect(value(:first_name)).to eq "Hans" }
it("last_name") { expect(value(:last_name)).to eq "Muster" }
it("adresszusatz") { expect(value(:adresszusatz)).to be_nil }
it("address") { expect(value(:address)).to eq "Musterstrasse 42" }
it("postfach") { expect(value(:postfach)).to be_nil }
it("zip_code") { expect(value(:zip_code)).to eq "4242" }
it("town") { expect(value(:town)).to eq "Musterhausen" }
it("email") { expect(value(:email)).to eq "[email protected]" }
it("layer_navision_id") do
expect(value(:layer_navision_id)).to eq groups(:bluemlisalp).navision_id
Copy link
Contributor

Choose a reason for hiding this comment

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

ich fände das jetzt einiges leserlicher wenn man einfach die row als hash vergleicht als das man hier it so viel verwendet. hier hat man so viele expect und value aufrufe

@TheWalkingLeek TheWalkingLeek force-pushed the feature/sac_574_export_family_recipients branch from b874903 to a10ef94 Compare September 13, 2024 08:40
@TheWalkingLeek TheWalkingLeek force-pushed the feature/sac_574_export_family_recipients branch from a94bbfe to 5fcfb2c Compare September 16, 2024 05:59
@TheWalkingLeek TheWalkingLeek merged commit f12a179 into master Sep 16, 2024
5 checks passed
@TheWalkingLeek TheWalkingLeek deleted the feature/sac_574_export_family_recipients branch September 16, 2024 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtnstar mtnstar mtnstar requested changes

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

Successfully merging this pull request may close these issues.

ABOS: Empfängerliste Haushaltsexport für physischen Versand
3 participants
@TheWalkingLeek @hunchr @mtnstar