search

humhub / calendar

Create one-time or recurring events, invite and manage attendees, and keep track of all your events with the Calendar module.
28 stars 46 forks source link

SoftDelete & Recurrence #370

Closed luke- closed 1 year ago

luke- commented 1 year ago

We would need to look at how we handle Recurrence. Ideally, all recurrent items should also be SoftDeleted. On together Hard Deleted or Restored.

This should also fixes the current tests

yurabakhtin commented 1 year ago

Copied from here https://github.com/humhub/humhub-internal/issues/31#issuecomment-1522193759:

@luke- I have started this in the PR https://github.com/humhub/calendar/pull/372, but it is not completed, because I am still thinking how it should be handled correctly.

yurabakhtin commented 1 year ago

@luke-

  • But I still don't know how to restore the deleted root, because on wall stream we display only child/recurrence entries, so on restore at least one child entry we could restore the root, but then it is not good to keep in DB the soft deleted child entries, becuase not clear how to handle them in the Calendar table/grid, I will decide this later...

I decided to restore all recurrence/child events on restore at least one event, because currently a cell of the deleted event(of one day) is hidden from the Calendar grid only when event is really deleted and its date is added to the column exdate of the root. Commit https://github.com/humhub/calendar/pull/372/commits/af4d9e2ae23b70e8930e1a3a7062a076d7f5cdb9.

It works now like this:

  1. On delete a recurrence/child event we should call hard deletion, because I don't find a way how to store in DB a single soft deleted recurrence/child event for proper display of the Calendar grid: recurrence_delete_hardly_always

  2. So on restore at least one child event we also should restore the whole recurrence event(root and all recurrence/child events), but as you can see there is a small issue when other related events from the same event are not hidden from the Filter "Deleted" right at the same moment when one event is restored. restore

Please note root/main event record is not visible on wall stream.

Also the menu item "Delete irrevocably" may confuse user, because a recurrence/child event is always hard deleted.

luke- commented 1 year ago

@yurabakhtin Ok, that seems like a good solution for now. The fact that other recurrent events are shortly displayed as deleted in the stream during the restore is not a problem.

can you please also add tests for this behavior.

yurabakhtin commented 1 year ago

@luke-

can you please also add tests for this behavior.

Added unit test "RecurrenceEditTest: Restore recurrent instance" in the commit https://github.com/humhub/calendar/pull/372/commits/8f62dedb10aabfb1bcefd446157b1c5f60fcfc9c.

However I see error in another api test:

api_test

I will find a solution either for REST API module or for core.

luke- commented 1 year ago

@yurabakhtin Thanks, then I'll merge this in the meantime.

yurabakhtin commented 1 year ago

@luke-

However I see error in another api test: I will find a solution either for REST API module or for core.

Fixed in this module PR https://github.com/humhub/calendar/pull/373, as I understand we should refresh or get a record from DB before send a response, because on creating I see the date hour has no 0 before hours less 10, so we have a date like 2023-04-28 8:38:52 but then on comparing from DB we see the date with 0 like 2023-04-28 08:38:52.

luke- commented 1 year ago

@yurabakhtin Oh, here is the bug: https://github.com/humhub/humhub/blob/master/protected/humhub/components/ActiveRecord.php#L52

Can you please create a 'master' PR for this.

yurabakhtin commented 1 year ago

@yurabakhtin Oh, here is the bug: https://github.com/humhub/humhub/blob/master/protected/humhub/components/ActiveRecord.php#L52

Can you please create a 'master' PR for this.

@luke- Thanks you for the point to the line. After this fix https://github.com/humhub/humhub/pull/6264 we might optimize similar fixes in modules where REST API is used to send a record after create/update actions, e.g. here:

so we don't need any more to do such addiitonal request from DB after creating/updating a record.