search

orzubalsky / tradeschool

Trade School is an alternative school that runs on barter
Other
16 stars 5 forks source link

Export event calendar to icalendar format #146

Closed dnephin closed 10 years ago

dnephin commented 10 years ago

This change is branched off of dnephin:some_build_things so the diff includes those changes.

This branch adds an export for courses to icalendar format. It uses webcal:// scheme so things like google calendar should know to update the calendar with new events as they get added.

Right now the link to calendar is unstyled, I could use some help with the style for the link and maybe even placement.

I've only added it to branch_default, but I noticed there was also a branch_base. What is the difference between these? Does it need to be added to both?

orzubalsky commented 10 years ago

I checked this out locally and it looks wonderful Daniel!

The template files in branch_default extend those in branch_base. When a new Trade School is created in the system, a new template folder is created for the school, and all of the templates are copied from branch_default.

I would edit branch_base/course_list and wrap the link in a template block:

{% block export_calendar %}
<a href="webcal://{{branch.domain}}{% url 'event-calendar' branch_slug=branch.slug %}">
        Export iCal</a>
{% endblock export_calendar %}

And then edit branch_default/course_list and include the block there, so it's easy to remove if anyone decides they don't want it:

{% block export_calendar %}
{{ block.super }}
{% endblock export_calendar %}

I hope that makes sense.

I'm not too familiar with webcal. Is it possible to export a single class? If it is, it would be great to include that in the confirmation page that students see after registering to a class.

Thank you again for all of this!

dnephin commented 10 years ago

Cool, I can make those template changes. The current url is for all the courses at a branch, but it would be easy to add another one for "add this course to your calendar" and add a link to the confirmation page. I'll add that as well.

Do you think the other branch #145 is ready to be merged?

orzubalsky commented 10 years ago

Yes, I think it is! I thought that this one included those commits as well. Should I go ahead and merge #145 first?

dnephin commented 10 years ago

Yes, I think it would be a good idea to merge that one first. Thanks!

Edit: I have another branch off of #145 as well, and I think it will be easier to see the diffs in the PR with that merged

orzubalsky commented 10 years ago

Thanks for working on the travis setup, it looks neat!

Let me know when you're ready with this one.

brittanyukulele commented 10 years ago

This is awesome!!! Will we be notified when this new change is in effect?

dnephin commented 10 years ago

Or will have to comment on that, I'm not sure how releases are done. We can definitely add a comment to this pull request when it is ready to go. I hope to get some time to work on this feature later this week.

dnephin commented 10 years ago

Ok, I moved the link to the base template and changed the wording slightly. I also added some style so that the link doesn't disappear on hover (the color was matching the background).

I think this could really use a bit of extra styling which is definitely not my strength. If you want to pretty it up, please do. If you're ok with how it looks, it works for me.

I'm going to see about adding the the calendar event link to the confirmation page in another branch, so I think this one is ready to merge if you're happy with it.

dnephin commented 10 years ago

Ok, I believe this resolves #144. Let me know if you'd like me to change anything.

I've added the confirmation calendar export to this branch as well (and changed the name of the other calendar to branch-calendar).

I did run into a few issues while trying to test the confirmation page. I noticed that I have to follow the ajax page flow to see that page. When I tried to remove signups from the database it seemed like they were cached somewhere. This might have something to do with how dajax works, I'm not sure what was happening.

orzubalsky commented 10 years ago

Sorry it took me so long, I had a very busy couple of months but now I have more time!

There was just one problem with https://github.com/orzubalsky/tradeschool/blob/master/ts/apps/tradeschool/calendar/export.py lines 12 & 19, where the string returned was not in unicode, so certain characters broke it. I fixed that, merged this and pushed it to the live site.

I'm going to go over other pull requests and issues soon. Thank you for the work and patience!

dnephin commented 10 years ago

Oops, unicode always gets me in python2!

We should update the tests to use unicode characters I guess.

Thanks for fixing it.