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.

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

Support for attachment property #335

Closed
Timebutt opened this issue Dec 14, 2021 · 17 comments
Closed

Support for attachment property #335

Timebutt opened this issue Dec 14, 2021 · 17 comments

Comments

@Timebutt
Copy link

Timebutt commented Dec 14, 2021

Hi! I noticed the current ICalEvent and ICalEventData don't support the ATTACH property when creating an event through createEvent, as defined in the spec.

Are there any plans to add support, or should we get an implementation running using the last property of ICalEventData?

x?: {
        key: string;
        value: string;
    }[] | [string, string][] | Record<string, string>;

Thanks!

@sebbo2002
Copy link
Owner

So far there has been no demand for it and therefore no plans to implement it. Definitely makes sense though and I would look into it when I get the time. Honestly though, I have to say that this probably won't be the case until early next year.

If this is too long for you, feel free to use x() or you can prepare a pull request.

@Timebutt
Copy link
Author

Thanks for your reply ;) I'll have a look at how I'll set it up, and might propose a PR to implement this feature!

@sebbo2002
Copy link
Owner

If you have any questions, feel free to contact me here. I will try to answer them in a timely manner.

sebbo2002 added a commit that referenced this issue Jan 3, 2022
sebbo2002 added a commit that referenced this issue Jan 3, 2022
@sebbo2002 sebbo2002 mentioned this issue Jan 3, 2022
github-actions bot pushed a commit that referenced this issue Jan 3, 2022
# [3.2.0-develop.1](v3.1.1...v3.2.0-develop.1) (2022-01-03)

### Features

* **Events:** Add `createAttachment` / `attachments` ([12a382f](12a382f)), closes [#335](#335)
@sebbo2002
Copy link
Owner

🎉 This issue has been resolved in version 3.2.0-develop.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Timebutt
Copy link
Author

Timebutt commented Jan 3, 2022

Hey @sebbo2002, I see you added support for attachments, great! I was just wondering, is the url the only property you need for this to work? I actually started implementing this myself a little while back, and did things a bit differently. This is my branch (this is the commit to make that happen) and implementation. It has support for the fileName, on top of the url.

I haven't reported back yet, because I couldn't get it to work with iCal or Google Calendar (attachments wouldn't show up, maybe I got something wrong), did you experience the same issues? I'll give your new code a go and see where it differs!

@sebbo2002
Copy link
Owner

I stuck with the spec and tested it with Apple's Calendar. Has worked flawlessly. Google Calendar I have not tested because I do not use that, but I assume that they stick to the spec or not?

Bildschirmfoto 2022-01-03 um 19 40 55

@Timebutt
Copy link
Author

Timebutt commented Jan 3, 2022

AFAIK they do yes, looks like I made a mistake on my end then, will have a look and see if anything might need changes for Google Calendar 👌

@sebbo2002
Copy link
Owner

Sounds good. Then I wait with the release (#339) until you have tested it with Google Calendar and given the okay.

@sebbo2002 sebbo2002 reopened this Jan 3, 2022
@Timebutt
Copy link
Author

Timebutt commented Jan 3, 2022

K just tested this, and I see it works when directly importing the .ics file (as did my solution). It doesn't work for me, when I fetch it from a remote location through a subscription, which is what my application aims to achieve. Will dig in a bit deeper if I can make it work that way, but basically the functionality is correct 👌

@sebbo2002
Copy link
Owner

Oh wow. And with filename it works? Thanks for your research.

@Timebutt
Copy link
Author

Timebutt commented Jan 4, 2022

No, the presence of a file name or even the FMTTYPE property makes no difference from what I've found so far :/

@sebbo2002
Copy link
Owner

Maybe the embedding of files directly works? I thought I leave that out, because the feeds are really unattractive large, but if it does not work otherwise we should adjust that again.

@Timebutt
Copy link
Author

Timebutt commented Jan 4, 2022

Might work, although I can imagine they'll strip that as well, on top of the the very undesirable fact that the calendar file might become huge as well :/

@sebbo2002
Copy link
Owner

Are there any updates here?

@Timebutt
Copy link
Author

Not yet, haven't been able to investigate more because of other work. I honestly don't think that it's related to the implementation of this package, rather in the way that both Google Calendar as well as iCal treat calendar subscriptions, feels like they might be sanitizing attachments for security reasons. I'll see if I get some more time in the near future to investigate, but meanwhile I'd argue this issue is done, as the implementation works great when importing generated .ics files!

@sebbo2002
Copy link
Owner

That makes sense. Then I'll mark this ticket as closed, then I can publish this within the next few days together with #340. If you find out anything in your investigation, feel free to let me know.

github-actions bot pushed a commit that referenced this issue Jan 12, 2022
# [3.2.0](v3.1.1...v3.2.0) (2022-01-12)

### Bug Fixes

* **Attendee:** Print RSVP also if rsvp is set to false ([27e5166](27e5166)), closes [#340](#340)

### Features

* **Events:** Add `createAttachment` / `attachments` ([12a382f](12a382f)), closes [#335](#335)
@sebbo2002
Copy link
Owner

🎉 This issue has been resolved in version 3.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

No branches or pull requests

2 participants