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.

Sign up for GitHub

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

Jump to bottom

feat(dav): dispatch out-of-office started and ended events #41440

Merged
merged 1 commit into from
Nov 24, 2023
Merged

feat(dav): dispatch out-of-office started and ended events #41440

merged 1 commit into from
Nov 24, 2023

Conversation

st3iny
Copy link
Member

@st3iny st3iny commented Nov 13, 2023 •
edited by ChristophWurst
Loading

Summary

Adds a new queued job that emits events when a out-of-office period of a user begins and ends.

TODO

  • Test using the integration in Mail
  • Deal with time zones

How to smoke test

  1. Open the availability settings page
  2. Enter out of office data
  3. Save
  4. Look into the oc_jobs database table, find the job for class \OCA\DAV\BackgroundJob\OutOfOfficeEventDispatcherJob, note its id
  5. Run php occ background-job:execute <id> --force-execute

Checklist

@st3iny st3iny added enhancement 2. developing Work in progress labels Nov 13, 2023
@st3iny st3iny added this to the Nextcloud 28 milestone Nov 13, 2023
@st3iny st3iny self-assigned this Nov 13, 2023
This was referenced Nov 14, 2023
Merged
Merged
ChristophWurst
ChristophWurst reviewed Nov 15, 2023
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Looks good! We'll need the user timezone to dispatch this at the correct time

@blizzz blizzz mentioned this pull request Nov 20, 2023
Merged
5 tasks
@st3iny st3iny mentioned this pull request Nov 20, 2023
Merged
2 tasks
@ChristophWurst
Copy link
Member

Deal with time zones

I'm looking into this now

@ChristophWurst
Copy link
Member

TZ handling based on a cleaned-up version of #41461 added. It's a bit messy to have to fetch the current timezone setting everywhere, OTOH this allows the tz to move with the user, should it ever change.

Tests will need adjustment. Looking into that shortly.

@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 23, 2023
View reviewed changes
apps/dav/lib/Db/PropertyMapper.php Fixed Show fixed Hide fixed
apps/dav/lib/Db/PropertyMapper.php Fixed Show fixed Hide fixed
apps/dav/lib/Db/PropertyMapper.php Fixed Show fixed Hide fixed
apps/dav/lib/Db/Property.php
* @method string getPropertyname()
* @method string getPropertyvalue()
*/
class Property extends Entity {

Check notice

Code scanning / Psalm

PropertyNotSetInConstructor Note

Property OCA\DAV\Db\Property::$id is not defined in constructor of OCA\DAV\Db\Property or in any methods called in the constructor
apps/dav/lib/Db/Property.php Fixed Show fixed Hide fixed
@ChristophWurst
Copy link
Member

Psalm and tests should be happy now

@ChristophWurst ChristophWurst marked this pull request as ready for review November 23, 2023 14:41
@ChristophWurst ChristophWurst added 3. to review Waiting for reviews pending documentation This pull request needs an associated documentation update and removed 2. developing Work in progress labels Nov 23, 2023
ChristophWurst
ChristophWurst approved these changes Nov 23, 2023
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

For @st3iny's part

@ChristophWurst ChristophWurst self-assigned this Nov 23, 2023
st3iny
st3iny commented Nov 23, 2023
View reviewed changes
Copy link
Member Author

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

✔️ for Christoph's part. There still is a off-by-one bug that will be fixed in a follow-up.

This was referenced Nov 23, 2023
Merged
Merged
@ChristophWurst
Copy link
Member

Merging because the 28 backport is in too

@ChristophWurst ChristophWurst merged commit eeba809 into master Nov 24, 2023
49 of 51 checks passed
@ChristophWurst ChristophWurst deleted the feat/dav/out-of-office-job branch November 24, 2023 06:20
@ChristophWurst ChristophWurst removed the pending documentation This pull request needs an associated documentation update label Nov 24, 2023
@ChristophWurst ChristophWurst mentioned this pull request Nov 24, 2023
Merged
5 tasks
@ChristophWurst ChristophWurst mentioned this pull request Dec 5, 2023
Closed
@blizzz blizzz mentioned this pull request Mar 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@miaulalala miaulalala Awaiting requested review from miaulalala miaulalala is a code owner

@tcitworld tcitworld Awaiting requested review from tcitworld tcitworld is a code owner

Assignees

@ChristophWurst ChristophWurst

@st3iny st3iny

Labels
3. to review Waiting for reviews enhancement
Projects
💌 📅 👥 Groupware team
Archived in project
Milestone
Nextcloud 29
Development

Successfully merging this pull request may close these issues.
3 participants
@st3iny @ChristophWurst @blizzz