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

fix: omit zone in "AllDay" event helpers #67

Merged
merged 1 commit into from
Sep 29, 2024

Conversation

dnwe
Copy link
Contributor

@dnwe dnwe commented Apr 5, 2023

For a date-only event (i.e., an event that lasts for the full day) the iCalendar specification indicates that the value for DTSTART / DTEND should be a DATE

https://icalendar.org/iCalendar-RFC-5545/3-6-1-event-component.html

The "VEVENT" is also the calendar component used to specify an
anniversary or daily reminder within a calendar. These events have a
DATE value type for the "DTSTART" property instead of the default value
type of DATE-TIME. If such a "VEVENT" has a "DTEND" property, it MUST be
specified as a DATE value also

The DATE format
(https://icalendar.org/iCalendar-RFC-5545/3-3-4-date.html) should omit both time and zone/location elements and additionally notes that "The "TZID" property parameter MUST NOT be applied to DATE properties"

As per the specification, this PR also adds an explicit "VALUE=DATE" parameter when the AllDay helpers were called, to indicate that the property's default value type has been overridden and the VEVENT is intended to be an all-day event
https://icalendar.org/iCalendar-RFC-5545/3-2-20-value-data-types.html

Finally the SetDuration call has been updated to preserve the "AllDay" characteristics if the existing start or end has been specified in DATE format, which is also a requirement of the spec.

Contributes-to: #55

@dnwe
Copy link
Contributor Author

dnwe commented Apr 5, 2023

I knocked up this PR whilst reading the spec and debugging why using the all day helpers wasn't producing an all day event in my generated .ics files. After finding the repo to submit the PR I spotted #55 and the abandoned PR #61, so this sort of supersedes that

@dnwe
Copy link
Contributor Author

dnwe commented Apr 27, 2023

It seems #69 was raised and merged 2 days ago which resurrected #61 but doesn't fixup SetDuration 😔

@arran4
Copy link
Owner

arran4 commented Apr 27, 2023

@jackhopner Can you verify that this change fixes your problem too?

@dnwe So sorry. I'm hoping a couple more months I will have time to put more diligence into this.

For a date-only event (i.e., an event that lasts for the full day) the
iCalendar specification indicates that the value for DTSTART / DTEND
should be a DATE

https://icalendar.org/iCalendar-RFC-5545/3-6-1-event-component.html

> The "VEVENT" is also the calendar component used to specify an
> anniversary or daily reminder within a calendar. These events have a
> DATE value type for the "DTSTART" property instead of the default value
> type of DATE-TIME. If such a "VEVENT" has a "DTEND" property, it MUST be
> specified as a DATE value also

The DATE format
(https://icalendar.org/iCalendar-RFC-5545/3-3-4-date.html) should omit
both time and zone/location elements and additionally notes that "The
"TZID" property parameter MUST NOT be applied to DATE properties"

As per the specification, this PR also adds an explicit "VALUE=DATE"
parameter when the AllDay helpers were called, to indicate that the
property's default value type has been overridden and the VEVENT is
intended to be an all-day event
https://icalendar.org/iCalendar-RFC-5545/3-2-20-value-data-types.html

Finally the SetDuration call has been updated to preserve the "AllDay"
characteristics if the existing start or end has been specified in DATE
format, which is also a requirement of the spec.

Contributes-to: arran4#55
@dnwe
Copy link
Contributor Author

dnwe commented Apr 28, 2023

Yes it will do, it's essentially the same fix for that area. I've rebased the changes on top of latest master so it includes the additional tests and the fix for SetDuration

@arran4
Copy link
Owner

arran4 commented Sep 28, 2024

I have attempted to resolve the merge conflicts in #101 please review

@arran4 arran4 merged commit 0c815e1 into arran4:master Sep 29, 2024
arran4 added a commit that referenced this pull request Oct 15, 2024
* master:
  refactor: rename unused arg as '_'
  refactor: use consistent receiver names
  refactor: prefer switch for readability
  refactor: use ReplaceAll
  refactor: rename var to reflect what it is
  refactor: remove unnecessary named return values, harmonizing code base
  Minor update
  Rename default.md to pull_request_template.md
  Create default.md
  Create other.md
  Create bug.md
  Reintegration of #67
  fix: omit zone in "AllDay" event helpers

# Conflicts:
#	components.go
arran4 added a commit that referenced this pull request Oct 15, 2024
* master: (23 commits)
  refactor: rename unused arg as '_'
  refactor: use consistent receiver names
  refactor: prefer switch for readability
  refactor: use ReplaceAll
  refactor: rename var to reflect what it is
  refactor: remove unnecessary named return values, harmonizing code base
  Minor update
  Rename default.md to pull_request_template.md
  Create default.md
  Create other.md
  Create bug.md
  Reintegration of #67
  New tool
  Merged
  Add method to remove property
  Move targeted Go version to 1.20
  Remove deprecated ioutil
  Improve error for property not found
  Should be able to distinguish unset from invalid time properties
  fix: omit zone in "AllDay" event helpers
  ...

# Conflicts:
#	.github/workflows/test.yml
#	calendar.go
#	components.go
#	components_test.go
#	errors.go
@arran4
Copy link
Owner

arran4 commented Oct 15, 2024

Hi @dnwe is it possible for me to rename the SetDuration to SetStartEndByDuration or something like that?

Otherwise I will have to create a function called SetDurationActually which smells a bit more.

Reason? I'm reviewing: #70

Happy for suggestions too.

ManoloTonto1 pushed a commit to ManoloTonto1/golang-ical that referenced this pull request Oct 21, 2024
* Add leaks and vunerability checks

* Requires secrets now

* Renamed interface as it's useful own it's own. Made it public. Added a comment

* fix: omit zone in "AllDay" event helpers

For a date-only event (i.e., an event that lasts for the full day) the
iCalendar specification indicates that the value for DTSTART / DTEND
should be a DATE

https://icalendar.org/iCalendar-RFC-5545/3-6-1-event-component.html

> The "VEVENT" is also the calendar component used to specify an
> anniversary or daily reminder within a calendar. These events have a
> DATE value type for the "DTSTART" property instead of the default value
> type of DATE-TIME. If such a "VEVENT" has a "DTEND" property, it MUST be
> specified as a DATE value also

The DATE format
(https://icalendar.org/iCalendar-RFC-5545/3-3-4-date.html) should omit
both time and zone/location elements and additionally notes that "The
"TZID" property parameter MUST NOT be applied to DATE properties"

As per the specification, this PR also adds an explicit "VALUE=DATE"
parameter when the AllDay helpers were called, to indicate that the
property's default value type has been overridden and the VEVENT is
intended to be an all-day event
https://icalendar.org/iCalendar-RFC-5545/3-2-20-value-data-types.html

Finally the SetDuration call has been updated to preserve the "AllDay"
characteristics if the existing start or end has been specified in DATE
format, which is also a requirement of the spec.

Contributes-to: arran4#55

* calendar parsing url support

* usage example, README

* added functionnal option pattern for url parsing

* Should be able to distinguish unset from invalid time properties

* Improve error for property not found

Co-authored-by: Arran Ubels <arran4@gmail.com>

* Remove deprecated ioutil

* Move targeted Go version to 1.20

And test the target forever

* Add method to remove property

Fixes arran4#95

* Merged

* New tool

* Reintegration of arran4#67

* Resynced and moved some functions around arran4#78

* Test fixed.

* Create bug.md

* Create other.md

* Create default.md

* Rename default.md to pull_request_template.md

* Minor update

* refactor: remove unnecessary named return values, harmonizing code base

* refactor: rename var to reflect what it is

These functions take optional variadic PropertyParam arguments, in ical
speak they are not properties, but parameters for a property.

* refactor: use ReplaceAll

* refactor: prefer switch for readability

* refactor: use consistent receiver names

* refactor: rename unused arg as '_'

* Tests added.

* Some larger upgrades.

* Fix

* Some multiple-ness.

* Duplication issue fixed.

* Merged

* Merge remote-tracking branch 'origin/master' into issue97

* origin/master:
  Duplication issue fixed.
  Some multiple-ness.
  Test fixed.
  Resynced and moved some functions around arran4#78
  added functionnal option pattern for url parsing
  usage example, README
  calendar parsing url support

# Conflicts:
#	calendar.go
#	calendar_test.go
#	components.go

---------

Co-authored-by: Dominic Evans <dominic.evans@uk.ibm.com>
Co-authored-by: tradulescu <tristan-mihai.radulescu@etu.umontpellier.fr>
Co-authored-by: Bracken Dawson <abdawson@gmail.com>
Co-authored-by: Daniel Lublin <daniel@lublin.se>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants