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.

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

Events als ICAL exportierbar machen #201

Merged
merged 18 commits into from Feb 23, 2018

Conversation

Projects
None yet
5 participants
@diegosteiner
Contributor

diegosteiner commented Mar 18, 2017

Idee

Die Events aus hitobito sollen einfach in ein externes Kalenderprogramm exportiert werden können. Zu diesem Zweck sollen die Events im ICAL-Format zur Verfügung gestellt werden.

Umsetzung

  • Erweiterung des Event-Models mit einer Methode ical_events
  • Erweiterung des EventsController mit einer Methode render_ical
  • Erweiterung der Views mit einem Button "Kalender Export"

image

Für die Generierung des ICAL-Formates wird das Gem icalendar verwendet.

diegosteiner added some commits Mar 18, 2017

@@ -425,3 +425,6 @@ Style/SymbolProc:
Style/UnneededInterpolation:
Enabled: false
Style/FrozenStringLiteralComment:

This comment has been minimized.

@diegosteiner

diegosteiner Mar 18, 2017

Contributor

Added this since rubocop complained for me. See rubocop-hq/rubocop#3284

This comment has been minimized.

@kronn

kronn Mar 22, 2017

Member

I wonder why this was not already in the file.

@@ -20,6 +20,14 @@ def new_event_button
end
end
def export_events_ical_button

This comment has been minimized.

@diegosteiner

diegosteiner Mar 18, 2017

Contributor

Unfortunately, I did not find how you want this helper tested. I'll add a test if you can provide me this information.

This comment has been minimized.

@kronn

kronn Mar 22, 2017

Member

Currently, there is no spec-file for the GroupsHelper, I would create one in spec/helpers/groups_helper_spec.rb and take inspiration from https://github.com/hitobito/hitobito/blob/master/spec/helpers/filter_navigation/people_spec.rb, which also handles the cancan-mocking. Does this help?

@RolandStuder RolandStuder requested a review from kronn Mar 20, 2017

@kronn

All in all, I like how this is done.

I would like to see a test for the helper and the model/ical-spec integrated in the main event_spec.rb.

require 'spec_helper'
describe Event do

This comment has been minimized.

@kronn

kronn Mar 22, 2017

Member

I would put this in the main event_spec.rb file. The other files in the directory are separate classes, not aspects. Putting this spec in the main event-spec would keep the structure consistent with the convention.

@@ -425,3 +425,6 @@ Style/SymbolProc:
Style/UnneededInterpolation:
Enabled: false
Style/FrozenStringLiteralComment:

This comment has been minimized.

@kronn

kronn Mar 22, 2017

Member

I wonder why this was not already in the file.

@@ -20,6 +20,14 @@ def new_event_button
end
end
def export_events_ical_button

This comment has been minimized.

@kronn

kronn Mar 22, 2017

Member

Currently, there is no spec-file for the GroupsHelper, I would create one in spec/helpers/groups_helper_spec.rb and take inspiration from https://github.com/hitobito/hitobito/blob/master/spec/helpers/filter_navigation/people_spec.rb, which also handles the cancan-mocking. Does this help?

def show
respond_to do |format|
format.html { entries }
format.ics { render_ical(entries) }

This comment has been minimized.

@codez

codez Mar 27, 2017

Contributor

There are no entries involved in the show action, just an entry. Refer to CrudController for the corresponding implementation.

@@ -258,6 +258,19 @@ def course_kind?
kind_class == Event::Kind && kind.present?
end
def ical_events

This comment has been minimized.

@codez

codez Mar 27, 2017

Contributor

Instead of putting everything into model files (fat model anti-pattern), create a specific class in app/domain/export/ical. This is where export formats for different models are already handled.

diegosteiner added some commits Sep 18, 2017

@diegosteiner

This comment has been minimized.

Contributor

diegosteiner commented Oct 2, 2017

@kronn: I've changed the things you requested (at least what I thought you wanted)

@RolandStuder RolandStuder moved this from Ready to In Code Review in Sprint September 2017 Oct 3, 2017

@RolandStuder RolandStuder moved this from In Code Review to Implemented in Sprint September 2017 Oct 3, 2017

@kronn

kronn approved these changes Oct 25, 2017

@kronn kronn moved this from Implemented to In Code Review in Sprint September 2017 Oct 25, 2017

@kronn kronn moved this from In Code Review to Code Reviewed in Sprint September 2017 Oct 25, 2017

@diegosteiner

This comment has been minimized.

Contributor

diegosteiner commented Dec 6, 2017

Ich habe die Mergekonflikte, die inzwischen entstanden sind behoben. Gibt es noch etwas Anderes, dass diesem PR im Wege steht?

@RolandStuder

This comment has been minimized.

Member

RolandStuder commented Dec 6, 2017

@kronn Ist das gut zum Mergen?

@kronn

This comment has been minimized.

Member

kronn commented Dec 7, 2017

@RolandStuder Ja, von mir aus kann das gemerged werden. Wenn kein Veto kommt würde ich es gerne bald mergen. Dafür hätte ich irgendwie gerne ein 👍 , technisch sehe ich keine Probleme.

@RolandStuder

This comment has been minimized.

Member

RolandStuder commented Jan 16, 2018

@amaierhofer Bitte mergen, habe mir das angeschaut, sieht gut aus.

@RolandStuder RolandStuder assigned amaierhofer and unassigned kronn Feb 16, 2018

@RolandStuder RolandStuder removed this from Code Reviewed in Sprint September 2017 Feb 16, 2018

@RolandStuder RolandStuder added this to Backlog in Aktuelle Arbeiten via automation Feb 16, 2018

@RolandStuder

This comment has been minimized.

Member

RolandStuder commented Feb 16, 2018

@amaierhofer bitte noch einen Changelog Eintrag für die Ergänzung machen und mergen.

@RolandStuder RolandStuder moved this from Backlog to Ready in Aktuelle Arbeiten Feb 16, 2018

@amaierhofer amaierhofer merged commit 059a781 into hitobito:master Feb 23, 2018

@amaierhofer

This comment has been minimized.

Contributor

amaierhofer commented Feb 23, 2018

Thanks

@amaierhofer amaierhofer moved this from Ready to In Progress in Aktuelle Arbeiten Feb 23, 2018

@amaierhofer amaierhofer moved this from In Progress to Implemented in Aktuelle Arbeiten Feb 23, 2018

psunix pushed a commit that referenced this pull request Mar 28, 2018

Merge pull request #201 from diegosteiner/feature/event_ical
Events als ICAL exportierbar machen

@RolandStuder RolandStuder added this to the Release 1.18 milestone Apr 2, 2018

@Chronyms Chronyms referenced this pull request Apr 6, 2018

Closed

iCal fehler Outlook #518

@RolandStuder RolandStuder removed this from Implemented in Aktuelle Arbeiten May 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment