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 allday events in custom_calendars #11272

Merged
merged 1 commit into from Dec 22, 2017

Conversation

Projects
None yet
5 participants
@maxlaverse
Copy link
Contributor

commented Dec 22, 2017

Description:

Fixes another issue with the webdav calendar that events that last the whole day are ignored in custom calendars. They should only be ignored on the main calendar.

Checklist:

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@maxlaverse maxlaverse force-pushed the maxlaverse:caldav_all_day branch Dec 22, 2017

@maxlaverse maxlaverse force-pushed the maxlaverse:caldav_all_day branch Dec 22, 2017

tests/components/calendar/test_caldav.py Outdated
@@ -167,6 +167,7 @@ def _add_device(devices):
assert len(devices) == 1
assert devices[0].name == "HomeOffice"
assert devices[0].dev_id == "Second HomeOffice"
assert devices[0].data.include_all_day == True

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Dec 22, 2017

comparison to True should be 'if cond is True:' or 'if cond:'

@maxlaverse maxlaverse force-pushed the maxlaverse:caldav_all_day branch Dec 22, 2017

tests/components/calendar/test_caldav.py Outdated
assert devices[1].name == "Second"
assert devices[1].dev_id == "Second"
self.assertFalse(assert devices[1].data.include_all_day)

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Dec 22, 2017

SyntaxError: invalid syntax

@maxlaverse maxlaverse force-pushed the maxlaverse:caldav_all_day branch to f7655f5 Dec 22, 2017

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Dec 22, 2017

The config option all_day that is mentioned in the docs seem to never have been present in the config validation schema. So I guess it never worked to configure that. So then this is not a breaking change?

https://home-assistant.io/components/calendar.caldav/#all_day

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Dec 22, 2017

Probably wise to mark this breaking anyway to highlight the change in the docs.

@maxlaverse

This comment has been minimized.

Copy link
Contributor Author

commented Dec 22, 2017

Hi @MartinHjelmare
No, it's not a breaking change.
I wanted to remove it before making this component public but I messed it up and did only half of the job. I'll open a PR to also update the documentation.

@maxlaverse maxlaverse referenced this pull request Dec 22, 2017

Merged

Remove all_day configuration key from caldav #4265

1 of 2 tasks complete
@MartinHjelmare
Copy link
Member

left a comment

Ok, thanks!

@MartinHjelmare MartinHjelmare merged commit 46df91f into home-assistant:dev Dec 22, 2017

4 checks passed

cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.07%) to 94.066%
Details
hound No violations found. Woof!

@MartinHjelmare MartinHjelmare added this to the 0.60.1 milestone Dec 22, 2017

ciotlosm added a commit to ciotlosm/home-assistant that referenced this pull request Jan 4, 2018

balloob added a commit that referenced this pull request Jan 5, 2018

@balloob balloob referenced this pull request Jan 5, 2018

Merged

0.60.1 #11470

@home-assistant home-assistant locked and limited conversation to collaborators May 29, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.