Skip to content

Add delete service action to OneDrive integration#168064

Merged
zweckj merged 32 commits into
home-assistant:devfrom
leodrivera:add-onedrive-delete-service
May 4, 2026
Merged

Add delete service action to OneDrive integration#168064
zweckj merged 32 commits into
home-assistant:devfrom
leodrivera:add-onedrive-delete-service

Conversation

@leodrivera
Copy link
Copy Markdown
Contributor

@leodrivera leodrivera commented Apr 12, 2026

Adds a new onedrive.delete service action to the OneDrive integration. This complements the existing onedrive.upload action, allowing users to remove files from their OneDrive app folder directly from automations.

Key design decisions:

  • destination_path (required): one or more paths to remote files within the app folder (e.g. photos/snapshots/image.jpg), consistent with the destination_folder naming convention of the upload action. Multiple paths are deleted in parallel via asyncio.gather, with all OneDriveException errors aggregated into a single translated HomeAssistantError.
  • Permanent deletion respects the existing per-entry delete_permanently option, keeping behavior consistent with backup deletion.
  • Path validation rejects empty strings, .. traversal segments, and : characters before any remote call is made.

Breaking change

N/A

Proposed change

Adds the onedrive.delete service action to the OneDrive integration.

Type of change

  • New feature (which adds functionality to an existing integration)

Additional information

Checklist

  • I understand the code I am submitting and can explain how it works.
  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.
  • Any generated code has been carefully reviewed for correctness and compliance with project standards.

To help with the load of incoming pull requests:

  • I have reviewed two other open pull requests in this repository.

Adds the `onedrive.delete` service action that deletes a file from the
OneDrive app folder. An optional `filename` field allows the local copy
to be removed at the same time. Permanent deletion respects the existing
`delete_permanently` config entry option.
@leodrivera leodrivera requested a review from zweckj as a code owner April 12, 2026 17:57
Copilot AI review requested due to automatic review settings April 12, 2026 17:57
@home-assistant home-assistant Bot added cla-signed has-tests integration: onedrive new-feature Top 200 Integration is ranked within the top 200 by usage labels Apr 12, 2026
@home-assistant
Copy link
Copy Markdown
Contributor

Hey there @zweckj, mind taking a look at this pull request as it has been labeled with an integration (onedrive) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of onedrive can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant mark-draft Mark the pull request as draft.
  • @home-assistant ready-for-review Remove the draft status from the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign onedrive Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant update-branch Update the pull request branch with the base branch.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component, problem in config, problem in device, feature-request) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component, problem in config, problem in device, feature-request) on the pull request.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a new onedrive.delete service to the OneDrive integration to delete a remote file in the app folder, with an optional local-file delete.

Changes:

  • Register onedrive.delete service with schema (destination_path required, optional filename)
  • Add localized strings + service metadata and icon for the new service
  • Add test coverage for delete service happy-path and basic error cases

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
homeassistant/components/onedrive/services.py Implements and registers the new delete service + optional local deletion helper
homeassistant/components/onedrive/strings.json Adds translated error message and service field descriptions for delete
homeassistant/components/onedrive/services.yaml Declares the new delete service fields for service UI/docs
homeassistant/components/onedrive/icons.json Adds an icon mapping for the delete service
tests/components/onedrive/test_services.py Adds tests for remote-only delete, remote+local delete, and error cases

Comment thread homeassistant/components/onedrive/services.py
Comment thread homeassistant/components/onedrive/services.py Outdated
Comment thread homeassistant/components/onedrive/services.py Outdated
Comment thread homeassistant/components/onedrive/services.py Outdated
Comment thread tests/components/onedrive/test_services.py Outdated
Comment thread homeassistant/components/onedrive/services.py
Comment thread homeassistant/components/onedrive/services.py Outdated
- Validate destination_path: reject empty, '..', and ':' segments
- Wrap Path.unlink() in OSError handler raising HomeAssistantError
- Add delete_local_file_error translation key
- Strengthen test assertions with full path format and delete_permanently value
- Add test for delete_permanently=True option wiring
Copilot AI review requested due to automatic review settings April 12, 2026 18:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Comment thread homeassistant/components/onedrive/services.py Outdated
Comment thread homeassistant/components/onedrive/services.py Outdated
Comment thread tests/components/onedrive/test_services.py
Comment thread tests/components/onedrive/test_services.py Outdated
…tion

- Split _delete_local_file into _validate_local_files (pre-check) and
  _delete_local_files (executor job after remote deletion)
- Add no_delete_access_to_path translation key for local path access errors
- Assert delete_drive_item not called when local path pre-validation fails
- Rename delete_error to mention OneDrive explicitly
- Add delete_local_file_error, invalid_destination_path and
  no_delete_access_to_path translation keys to en.json
- Update delete service descriptions for multi-file support
Copilot AI review requested due to automatic review settings April 12, 2026 19:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Comment thread homeassistant/components/onedrive/services.py Outdated
Comment thread homeassistant/components/onedrive/services.py Outdated
Comment thread homeassistant/components/onedrive/services.py Outdated
Comment thread homeassistant/components/onedrive/strings.json
Comment thread homeassistant/components/onedrive/services.py
- Add vol.Length(min=1) to destination_path schema to reject empty lists
- Move is_allowed_path check to event loop; offload only Path.exists() to executor
- Treat FileNotFoundError in unlink() as non-fatal (race condition tolerance)
- Extract _raise_invalid_destination_path helper to remove duplicate error construction
- Update service name/description to reflect multi-file support
- Add tests for empty destination_path and vanishing local file
Copilot AI review requested due to automatic review settings April 12, 2026 19:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comment thread homeassistant/components/onedrive/services.py
Comment thread homeassistant/components/onedrive/strings.json Outdated
Comment thread homeassistant/components/onedrive/strings.json Outdated
Copilot AI review requested due to automatic review settings April 28, 2026 00:34
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread homeassistant/components/onedrive/services.py
Copilot AI review requested due to automatic review settings April 28, 2026 01:04
@leodrivera leodrivera marked this pull request as ready for review April 28, 2026 01:04
@home-assistant home-assistant Bot requested a review from zweckj April 28, 2026 01:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comment thread homeassistant/components/onedrive/strings.json Outdated
Co-authored-by: Josef Zweck <josef@zweck.dev>
Copilot AI review requested due to automatic review settings April 30, 2026 02:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment thread homeassistant/components/onedrive/strings.json Outdated
Comment thread homeassistant/components/onedrive/services.py
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 30, 2026 02:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread homeassistant/components/onedrive/services.py
- Removed specific error handling for single file deletion failure in services.py.
- Updated error messages in strings.json for consistency.
- Adjusted unit tests to reflect changes in error handling and message structure.
@leodrivera leodrivera requested a review from zweckj April 30, 2026 03:14
@zweckj zweckj merged commit 2846dcc into home-assistant:dev May 4, 2026
33 checks passed
@leodrivera leodrivera deleted the add-onedrive-delete-service branch May 4, 2026 18:27
@leodrivera
Copy link
Copy Markdown
Contributor Author

The home-assistant/home-assistant.io#44730 should also be merged

rajlaud pushed a commit to rajlaud/home-assistant that referenced this pull request May 5, 2026
)

Co-authored-by: Josef Zweck <josef@zweck.dev>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@github-actions github-actions Bot locked and limited conversation to collaborators May 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants