hitobito / hitobito_jubla

A hitobito wagon defining the organization hierarchy and additional features for Jungwacht Blauring Schweiz.
Other
15 stars 13 forks source link

Statistik Exportbutton #86

Closed patriziajubla closed 1 month ago

patriziajubla commented 7 months ago

Mehrere Buttons hinzugefügt und angepasst im Reiter Statistik

richardjubla commented 7 months ago

(bumping issue) Schafft es dieser pull noch in den nächsten Release? 😇

ThomasEllenberger commented 7 months ago

Lieber @richardjubla

Merci für den Bump. :)

Wir haben uns das ganz kurz angeschaut: Auf den ersten Blick müsste man wohl im Code noch kleinere Anpassungen vornehmen, sowie die Specs für den Change schreiben. Dies ist desshalb ein PR, welchen wir nicht noch während dem Mergefreeze mergen möchten.

Gerne können wir einmal gemeinsam anschauen, ob ihr dies noch fertig machen wollt, oder ob wir das übernehmen sollen.

richardjubla commented 6 months ago

@patriziajubla

Wir wollen den PR mit Anwendungsfälle/Use Cases ergänzen, damit wir ihn umsetzten können. Damit können wir zwischen uns und Puzzle sicherstellen, dass alles wie gewollt "korrekt" funktioniert. Testen, Abnehmen und Specs schreiben.

patriziajubla commented 4 months ago

@ThomasEllenberger Hier die gewünschten Specs:

Problembeschreibung

Als Verantwortliche der Versicherung und der Statistik möchten wir die bestehenden und neuen Exporte erweitern und hinzufügen, um zusätzliche Datenpunkte einzuschließen und neue Exportmöglichkeiten zu bieten.

Anforderungen:

  1. Erweiterung des bestehenden Exports:

    • Ort: census/federation im Tab Statistik
    • Button: Export CSV Versicherungen
    • Änderungen:
      • Inhalt Region und Kanton hinzufügen.
      • Inhalt Name durch Schar ersetzen.
      • Hinzufügen einer Bedingung in der format.csv-Sektion der index-Methode der CensusEvaluation::FederationController, um basierend auf params[:type] entweder Export::Tabular::CensusFlock.csv(year) oder Export::Tabular::CensusFlockFederation.csv(year) zu verwenden.
  2. Erstellung neuer Exporte:

    • Export 1:

      • Ort: census/federation im Tab Statistik
      • Button: Export CSV Kantone
      • Inhalt: Kanton, Region, Schar, Leitende, Kinder
      • Implementierung der Logik in Export::Tabular::CensusFlockFederation.csv(year).
    • Export 2:

      • Ort: census/state im Tab Statistik
      • Button: Export CSV Scharen
      • Inhalt: Details über die Scharen
      • Hinzufügen einer neuen index-Methode in CensusEvaluation::StateController, die den CSV-Export für Scharen (Export::Tabular::CensusFlockState.csv(year)) bereitstellt.
  3. Anpassung der Ansichtslogik für Buttons:

    • Die Ansicht für die Buttons in census/federation wurde aktualisiert:
      - if can?(:create, Census.new)
      = action_button('Export CSV Kantone', params.merge(format: :csv), :download)
      = action_button('Neue Bestandesmeldungen einfordern', new_census_path, :bullhorn)
      = action_button('Export CSV Versicherungen', params.merge(format: :csv), :download)

Änderungen im Code:

  1. Erweiterung der index-Methode in CensusEvaluation::FederationController:

    def index
     super
    
     respond_to do |format|
       format.html do
         @flocks = flock_confirmation_ratios if evaluation.current_census_year?
       end
       format.csv do
         if params[:type] == 'kantone'
           authorize!(:create, Census)
           send_data Export::Tabular::CensusFlockFederation.csv(year), type: :csv
         else
           authorize!(:create, Census)
           send_data Export::Tabular::CensusFlock.csv(year), type: :csv
         end
       end
     end
    end
  2. Hinzufügen einer neuen index-Methode in CensusEvaluation::StateController:

    def index
     super
    
     respond_to do |format|
       format.html do
         # Implementiere entsprechende Logik für HTML-Ansicht, falls benötigt
       end
       format.csv do
         authorize!(:create, Census)
         send_data Export::Tabular::CensusFlockState.csv(year), type: :csv
       end
     end
    end

Testanweisungen

  1. Führe Tests durch, um sicherzustellen, dass die neuen Exporte und die geänderte index-Methode in beiden Controllern korrekt funktionieren.
  2. Überprüfe die Funktion der Bedingung in der index-Methode der CensusEvaluation::FederationController, um sicherzustellen, dass der richtige Export basierend auf params[:type] ausgewählt wird.
  3. Stelle sicher, dass die Buttons in census/federation gemäß den Berechtigungen korrekt angezeigt werden.

Zusätzliche Anmerkungen

Die Implementierung umfasst Anpassungen in den Exportklassen (Export::Tabular::CensusFlock, Export::Tabular::CensusFlockFederation, Export::Tabular::CensusFlockState) sowie die Aktualisierung der Ansichtslogik für die Buttons in census/federation.

richardjubla commented 2 months ago

@patriziajubla Ich behaupte, es ist "Bestandsmeldung" anstelle von Bestandesmeldung

ThomasEllenberger commented 2 months ago

@richardjubla Sollen wir uns dies entsprechend nochmal anschauen und mergen falls i.O? Würde auch diesen vorerst einmal auf 30 min Timeboxen falls das für dich stimmt.

richardjubla commented 2 months ago

@richardjubla Sollen wir uns dies entsprechend nochmal anschauen und mergen falls i.O? Würde auch diesen vorerst einmal auf 30 min Timeboxen falls das für dich stimmt.

Ja, gerne nochmal anschauen. Merci.

TheWalkingLeek commented 1 month ago

Ich sehe aktuell noch zwei grössere Punkte die für mich noch offen sind:

  1. Es gibt keine Tests. Weder Controller Tests für den Endpunkt, noch Tests für den Export und das Resultat des Exports
  2. Die Exports werden mit dieser Implementation komplett im Request gemacht. Das heisst währenddessen wird der Browser Tab komplett blockiert und wenn der Export zu lang geht wird die Request vom Nginx Proxy gekillt und das Resultat kommt nie an. State of the art im Hitobito ist dazu einen entsprechenden Delayed::Job zu schedulen, welcher dann den Export übernimmt: https://github.com/hitobito/hitobito/blob/master/app/jobs/export/people_export_job.rb

Der Code sieht in View/Controller eigentlich ok aus, der Exporter Code etwas overengineered auf den ersten Blick aber das ist ja ziemlich abgekapselt

richardjubla commented 1 month ago

@patriziajubla Habe veranlasst, dass der PR noch inklusive den gefundenen Hinweisen umgesetzt wird.

TheWalkingLeek commented 1 month ago

Wird hier weiter getracked/abgeschlossen:

127